From 4950dd475bb861d916114787efa0f44e93bcd521 Mon Sep 17 00:00:00 2001 From: konsti Date: Tue, 6 Jan 2026 10:20:02 +0100 Subject: [PATCH] Remove retries duplication uv_client error (#17325) Previously, we had a retry count both on the top level error type, and on an error variant, and had a conversion step in between. When reviewing #17274, I noticed we can simplify that. --- crates/uv-client/src/cached_client.rs | 4 +- crates/uv-client/src/error.rs | 67 +++++++++++---------------- 2 files changed, 30 insertions(+), 41 deletions(-) diff --git a/crates/uv-client/src/cached_client.rs b/crates/uv-client/src/cached_client.rs index 640567dab..05f1f91b2 100644 --- a/crates/uv-client/src/cached_client.rs +++ b/crates/uv-client/src/cached_client.rs @@ -540,7 +540,7 @@ impl CachedClient { .execute(req) .instrument(info_span!("revalidation_request", url = url.as_str())) .await - .map_err(|err| ErrorKind::from_reqwest_middleware(url.clone(), err))?; + .map_err(|err| Error::from_reqwest_middleware(url.clone(), err))?; // Check for HTTP error status and extract problem details if available if let Err(status_error) = response.error_for_status_ref() { @@ -610,7 +610,7 @@ impl CachedClient { .0 .execute(req) .await - .map_err(|err| ErrorKind::from_reqwest_middleware(url.clone(), err))?; + .map_err(|err| Error::from_reqwest_middleware(url.clone(), err))?; // If the user set a custom `Cache-Control` header, override it. if let CacheControl::Override(header) = cache_control { diff --git a/crates/uv-client/src/error.rs b/crates/uv-client/src/error.rs index beee98501..e97fd5126 100644 --- a/crates/uv-client/src/error.rs +++ b/crates/uv-client/src/error.rs @@ -143,6 +143,31 @@ impl Error { ErrorKind::BadMessagePack { source: err, url }.into() } + /// Create an [`Error`] from a [`reqwest_middleware::Error`]. + pub(crate) fn from_reqwest_middleware( + url: DisplaySafeUrl, + err: reqwest_middleware::Error, + ) -> Self { + if let reqwest_middleware::Error::Middleware(ref underlying) = err { + if let Some(offline_err) = underlying.downcast_ref::() { + return ErrorKind::Offline(offline_err.url().to_string()).into(); + } + if let Some(reqwest_retry::RetryError::WithRetries { retries, .. }) = + underlying.downcast_ref::() + { + let retries = *retries; + return Self::new( + ErrorKind::WrappedReqwestError(url, WrappedReqwestError::from(err)), + retries, + ); + } + } + Self::from(ErrorKind::WrappedReqwestError( + url, + WrappedReqwestError::from(err), + )) + } + /// Returns `true` if this error corresponds to an offline error. pub(crate) fn is_offline(&self) -> bool { matches!(&*self.kind, ErrorKind::Offline(_)) @@ -246,15 +271,9 @@ impl Error { impl From for Error { fn from(kind: ErrorKind) -> Self { - match kind { - ErrorKind::RequestWithRetries { source, retries } => Self { - kind: source, - retries, - }, - other => Self { - kind: Box::new(other), - retries: 0, - }, + Self { + kind: Box::new(kind), + retries: 0, } } } @@ -306,10 +325,6 @@ pub enum ErrorKind { #[error("Failed to fetch: `{0}`")] WrappedReqwestError(DisplaySafeUrl, #[source] WrappedReqwestError), - /// Add the number of failed retries to the error. - #[error("Request failed after {retries} {subject}", subject = if *retries > 1 { "retries" } else { "retry" })] - RequestWithRetries { source: Box, retries: u32 }, - #[error("Received some unexpected JSON from {}", url)] BadJson { source: serde_json::Error, @@ -388,32 +403,6 @@ impl ErrorKind { Self::WrappedReqwestError(url, WrappedReqwestError::from(error)) } - /// Create an [`ErrorKind`] from a [`reqwest_middleware::Error`]. - pub(crate) fn from_reqwest_middleware( - url: DisplaySafeUrl, - 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()); - } - if let Some(reqwest_retry::RetryError::WithRetries { retries, .. }) = - underlying.downcast_ref::() - { - let retries = *retries; - return Self::RequestWithRetries { - source: Box::new(Self::WrappedReqwestError( - url, - WrappedReqwestError::from(err), - )), - retries, - }; - } - } - - Self::WrappedReqwestError(url, WrappedReqwestError::from(err)) - } - /// Create an [`ErrorKind`] from a [`reqwest::Error`] with problem details. pub(crate) fn from_reqwest_with_problem_details( url: DisplaySafeUrl,