Repository: seanmonstar/reqwest (language: rust)

Issue:
connect_timeout does not respect multiple IP addresses
> NOTE: This might be incorrect as I haven't actually tested it, but I've been looking at the code to get this understanding.

## Problem

We have an interesting problem where our load balancer occasionally returns 3 IP addresses, and 1 of these addresses seemingly refuses to connect. If we get unlucky and that IP is first in the list, then it fails the request. `hyper` is able to try all 3 IP addresses while connecting, but it appears that `reqwest` is not able to.

## Diagnosis

`hyper` exposes a `connect_timeout` on `HttpConnector` which is [divided in order to share the timeout across multiple IP addresses](https://github.com/hyperium/hyper/blob/d342c2c714498d33891fa285a3c9ae991dc34769/src/client/connect/http.rs#L524)

`reqwest` also exposes a `connect_timeout`. However, [this wraps the entire `HttpConnector` in a timeout](https://github.com/seanmonstar/reqwest/blob/1f6c2cfcf9315a9721314171568a41beebf5b311/src/connect.rs#L440-L443), and does not forward the timeout to `hyper`




Discussion / hints:
Nice find! It seems like we could just simply also enable the timeout for hyper's connector. We still need it in reqwest, since it includes TLS handshake too.
Feel free to assign me. I'll raise a PR shortly

Fix patch:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 79089704c..be2467078 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -277,10 +277,11 @@ jobs:
         with:
           toolchain: ${{ steps.metadata.outputs.msrv }}
 
-      - name: Make sure log v0.4.18 is used
+      - name: Fix log and tokio versions
         run: |
           cargo update
           cargo update -p log --precise 0.4.18
+          cargo update -p tokio --precise 1.29.1
 
       - uses: Swatinem/rust-cache@v2
 
diff --git a/src/async_impl/client.rs b/src/async_impl/client.rs
index 23e491734..ad5bc3d80 100644
--- a/src/async_impl/client.rs
+++ b/src/async_impl/client.rs
@@ -279,7 +279,8 @@ impl ClientBuilder {
                     config.dns_overrides,
                 ));
             }
-            let http = HttpConnector::new_with_resolver(DynResolver::new(resolver.clone()));
+            let mut http = HttpConnector::new_with_resolver(DynResolver::new(resolver.clone()));
+            http.set_connect_timeout(config.connect_timeout);
 
             #[cfg(feature = "__tls")]
             match config.tls {
diff --git a/src/error.rs b/src/error.rs
index 0e6bd247d..9453fcd92 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -105,6 +105,11 @@ impl Error {
             if err.is::<TimedOut>() {
                 return true;
             }
+            if let Some(io) = err.downcast_ref::<io::Error>() {
+                if io.kind() == io::ErrorKind::TimedOut {
+                    return true;
+                }
+            }
             source = err.source();
         }
 


Test patch:
diff --git a/tests/timeouts.rs b/tests/timeouts.rs
index caf48c27b..355e059ce 100644
--- a/tests/timeouts.rs
+++ b/tests/timeouts.rs
@@ -86,6 +86,63 @@ async fn connect_timeout() {
     assert!(err.is_connect() && err.is_timeout());
 }
 
+#[cfg(not(target_arch = "wasm32"))]
+#[tokio::test]
+async fn connect_many_timeout_succeeds() {
+    let _ = env_logger::try_init();
+
+    let server = server::http(move |_req| async { http::Response::default() });
+    let port = server.addr().port();
+
+    let client = reqwest::Client::builder()
+        .resolve_to_addrs(
+            "many_addrs",
+            &["10.255.255.1:81".parse().unwrap(), server.addr()],
+        )
+        .connect_timeout(Duration::from_millis(100))
+        .build()
+        .unwrap();
+
+    let url = format!("http://many_addrs:{port}/eventual");
+
+    let _res = client
+        .get(url)
+        .timeout(Duration::from_millis(1000))
+        .send()
+        .await
+        .unwrap();
+}
+
+#[cfg(not(target_arch = "wasm32"))]
+#[tokio::test]
+async fn connect_many_timeout() {
+    let _ = env_logger::try_init();
+
+    let client = reqwest::Client::builder()
+        .resolve_to_addrs(
+            "many_addrs",
+            &[
+                "10.255.255.1:81".parse().unwrap(),
+                "10.255.255.2:81".parse().unwrap(),
+            ],
+        )
+        .connect_timeout(Duration::from_millis(100))
+        .build()
+        .unwrap();
+
+    let url = format!("http://many_addrs:81/slow");
+
+    let res = client
+        .get(url)
+        .timeout(Duration::from_millis(1000))
+        .send()
+        .await;
+
+    let err = res.unwrap_err();
+
+    assert!(err.is_connect() && err.is_timeout());
+}
+
 #[tokio::test]
 async fn response_timeout() {
     let _ = env_logger::try_init();