Add effective_url to http_async (#16899)

### What does this PR try to resolve?

Adds `effective_url` to `http_async`. The effective URL is already
captured in some errors when using curl directly via the
`HttpNotSuccessful::new_from_handle` function. Effective URL may differ
from the original URL due to redirects.

This change adds the same functionality in `http_async` with a new
`HttpNotSuccessful::new_from_response` function. The new function is
then used in sparse registry, which impacts error messages.

Additionally, the http_async `curl` errors are changed to be transparent
to avoid making even longer error chains.

cc #16845

Author: weihanglo

src/cargo/sources/registry/http_remote.rs

@@ -18,7 +18,6 @@ use crate::util::cache_lock::CacheLockMode;
 use crate::util::errors::CargoResult;
 use crate::util::errors::HttpNotSuccessful;
 use crate::util::interning::InternedString;
-use crate::util::network::http_async::ResponsePartsExtensions;
 use crate::util::network::retry::Retry;
 use crate::util::network::retry::RetryResult;
 use anyhow::Context as _;
@@ -27,6 +26,7 @@ use cargo_util::paths;
 use futures::lock::Mutex;
 use http::HeaderName;
 use http::HeaderValue;
+use http::Response;
 use std::cell::Cell;
 use std::cell::RefCell;
 use std::collections::HashSet;
@@ -599,17 +599,10 @@ impl<'gctx> HttpBackend<'gctx> {
                     }
                 }
 
-                let mut err = Err(HttpNotSuccessful {
-                    code: http::StatusCode::UNAUTHORIZED.as_u16() as u32,
-                    body: body,
-                    url: full_url,
-                    ip: None,
-                    headers: response
-                        .headers
-                        .iter()
-                        .map(|(k, v)| format!("{}: {}", k, v.to_str().unwrap_or_default()))
-                        .collect(),
-                }
+                let mut err = Err(HttpNotSuccessful::new_from_response(
+                    Response::from_parts(response, body),
+                    &full_url,
+                )
                 .into());
                 if self.auth_required.get() {
                     let auth_error = auth::AuthorizationError::new(
@@ -622,17 +615,10 @@ impl<'gctx> HttpBackend<'gctx> {
                 }
                 err
             }
-            code => Err(HttpNotSuccessful {
-                code: code.as_u16() as u32,
-                body: body,
-                url: full_url,
-                ip: response.client_ip().map(str::to_owned),
-                headers: response
-                    .headers
-                    .iter()
-                    .map(|(k, v)| format!("{}: {}", k, v.to_str().unwrap_or_default()))
-                    .collect(),
-            }
+            _ => Err(HttpNotSuccessful::new_from_response(
+                Response::from_parts(response, body),
+                &full_url,
+            )
             .into()),
         }
     }

src/cargo/util/errors.rs

@@ -1,8 +1,11 @@
 use anyhow::Error;
 use curl::easy::Easy;
+use http::Response;
 use std::fmt::{self, Write};
 use std::path::PathBuf;
 
+use crate::util::network::http_async::ResponsePartsExtensions;
+
 use super::truncate_with_ellipsis;
 
 pub type CargoResult<T> = anyhow::Result<T>;
@@ -59,6 +62,24 @@ impl HttpNotSuccessful {
         }
     }
 
