From 3eda248ef5678fe07b5424fb4f256011800fbb15 Mon Sep 17 00:00:00 2001 From: konsti Date: Fri, 25 Oct 2024 11:10:18 +0200 Subject: [PATCH] Always attach URL to network errors (#8444) --- crates/uv-client/src/cached_client.rs | 13 +++--- crates/uv-client/src/error.rs | 20 ++++----- crates/uv-client/src/flat_index.rs | 7 ++- crates/uv-client/src/registry_client.rs | 27 ++++++++---- crates/uv-client/src/tls.rs | 7 ++- .../src/distribution_database.rs | 3 +- crates/uv-publish/src/lib.rs | 8 +++- crates/uv-publish/src/trusted_publishing.rs | 43 +++++++++++++----- crates/uv-python/src/downloads.rs | 37 +++++++++------- crates/uv-requirements-txt/src/lib.rs | 44 +++++++++---------- crates/uv/tests/it/lock.rs | 3 ++ crates/uv/tests/it/pip_compile.rs | 1 + 12 files changed, 130 insertions(+), 83 deletions(-) diff --git a/crates/uv-client/src/cached_client.rs b/crates/uv-client/src/cached_client.rs index f365227e9d79..56266c6f88e0 100644 --- a/crates/uv-client/src/cached_client.rs +++ b/crates/uv-client/src/cached_client.rs @@ -468,9 +468,9 @@ impl CachedClient { .execute(req) .instrument(info_span!("revalidation_request", url = url.as_str())) .await - .map_err(ErrorKind::from)? + .map_err(|err| ErrorKind::from_reqwest_middleware(url.clone(), err))? .error_for_status() - .map_err(ErrorKind::from)?; + .map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?; match cached .cache_policy .after_response(new_cache_policy_builder, &response) @@ -500,16 +500,17 @@ impl CachedClient { &self, req: Request, ) -> Result<(Response, Option>), Error> { - trace!("Sending fresh {} request for {}", req.method(), req.url()); + let url = req.url().clone(); + trace!("Sending fresh {} request for {}", req.method(), url); let cache_policy_builder = CachePolicyBuilder::new(&req); let response = self .0 - .for_host(req.url()) + .for_host(&url) .execute(req) .await - .map_err(ErrorKind::from)? + .map_err(|err| ErrorKind::from_reqwest_middleware(url.clone(), err))? .error_for_status() - .map_err(ErrorKind::from)?; + .map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?; let cache_policy = cache_policy_builder.build(&response); let cache_policy = if cache_policy.to_archived().is_storable() { Some(Box::new(cache_policy)) diff --git a/crates/uv-client/src/error.rs b/crates/uv-client/src/error.rs index cded0db494b7..f55d351814e3 100644 --- a/crates/uv-client/src/error.rs +++ b/crates/uv-client/src/error.rs @@ -73,7 +73,7 @@ impl Error { // The server returned a "Method Not Allowed" error, indicating it doesn't support // HEAD requests, so we can't check for range requests. - ErrorKind::WrappedReqwestError(err) => { + ErrorKind::WrappedReqwestError(_url, err) => { if let Some(status) = err.status() { // If the server doesn't support HEAD requests, we can't check for range // requests. @@ -180,8 +180,8 @@ pub enum ErrorKind { MetadataNotFound(WheelFilename, String), /// An error that happened while making a request or in a reqwest middleware. - #[error(transparent)] - WrappedReqwestError(#[from] WrappedReqwestError), + #[error("Failed to fetch: `{0}`")] + WrappedReqwestError(Url, #[source] WrappedReqwestError), #[error("Received some unexpected JSON from {url}")] BadJson { source: serde_json::Error, url: Url }, @@ -208,7 +208,7 @@ pub enum ErrorKind { CacheWrite(#[source] std::io::Error), #[error(transparent)] - Io(#[from] std::io::Error), + Io(std::io::Error), #[error("Cache deserialization failed")] Decode(#[source] rmp_serde::decode::Error), @@ -235,21 +235,19 @@ pub enum ErrorKind { Offline(String), } -impl From for ErrorKind { - fn from(error: reqwest::Error) -> Self { - Self::WrappedReqwestError(WrappedReqwestError::from(error)) +impl ErrorKind { + pub(crate) fn from_reqwest(url: Url, error: reqwest::Error) -> Self { + Self::WrappedReqwestError(url, WrappedReqwestError::from(error)) } -} -impl From for ErrorKind { - fn from(err: reqwest_middleware::Error) -> Self { + pub(crate) fn from_reqwest_middleware(url: Url, err: reqwest_middleware::Error) -> Self { if let reqwest_middleware::Error::Middleware(ref underlying) = err { if let Some(err) = underlying.downcast_ref::() { return Self::Offline(err.url().to_string()); } } - Self::WrappedReqwestError(WrappedReqwestError(err)) + Self::WrappedReqwestError(url, WrappedReqwestError(err)) } } diff --git a/crates/uv-client/src/flat_index.rs b/crates/uv-client/src/flat_index.rs index 9b9d6fe618db..ef7caf7fd17d 100644 --- a/crates/uv-client/src/flat_index.rs +++ b/crates/uv-client/src/flat_index.rs @@ -160,14 +160,17 @@ impl<'a> FlatIndexClient<'a> { .header("Accept-Encoding", "gzip") .header("Accept", "text/html") .build() - .map_err(ErrorKind::from)?; + .map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?; let parse_simple_response = |response: Response| { async { // Use the response URL, rather than the request URL, as the base for relative URLs. // This ensures that we handle redirects and other URL transformations correctly. let url = response.url().clone(); - let text = response.text().await.map_err(ErrorKind::from)?; + let text = response + .text() + .await + .map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?; let SimpleHtml { base, files } = SimpleHtml::parse(&text, &url) .map_err(|err| Error::from_html_err(err, url.clone()))?; diff --git a/crates/uv-client/src/registry_client.rs b/crates/uv-client/src/registry_client.rs index 2930201ceefc..91bc991c0156 100644 --- a/crates/uv-client/src/registry_client.rs +++ b/crates/uv-client/src/registry_client.rs @@ -232,7 +232,7 @@ impl RegistryClient { } Err(err) => match err.into_kind() { // The package could not be found in the remote index. - ErrorKind::WrappedReqwestError(err) => match err.status() { + ErrorKind::WrappedReqwestError(url, err) => match err.status() { Some(StatusCode::NOT_FOUND) => {} Some(StatusCode::UNAUTHORIZED) => { capabilities.set_unauthorized(index.clone()); @@ -240,7 +240,7 @@ impl RegistryClient { Some(StatusCode::FORBIDDEN) => { capabilities.set_forbidden(index.clone()); } - _ => return Err(ErrorKind::from(err).into()), + _ => return Err(ErrorKind::WrappedReqwestError(url, err).into()), }, // The package is unavailable due to a lack of connectivity. @@ -323,7 +323,7 @@ impl RegistryClient { .header("Accept-Encoding", "gzip") .header("Accept", MediaType::accepts()) .build() - .map_err(ErrorKind::from)?; + .map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?; let parse_simple_response = |response: Response| { async { // Use the response URL, rather than the request URL, as the base for relative URLs. @@ -347,14 +347,20 @@ impl RegistryClient { let unarchived = match media_type { MediaType::Json => { - let bytes = response.bytes().await.map_err(ErrorKind::from)?; + let bytes = response + .bytes() + .await + .map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?; let data: SimpleJson = serde_json::from_slice(bytes.as_ref()) .map_err(|err| Error::from_json_err(err, url.clone()))?; SimpleMetadata::from_files(data.files, package_name, &url) } MediaType::Html => { - let text = response.text().await.map_err(ErrorKind::from)?; + let text = response + .text() + .await + .map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?; SimpleMetadata::from_html(&text, package_name, &url)? } }; @@ -547,7 +553,10 @@ impl RegistryClient { }; let response_callback = |response: Response| async { - let bytes = response.bytes().await.map_err(ErrorKind::from)?; + let bytes = response + .bytes() + .await + .map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?; info_span!("parse_metadata21") .in_scope(|| ResolutionMetadata::parse_metadata(bytes.as_ref())) @@ -563,7 +572,7 @@ impl RegistryClient { .uncached_client(&url) .get(url.clone()) .build() - .map_err(ErrorKind::from)?; + .map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?; Ok(self .cached_client() .get_serde(req, &cache_entry, cache_control, response_callback) @@ -616,7 +625,7 @@ impl RegistryClient { http::HeaderValue::from_static("identity"), ) .build() - .map_err(ErrorKind::from)?; + .map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?; // Copy authorization headers from the HEAD request to subsequent requests let mut headers = HeaderMap::default(); @@ -694,7 +703,7 @@ impl RegistryClient { reqwest::header::HeaderValue::from_static("identity"), ) .build() - .map_err(ErrorKind::from)?; + .map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?; // Stream the file, searching for the METADATA. let read_metadata_stream = |response: Response| { diff --git a/crates/uv-client/src/tls.rs b/crates/uv-client/src/tls.rs index 41aef9613bca..8a807234b2fa 100644 --- a/crates/uv-client/src/tls.rs +++ b/crates/uv-client/src/tls.rs @@ -7,12 +7,15 @@ pub(crate) enum CertificateError { #[error(transparent)] Io(#[from] std::io::Error), #[error(transparent)] - Reqwest(#[from] reqwest::Error), + Reqwest(reqwest::Error), } /// Return the `Identity` from the provided file. pub(crate) fn read_identity(ssl_client_cert: &OsStr) -> Result { let mut buf = Vec::new(); fs_err::File::open(ssl_client_cert)?.read_to_end(&mut buf)?; - Ok(Identity::from_pem(&buf)?) + Identity::from_pem(&buf).map_err(|tls_err| { + debug_assert!(tls_err.is_builder(), "must be a rustls::Error internally"); + CertificateError::Reqwest(tls_err) + }) } diff --git a/crates/uv-distribution/src/distribution_database.rs b/crates/uv-distribution/src/distribution_database.rs index 03ddd809de0f..f96154f33057 100644 --- a/crates/uv-distribution/src/distribution_database.rs +++ b/crates/uv-distribution/src/distribution_database.rs @@ -87,7 +87,8 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> { io::Error::new( io::ErrorKind::TimedOut, format!( - "Failed to download distribution due to network timeout. Try increasing UV_HTTP_TIMEOUT (current value: {}s).", self.client.unmanaged.timeout().as_secs() + "Failed to download distribution due to network timeout. Try increasing UV_HTTP_TIMEOUT (current value: {}s).", + self.client.unmanaged.timeout().as_secs() ), ) } else { diff --git a/crates/uv-publish/src/lib.rs b/crates/uv-publish/src/lib.rs index d3a68a1c6509..5d180a9ef0bc 100644 --- a/crates/uv-publish/src/lib.rs +++ b/crates/uv-publish/src/lib.rs @@ -79,7 +79,7 @@ pub enum PublishPrepareError { #[derive(Error, Debug)] pub enum PublishSendError { #[error("Failed to send POST request")] - ReqwestMiddleware(#[from] reqwest_middleware::Error), + ReqwestMiddleware(#[source] reqwest_middleware::Error), #[error("Upload failed with status {0}")] StatusNoBody(StatusCode, #[source] reqwest::Error), #[error("Upload failed with status code {0}. Server says: {1}")] @@ -336,7 +336,11 @@ pub async fn upload( } let response = result.map_err(|err| { - PublishError::PublishSend(file.to_path_buf(), registry.clone(), err.into()) + PublishError::PublishSend( + file.to_path_buf(), + registry.clone(), + PublishSendError::ReqwestMiddleware(err), + ) })?; return handle_response(registry, response) diff --git a/crates/uv-publish/src/trusted_publishing.rs b/crates/uv-publish/src/trusted_publishing.rs index 98b6d93159dc..8230e60079e6 100644 --- a/crates/uv-publish/src/trusted_publishing.rs +++ b/crates/uv-publish/src/trusted_publishing.rs @@ -17,10 +17,10 @@ pub enum TrustedPublishingError { Var(#[from] VarError), #[error(transparent)] Url(#[from] url::ParseError), - #[error(transparent)] - Reqwest(#[from] reqwest::Error), - #[error(transparent)] - ReqwestMiddleware(#[from] reqwest_middleware::Error), + #[error("Failed to fetch: `{0}`")] + Reqwest(Url, #[source] reqwest::Error), + #[error("Failed to fetch: `{0}`")] + ReqwestMiddleware(Url, #[source] reqwest_middleware::Error), #[error(transparent)] SerdeJson(#[from] serde_json::error::Error), #[error( @@ -105,8 +105,17 @@ async fn get_audience( // (RFC 3986). let audience_url = Url::parse(&format!("https://{}/_/oidc/audience", registry.authority()))?; debug!("Querying the trusted publishing audience from {audience_url}"); - let response = client.get(audience_url).send().await?; - let audience = response.error_for_status()?.json::().await?; + let response = client + .get(audience_url.clone()) + .send() + .await + .map_err(|err| TrustedPublishingError::ReqwestMiddleware(audience_url.clone(), err))?; + let audience = response + .error_for_status() + .map_err(|err| TrustedPublishingError::Reqwest(audience_url.clone(), err))? + .json::() + .await + .map_err(|err| TrustedPublishingError::Reqwest(audience_url.clone(), err))?; trace!("The audience is `{}`", &audience.audience); Ok(audience.audience) } @@ -123,11 +132,17 @@ async fn get_oidc_token( debug!("Querying the trusted publishing OIDC token from {oidc_token_url}"); let authorization = format!("bearer {oidc_token_request_token}"); let response = client - .get(oidc_token_url) + .get(oidc_token_url.clone()) .header(header::AUTHORIZATION, authorization) .send() - .await?; - let oidc_token: OidcToken = response.error_for_status()?.json().await?; + .await + .map_err(|err| TrustedPublishingError::ReqwestMiddleware(oidc_token_url.clone(), err))?; + let oidc_token: OidcToken = response + .error_for_status() + .map_err(|err| TrustedPublishingError::Reqwest(oidc_token_url.clone(), err))? + .json() + .await + .map_err(|err| TrustedPublishingError::Reqwest(oidc_token_url.clone(), err))?; Ok(oidc_token.value) } @@ -145,14 +160,18 @@ async fn get_publish_token( token: oidc_token.to_string(), }; let response = client - .post(mint_token_url) + .post(mint_token_url.clone()) .body(serde_json::to_vec(&mint_token_payload)?) .send() - .await?; + .await + .map_err(|err| TrustedPublishingError::ReqwestMiddleware(mint_token_url.clone(), err))?; // reqwest's implementation of `.json()` also goes through `.bytes()` let status = response.status(); - let body = response.bytes().await?; + let body = response + .bytes() + .await + .map_err(|err| TrustedPublishingError::Reqwest(mint_token_url.clone(), err))?; if status.is_success() { let publish_token: PublishToken = serde_json::from_slice(&body)?; diff --git a/crates/uv-python/src/downloads.rs b/crates/uv-python/src/downloads.rs index 4e3e3f317f6a..ac1bb5ed4a1d 100644 --- a/crates/uv-python/src/downloads.rs +++ b/crates/uv-python/src/downloads.rs @@ -39,10 +39,10 @@ pub enum Error { InvalidPythonVersion(String), #[error("Invalid request key (too many parts): {0}")] TooManyParts(String), - #[error(transparent)] - NetworkError(#[from] WrappedReqwestError), - #[error(transparent)] - NetworkMiddlewareError(#[from] anyhow::Error), + #[error("Failed to download {0}")] + NetworkError(Url, #[source] WrappedReqwestError), + #[error("Failed to download {0}")] + NetworkMiddlewareError(Url, #[source] anyhow::Error), #[error("Failed to extract archive: {0}")] ExtractError(String, #[source] uv_extract::Error), #[error("Failed to hash installation")] @@ -619,18 +619,18 @@ impl ManagedPythonDownload { } } -impl From for Error { - fn from(error: reqwest::Error) -> Self { - Self::NetworkError(WrappedReqwestError::from(error)) +impl Error { + pub(crate) fn from_reqwest(url: Url, err: reqwest::Error) -> Self { + Self::NetworkError(url, WrappedReqwestError::from(err)) } -} -impl From for Error { - fn from(error: reqwest_middleware::Error) -> Self { - match error { - reqwest_middleware::Error::Middleware(error) => Self::NetworkMiddlewareError(error), + pub(crate) fn from_reqwest_middleware(url: Url, err: reqwest_middleware::Error) -> Self { + match err { + reqwest_middleware::Error::Middleware(error) => { + Self::NetworkMiddlewareError(url, error) + } reqwest_middleware::Error::Reqwest(error) => { - Self::NetworkError(WrappedReqwestError::from(error)) + Self::NetworkError(url, WrappedReqwestError::from(error)) } } } @@ -701,10 +701,17 @@ async fn read_url( Ok((Either::Left(reader), Some(size))) } else { - let response = client.for_host(url).get(url.clone()).send().await?; + let response = client + .for_host(url) + .get(url.clone()) + .send() + .await + .map_err(|err| Error::from_reqwest_middleware(url.clone(), err))?; // Ensure the request was successful. - response.error_for_status_ref()?; + response + .error_for_status_ref() + .map_err(|err| Error::from_reqwest(url.clone(), err))?; let size = response.content_length(); let stream = response diff --git a/crates/uv-requirements-txt/src/lib.rs b/crates/uv-requirements-txt/src/lib.rs index 3ebe38b40951..1983f2dadb62 100644 --- a/crates/uv-requirements-txt/src/lib.rs +++ b/crates/uv-requirements-txt/src/lib.rs @@ -800,14 +800,19 @@ async fn read_url_to_string( let url = Url::from_str(path_utf8) .map_err(|err| RequirementsTxtParserError::InvalidUrl(path_utf8.to_string(), err))?; - Ok(client + let response = client .for_host(&url) - .get(url) + .get(url.clone()) .send() - .await? - .error_for_status()? + .await + .map_err(|err| RequirementsTxtParserError::from_reqwest_middleware(url.clone(), err))?; + let text = response + .error_for_status() + .map_err(|err| RequirementsTxtParserError::from_reqwest(url.clone(), err))? .text() - .await?) + .await + .map_err(|err| RequirementsTxtParserError::from_reqwest(url.clone(), err))?; + Ok(text) } /// Error parsing requirements.txt, wrapper with filename @@ -891,7 +896,7 @@ pub enum RequirementsTxtParserError { url: PathBuf, }, #[cfg(feature = "http")] - Reqwest(reqwest_middleware::Error), + Reqwest(Url, reqwest_middleware::Error), #[cfg(feature = "http")] InvalidUrl(String, url::ParseError), } @@ -957,8 +962,8 @@ impl Display for RequirementsTxtParserError { ) } #[cfg(feature = "http")] - Self::Reqwest(err) => { - write!(f, "Error while accessing remote requirements file {err}") + Self::Reqwest(url, _err) => { + write!(f, "Error while accessing remote requirements file: `{url}`") } #[cfg(feature = "http")] Self::InvalidUrl(url, err) => { @@ -989,7 +994,7 @@ impl std::error::Error for RequirementsTxtParserError { Self::Parser { .. } => None, Self::NonUnicodeUrl { .. } => None, #[cfg(feature = "http")] - Self::Reqwest(err) => err.source(), + Self::Reqwest(_, err) => err.source(), #[cfg(feature = "http")] Self::InvalidUrl(_, err) => err.source(), } @@ -1117,12 +1122,8 @@ impl Display for RequirementsTxtFileError { ) } #[cfg(feature = "http")] - RequirementsTxtParserError::Reqwest(err) => { - write!( - f, - "Error while accessing remote requirements file {}: {err}", - self.file.user_display(), - ) + RequirementsTxtParserError::Reqwest(url, _err) => { + write!(f, "Error while accessing remote requirements file: `{url}`") } #[cfg(feature = "http")] RequirementsTxtParserError::InvalidUrl(url, err) => { @@ -1145,16 +1146,13 @@ impl From for RequirementsTxtParserError { } #[cfg(feature = "http")] -impl From for RequirementsTxtParserError { - fn from(err: reqwest::Error) -> Self { - Self::Reqwest(reqwest_middleware::Error::Reqwest(err)) +impl RequirementsTxtParserError { + fn from_reqwest(url: Url, err: reqwest::Error) -> Self { + Self::Reqwest(url, reqwest_middleware::Error::Reqwest(err)) } -} -#[cfg(feature = "http")] -impl From for RequirementsTxtParserError { - fn from(err: reqwest_middleware::Error) -> Self { - Self::Reqwest(err) + fn from_reqwest_middleware(url: Url, err: reqwest_middleware::Error) -> Self { + Self::Reqwest(url, err) } } diff --git a/crates/uv/tests/it/lock.rs b/crates/uv/tests/it/lock.rs index 4405d22d0418..b1a6aa012f29 100644 --- a/crates/uv/tests/it/lock.rs +++ b/crates/uv/tests/it/lock.rs @@ -6304,6 +6304,7 @@ fn lock_redact_https() -> Result<()> { ----- stderr ----- error: Failed to prepare distributions Caused by: Failed to download `iniconfig==2.0.0` + Caused by: Failed to fetch: `https://pypi-proxy.fly.dev/basic-auth/files/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl` Caused by: HTTP status client error (401 Unauthorized) for url (https://pypi-proxy.fly.dev/basic-auth/files/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl) "###); @@ -6316,6 +6317,7 @@ fn lock_redact_https() -> Result<()> { ----- stderr ----- error: Failed to prepare distributions Caused by: Failed to download `iniconfig==2.0.0` + Caused by: Failed to fetch: `https://pypi-proxy.fly.dev/basic-auth/files/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl` Caused by: HTTP status client error (401 Unauthorized) for url (https://pypi-proxy.fly.dev/basic-auth/files/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl) "###); @@ -6355,6 +6357,7 @@ fn lock_redact_https() -> Result<()> { ----- stderr ----- error: Failed to prepare distributions Caused by: Failed to download `iniconfig==2.0.0` + Caused by: Failed to fetch: `https://pypi-proxy.fly.dev/basic-auth/files/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl` Caused by: HTTP status client error (401 Unauthorized) for url (https://pypi-proxy.fly.dev/basic-auth/files/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl) "###); diff --git a/crates/uv/tests/it/pip_compile.rs b/crates/uv/tests/it/pip_compile.rs index 964542dfe2da..83b112440549 100644 --- a/crates/uv/tests/it/pip_compile.rs +++ b/crates/uv/tests/it/pip_compile.rs @@ -9365,6 +9365,7 @@ fn not_found_direct_url() -> Result<()> { ----- stderr ----- error: Failed to download: `iniconfig @ https://files.pythonhosted.org/packages/ef/a6/fake/iniconfig-2.0.0-py3-none-any.whl` + Caused by: Failed to fetch: `https://files.pythonhosted.org/packages/ef/a6/fake/iniconfig-2.0.0-py3-none-any.whl` Caused by: HTTP status client error (404 Not Found) for url (https://files.pythonhosted.org/packages/ef/a6/fake/iniconfig-2.0.0-py3-none-any.whl) "### );