improve retries more

src/cli.rs

@@ -173,7 +173,7 @@ pub struct Ic {
     #[clap(env, long)]
     pub ic_root_key: Option<PathBuf>,
 
-    /// Maximum mumber of request retries in the transport layer for both network- and http-related failures.
+    /// Maximum number of request retries for connection failures.
     #[clap(env, long, default_value = "5")]
     pub ic_max_request_retries: u32,
 

src/routing/ic/transport.rs

@@ -2,7 +2,6 @@
 
 use std::{cell::RefCell, pin::Pin, sync::Arc, time::Duration};
 
-use bytes::Bytes;
 use futures::Future;
 use futures_util::StreamExt;
 use ic_agent::{
@@ -18,6 +17,7 @@ use reqwest::{
     Body, Method, Request, StatusCode,
 };
 use tokio::task_local;
+use url::Url;
 
 use crate::http::{headers::CONTENT_TYPE_CBOR, Client as HttpClient};
 
@@ -120,32 +120,38 @@ impl ReqwestTransport {
         endpoint: &str,
         body: Option<Vec<u8>>,
     ) -> Result<(StatusCode, Vec<u8>), AgentError> {
-        // Convert body to Bytes to make it cheaply cloneable between retries.
-        // Underlying Reqwest body is anyway Bytes and can be then used as-is.
-        let body = body.map(Bytes::from);
+        // Create the initial request with a fake URL which will be overridden later
+        let mut http_request = Request::new(method.clone(), Url::parse("http://foo").unwrap());
 
-        let create_request_with_generated_url = || -> Result<Request, AgentError> {
-            let url = self.route_provider.route()?.join(endpoint)?;
-            let hostname = url.authority().to_string();
+        http_request
+            .headers_mut()
+            .insert(CONTENT_TYPE, CONTENT_TYPE_CBOR);
 
-            let mut http_request = Request::new(method.clone(), url);
-            http_request
-                .headers_mut()
-                .insert(CONTENT_TYPE, CONTENT_TYPE_CBOR);
+        // Add HTTP headers if requested
+        let _ = CONTEXT.try_with(|x| {
+            let mut ctx = x.borrow_mut();
 
-            // Add HTTP headers if requested
-            let _ = CONTEXT.try_with(|x| {
-                let mut ctx = x.borrow_mut();
-                ctx.hostname = Some(hostname);
+            for (k, v) in &ctx.headers_out {
+                http_request.headers_mut().append(k, v.clone());
+            }
 
-                for (k, v) in &ctx.headers_out {
-                    http_request.headers_mut().append(k, v.clone());
-                }
+            ctx.headers_out.clear();
+        });
 
-                ctx.headers_out.clear();
+        *http_request.body_mut() = body.clone().map(Body::from);
+
+        let create_request_with_generated_url = || -> Result<Request, AgentError> {
+            let url = self.route_provider.route()?.join(endpoint)?;
+
+            // Update/set the hostname
+            let _ = CONTEXT.try_with(|x| {
+                x.borrow_mut().hostname = Some(url.authority().to_string());
             });
 
-            *http_request.body_mut() = body.clone().map(Body::from);
+            // This cannot fail since the body is always cloneable.
+            // Cloning is also cheap because the body is Bytes under the hood.
+            let mut http_request = http_request.try_clone().unwrap();
+            *http_request.url_mut() = url;
 
             Ok(http_request)
         };
@@ -177,7 +183,8 @@ impl ReqwestTransport {
                                     }
 
                                     retries -= 1;
-                                    // Sleep before retrying. Delay time is not changed, as is the case for http retry.
+
+                                    // Sleep before retrying
                                     tokio::time::sleep(delay).await;
 
                                     continue;