+    pub fn new_from_response(response: Response<Vec<u8>>, url: &str) -> HttpNotSuccessful {
+        let ip = response.client_ip().map(str::to_string);
+        let url = response.effective_url().unwrap_or(url).to_string();
+        let (head, body) = response.into_parts();
+        let headers = head
+            .headers
+            .into_iter()
+            .filter_map(|(k, v)| Some(format!("{}: {}", k?.as_str(), v.to_str().ok()?)))
+            .collect();
+        HttpNotSuccessful {
+            code: head.status.as_u16() as u32,
+            url,
+            ip,
+            body,
+            headers,
+        }
+    }
+
     /// Renders the error in a compact form.
     pub fn display_short(&self) -> String {
         self.render(false)

src/cargo/util/network/http_async.rs

@@ -25,10 +25,10 @@ type HttpResult<T> = std::result::Result<T, Error>;
 #[derive(Debug, Clone, thiserror::Error)]
 #[non_exhaustive]
 pub enum Error {
-    #[error("curl multi failed")]
+    #[error(transparent)]
     Multi(#[from] curl::MultiError),
 
-    #[error("curl failed")]
+    #[error(transparent)]
     Easy(#[from] curl::Error),
 
     #[error("failed to convert header value of `{name}` to string: {bytes:?}")]
@@ -212,6 +212,7 @@ impl WorkerServer {
                         // Would be nice to set HTTP version via `response.version_mut()`, but `curl` doesn't have it exposed.
                         let extensions = Extensions {
                             client_ip: easy.primary_ip().ok().flatten().map(str::to_string),
+                            effective_url: easy.effective_url().ok().flatten().map(str::to_string),
                         };
                         response.extensions_mut().insert(extensions);
                         let _ = sender.send(result.map(|()| response).map_err(Into::into));
@@ -340,10 +341,12 @@ impl Handler for Collector {
 #[derive(Clone)]
 struct Extensions {
     client_ip: Option<String>,
+    effective_url: Option<String>,
 }
 
 pub trait ResponsePartsExtensions {
     fn client_ip(&self) -> Option<&str>;
+    fn effective_url(&self) -> Option<&str>;
 }
 
 impl ResponsePartsExtensions for http::response::Parts {
@@ -352,6 +355,12 @@ impl ResponsePartsExtensions for http::response::Parts {
             .get::<Extensions>()
             .and_then(|extensions| extensions.client_ip.as_deref())
     }
+
+    fn effective_url(&self) -> Option<&str> {
+        self.extensions
+            .get::<Extensions>()
+            .and_then(|extensions| extensions.effective_url.as_deref())
+    }
 }
 
 impl ResponsePartsExtensions for Response {
@@ -360,6 +369,12 @@ impl ResponsePartsExtensions for Response {
             .get::<Extensions>()
             .and_then(|extensions| extensions.client_ip.as_deref())
     }
+
+    fn effective_url(&self) -> Option<&str> {
+        self.extensions()
+            .get::<Extensions>()
+            .and_then(|extensions| extensions.effective_url.as_deref())
+    }
 }
 
 /// Splits HTTP `HEADER: VALUE` to a tuple.

tests/testsuite/registry_auth.rs

@@ -234,7 +234,7 @@ Caused by:
   [NOTE] the token does not include an authentication scheme
 
 Caused by:
-  failed to get successful HTTP response from `http://127.0.0.1:[..]/index/config.json`, got 401
+  failed to get successful HTTP response from `http://127.0.0.1:[..]/index/config.json` ([..]), got 401
   body:
   Unauthorized message from server.
 
@@ -276,7 +276,7 @@ Caused by:
   [NOTE] the token does not include an authentication scheme
 
 Caused by:
-  failed to get successful HTTP response from `http://127.0.0.1:[..]/index/config.json`, got 401
+  failed to get successful HTTP response from `http://127.0.0.1:[..]/index/config.json` ([..]), got 401
   body:
   Unauthorized message from server.
 
@@ -321,7 +321,7 @@ Caused by:
   [NOTE] the token does not include an authentication scheme
 
 Caused by:
-  failed to get successful HTTP response from `http://127.0.0.1:[..]/index/config.json`, got 401
+  failed to get successful HTTP response from `http://127.0.0.1:[..]/index/config.json` ([..]), got 401
   body:
   Unauthorized message from server.
 
@@ -416,7 +416,7 @@ Caused by:
   [NOTE] the token does not include an authentication scheme
 
 Caused by:
-  failed to get successful HTTP response from `http://127.0.0.1:[..]/index/config.json`, got 401
+  failed to get successful HTTP response from `http://127.0.0.1:[..]/index/config.json` ([..]), got 401
   body:
   Unauthorized message from server.
 
@@ -480,7 +480,7 @@ Caused by:
   or use environment variable CARGO_REGISTRIES_ALTERNATIVE_TOKEN
 
 Caused by:
-  failed to get successful HTTP response from `http://127.0.0.1:[..]/index/config.json`, got 401
+  failed to get successful HTTP response from `http://127.0.0.1:[..]/index/config.json` ([..]), got 401
   body:
   Unauthorized message from server.