Show retries for HTTP status code errors (#13897)

Using a companion change in the middleware
(https://github.com/TrueLayer/reqwest-middleware/pull/235, forked&tagged
pending review), we can check and show retries for HTTP status core
errors, to consistently report retries again.

We fix two cases:
* Show retries for status code errors for cache client requests
* Show retries for status code errors for Python download requests

Not handled:
* Show previous retries when a distribution download fails mid-streaming
* Perform retries when a distribution download fails mid-streaming
* Show previous retries when a Python download fails mid-streaming
* Perform retries when a Python download fails mid-streaming

crates/uv-client/src/cached_client.rs

@@ -1,4 +1,3 @@
-use std::fmt::{Debug, Display, Formatter};
 use std::time::{Duration, SystemTime};
 use std::{borrow::Cow, path::Path};
 
@@ -100,44 +99,62 @@ where
     }
 }
 
-/// Either a cached client error or a (user specified) error from the callback
+/// Dispatch type: Either a cached client error or a (user specified) error from the callback
 pub enum CachedClientError<CallbackError: std::error::Error + 'static> {
-    Client(Error),
-    Callback(CallbackError),
+    Client {
+        retries: Option<u32>,
+        err: Error,
+    },
+    Callback {
+        retries: Option<u32>,
+        err: CallbackError,
+    },
 }
 
-impl<CallbackError: std::error::Error + 'static> Display for CachedClientError<CallbackError> {
-    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+impl<CallbackError: std::error::Error + 'static> CachedClientError<CallbackError> {
+    /// Attach the number of retries to the error context.
+    ///
+    /// Adds to existing errors if any, in case different layers retried.
+    fn with_retries(self, retries: u32) -> Self {
         match self {
-            CachedClientError::Client(err) => write!(f, "{err}"),
-            CachedClientError::Callback(err) => write!(f, "{err}"),
+            CachedClientError::Client {
+                retries: existing_retries,
+                err,
+            } => CachedClientError::Client {
+                retries: Some(existing_retries.unwrap_or_default() + retries),
+                err,
+            },
+            CachedClientError::Callback {
+                retries: existing_retries,
+                err,
+            } => CachedClientError::Callback {
+                retries: Some(existing_retries.unwrap_or_default() + retries),
+                err,
+            },
         }
     }
-}
 
-impl<CallbackError: std::error::Error + 'static> Debug for CachedClientError<CallbackError> {
-    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+    fn retries(&self) -> Option<u32> {
         match self {
-            CachedClientError::Client(err) => write!(f, "{err:?}"),
-            CachedClientError::Callback(err) => write!(f, "{err:?}"),
+            CachedClientError::Client { retries, .. } => *retries,
+            CachedClientError::Callback { retries, .. } => *retries,
         }
     }
-}
 
-impl<CallbackError: std::error::Error + 'static> std::error::Error
-    for CachedClientError<CallbackError>
-{
-    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+    fn error(&self) -> &dyn std::error::Error {
         match self {
-            CachedClientError::Client(err) => Some(err),
-            CachedClientError::Callback(err) => Some(err),
+            CachedClientError::Client { err, .. } => err,
+            CachedClientError::Callback { err, .. } => err,
         }
     }
 }
 
 impl<CallbackError: std::error::Error + 'static> From<Error> for CachedClientError<CallbackError> {
     fn from(error: Error) -> Self {
-        Self::Client(error)
+        Self::Client {
+            retries: None,
+            err: error,
+        }
     }
 }
 
@@ -145,15 +162,35 @@ impl<CallbackError: std::error::Error + 'static> From<ErrorKind>
     for CachedClientError<CallbackError>
 {
     fn from(error: ErrorKind) -> Self {
-        Self::Client(error.into())
+        Self::Client {
+            retries: None,
+            err: error.into(),
+        }
     }
 }
 
 impl<E: Into<Self> + std::error::Error + 'static> From<CachedClientError<E>> for Error {
+    /// Attach retry error context, if there were retries.
     fn from(error: CachedClientError<E>) -> Self {
         match error {
-            CachedClientError::Client(error) => error,
-            CachedClientError::Callback(error) => error.into(),
+            CachedClientError::Client {
+                retries: Some(retries),
+                err,
+            } => ErrorKind::RequestWithRetries {
+                source: Box::new(err.into_kind()),
+                retries,
+            }
+            .into(),
+            CachedClientError::Client { retries: None, err } => err,
+            CachedClientError::Callback {
+                retries: Some(retries),
+                err,
+            } => ErrorKind::RequestWithRetries {
+                source: Box::new(err.into().into_kind()),
+                retries,
+            }
+            .into(),
+            CachedClientError::Callback { retries: None, err } => err.into(),
         }
     }
 }
@@ -385,7 +422,7 @@ impl CachedClient {
         let data = response_callback(response)
             .boxed_local()
             .await
-            .map_err(|err| CachedClientError::Callback(err))?;
+            .map_err(|err| CachedClientError::Callback { retries: None, err })?;
         let Some(cache_policy) = cache_policy else {
             return Ok(data.into_target());
         };
@@ -530,9 +567,21 @@ impl CachedClient {
             .for_host(&url)
             .execute(req)
             .await
-            .map_err(|err| ErrorKind::from_reqwest_middleware(url.clone(), err))?
-            .error_for_status()
-            .map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?;
+            .map_err(|err| ErrorKind::from_reqwest_middleware(url.clone(), err))?;
+
+        let retry_count = response
+            .extensions()
+            .get::<reqwest_retry::RetryCount>()
+            .map(|retries| retries.value());
+
+        if let Err(status_error) = response.error_for_status_ref() {
+            return Err(CachedClientError::<Error>::Client {
+                retries: retry_count,
+                err: ErrorKind::from_reqwest(url, status_error).into(),
+            }
+            .into());
+        }
+
         let cache_policy = cache_policy_builder.build(&response);
         let cache_policy = if cache_policy.to_archived().is_storable() {
             Some(Box::new(cache_policy))
@@ -579,7 +628,7 @@ impl CachedClient {
         cache_control: CacheControl,
         response_callback: Callback,
     ) -> Result<Payload::Target, CachedClientError<CallBackError>> {
-        let mut n_past_retries = 0;
+        let mut past_retries = 0;
         let start_time = SystemTime::now();
         let retry_policy = self.uncached().retry_policy();
         loop {
@@ -587,11 +636,20 @@ impl CachedClient {
             let result = self
                 .get_cacheable(fresh_req, cache_entry, cache_control, &response_callback)
                 .await;
+
+            // Check if the middleware already performed retries
+            let middleware_retries = match &result {
+                Err(err) => err.retries().unwrap_or_default(),
+                Ok(_) => 0,
+            };
+
             if result
                 .as_ref()
-                .is_err_and(|err| is_extended_transient_error(err))
+                .is_err_and(|err| is_extended_transient_error(err.error()))
             {
-                let retry_decision = retry_policy.should_retry(start_time, n_past_retries);
+                // If middleware already retried, consider that in our retry budget
+                let total_retries = past_retries + middleware_retries;
+                let retry_decision = retry_policy.should_retry(start_time, total_retries);
                 if let reqwest_retry::RetryDecision::Retry { execute_after } = retry_decision {
                     debug!(
                         "Transient failure while handling response from {}; retrying...",
@@ -601,10 +659,15 @@ impl CachedClient {
                         .duration_since(SystemTime::now())
                         .unwrap_or_else(|_| Duration::default());
                     tokio::time::sleep(duration).await;
-                    n_past_retries += 1;
+                    past_retries += 1;
                     continue;
                 }
             }
+
+            if past_retries > 0 {
+                return result.map_err(|err| err.with_retries(past_retries));
+            }
+
             return result;
         }
     }
@@ -622,7 +685,7 @@ impl CachedClient {
         cache_entry: &CacheEntry,
         response_callback: Callback,
     ) -> Result<Payload, CachedClientError<CallBackError>> {
-        let mut n_past_retries = 0;
+        let mut past_retries = 0;
         let start_time = SystemTime::now();
         let retry_policy = self.uncached().retry_policy();
         loop {
@@ -630,12 +693,20 @@ impl CachedClient {
             let result = self
                 .skip_cache(fresh_req, cache_entry, &response_callback)
                 .await;
+
+            // Check if the middleware already performed retries
+            let middleware_retries = match &result {
+                Err(err) => err.retries().unwrap_or_default(),
+                _ => 0,
+            };
+
             if result
                 .as_ref()
                 .err()
-                .is_some_and(|err| is_extended_transient_error(err))
+                .is_some_and(|err| is_extended_transient_error(err.error()))
             {
-                let retry_decision = retry_policy.should_retry(start_time, n_past_retries);
+                let total_retries = past_retries + middleware_retries;
+                let retry_decision = retry_policy.should_retry(start_time, total_retries);
                 if let reqwest_retry::RetryDecision::Retry { execute_after } = retry_decision {
                     debug!(
                         "Transient failure while handling response from {}; retrying...",
@@ -645,10 +716,15 @@ impl CachedClient {
                         .duration_since(SystemTime::now())
                         .unwrap_or_else(|_| Duration::default());
                     tokio::time::sleep(duration).await;
-                    n_past_retries += 1;
+                    past_retries += 1;
                     continue;
                 }
             }
+
+            if past_retries > 0 {
+                return result.map_err(|err| err.with_retries(past_retries));
+            }
+
             return result;
         }
     }

crates/uv-client/src/error.rs

@@ -197,6 +197,13 @@ 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} retries")]
+    RequestWithRetries {
+        source: Box<ErrorKind>,
+        retries: u32,
+    },
+
     #[error("Received some unexpected JSON from {}", url)]
     BadJson {
         source: serde_json::Error,

crates/uv-client/src/flat_index.rs

@@ -246,7 +246,7 @@ impl<'a> FlatIndexClient<'a> {
                     .collect();
                 Ok(FlatIndexEntries::from_entries(files))
             }
-            Err(CachedClientError::Client(err)) if err.is_offline() => {
+            Err(CachedClientError::Client { err, .. }) if err.is_offline() => {
                 Ok(FlatIndexEntries::offline())
             }
             Err(err) => Err(err.into()),

crates/uv-client/src/registry_client.rs

@@ -41,10 +41,7 @@ use crate::flat_index::FlatIndexEntry;
 use crate::html::SimpleHtml;
 use crate::remote_metadata::wheel_metadata_from_remote_zip;
 use crate::rkyvutil::OwnedArchive;
-use crate::{
-    BaseClient, CachedClient, CachedClientError, Error, ErrorKind, FlatIndexClient,
-    FlatIndexEntries,
-};
+use crate::{BaseClient, CachedClient, Error, ErrorKind, FlatIndexClient, FlatIndexEntries};
 
 /// A builder for an [`RegistryClient`].
 #[derive(Debug, Clone)]
@@ -607,18 +604,16 @@ impl RegistryClient {
             .boxed_local()
             .instrument(info_span!("parse_simple_api", package = %package_name))
         };
-        self.cached_client()
+        let simple = self
+            .cached_client()
             .get_cacheable_with_retry(
                 simple_request,
                 cache_entry,
                 cache_control,
                 parse_simple_response,
             )
-            .await
-            .map_err(|err| match err {
-                CachedClientError::Client(err) => err,
-                CachedClientError::Callback(err) => err,
-            })
+            .await?;
+        Ok(simple)
     }
 
     /// Fetch the [`SimpleMetadata`] from a local file, using a PEP 503-compatible directory
@@ -900,15 +895,13 @@ impl RegistryClient {
                     .map_err(|err| ErrorKind::AsyncHttpRangeReader(url.clone(), err))?;
                     trace!("Getting metadata for {filename} by range request");
                     let text = wheel_metadata_from_remote_zip(filename, url, &mut reader).await?;
-                    let metadata =
-                        ResolutionMetadata::parse_metadata(text.as_bytes()).map_err(|err| {
-                            Error::from(ErrorKind::MetadataParseError(
-                                filename.clone(),
-                                url.to_string(),
-                                Box::new(err),
-                            ))
-                        })?;
-                    Ok::<ResolutionMetadata, CachedClientError<Error>>(metadata)
+                    ResolutionMetadata::parse_metadata(text.as_bytes()).map_err(|err| {
+                        Error::from(ErrorKind::MetadataParseError(
+                            filename.clone(),
+                            url.to_string(),
+                            Box::new(err),
+                        ))
+                    })
                 }
                 .boxed_local()
                 .instrument(info_span!("read_metadata_range_request", wheel = %filename))

crates/uv-distribution/src/distribution_database.rs

@@ -644,8 +644,8 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> {
             })
             .await
             .map_err(|err| match err {
-                CachedClientError::Callback(err) => err,
-                CachedClientError::Client(err) => Error::Client(err),
+                CachedClientError::Callback { err, .. } => err,
+                CachedClientError::Client { err, .. } => Error::Client(err),
             })?;
 
         // If the archive is missing the required hashes, or has since been removed, force a refresh.
@@ -663,8 +663,8 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> {
                         .skip_cache_with_retry(self.request(url)?, &http_entry, download)
                         .await
                         .map_err(|err| match err {
-                            CachedClientError::Callback(err) => err,
-                            CachedClientError::Client(err) => Error::Client(err),
+                            CachedClientError::Callback { err, .. } => err,
+                            CachedClientError::Client { err, .. } => Error::Client(err),
                         })
                 })
                 .await?
@@ -811,8 +811,8 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> {
             })
             .await
             .map_err(|err| match err {
-                CachedClientError::Callback(err) => err,
-                CachedClientError::Client(err) => Error::Client(err),
+                CachedClientError::Callback { err, .. } => err,
+                CachedClientError::Client { err, .. } => Error::Client(err),
             })?;
 
         // If the archive is missing the required hashes, or has since been removed, force a refresh.
@@ -830,8 +830,8 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> {
                         .skip_cache_with_retry(self.request(url)?, &http_entry, download)
                         .await
                         .map_err(|err| match err {
-                            CachedClientError::Callback(err) => err,
-                            CachedClientError::Client(err) => Error::Client(err),
+                            CachedClientError::Callback { err, .. } => err,
+                            CachedClientError::Client { err, .. } => Error::Client(err),
                         })
                 })
                 .await?

crates/uv-distribution/src/source/mod.rs

@@ -728,8 +728,8 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
             })
             .await
             .map_err(|err| match err {
-                CachedClientError::Callback(err) => err,
-                CachedClientError::Client(err) => Error::Client(err),
+                CachedClientError::Callback { err, .. } => err,
+                CachedClientError::Client { err, .. } => Error::Client(err),
             })?;
 
         // If the archive is missing the required hashes, force a refresh.
@@ -747,8 +747,8 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
                         )
                         .await
                         .map_err(|err| match err {
-                            CachedClientError::Callback(err) => err,
-                            CachedClientError::Client(err) => Error::Client(err),
+                            CachedClientError::Callback { err, .. } => err,
+                            CachedClientError::Client { err, .. } => Error::Client(err),
                         })
                 })
                 .await
@@ -2084,8 +2084,8 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
                     )
                     .await
                     .map_err(|err| match err {
-                        CachedClientError::Callback(err) => err,
-                        CachedClientError::Client(err) => Error::Client(err),
+                        CachedClientError::Callback { err, .. } => err,
+                        CachedClientError::Client { err, .. } => Error::Client(err),
                     })
             })
             .await

crates/uv-python/src/downloads.rs

@@ -53,6 +53,12 @@ pub enum Error {
     TooManyParts(String),
     #[error("Failed to download {0}")]
     NetworkError(DisplaySafeUrl, #[source] WrappedReqwestError),
+    #[error("Request failed after {retries} retries")]
+    NetworkErrorWithRetries {
+        #[source]
+        err: Box<Error>,
+        retries: u32,
+    },
     #[error("Failed to download {0}")]
     NetworkMiddlewareError(DisplaySafeUrl, #[source] anyhow::Error),
     #[error("Failed to extract archive: {0}")]
@@ -1143,8 +1149,20 @@ fn parse_json_downloads(
 }
 
 impl Error {
-    pub(crate) fn from_reqwest(url: DisplaySafeUrl, err: reqwest::Error) -> Self {
-        Self::NetworkError(url, WrappedReqwestError::from(err))
+    pub(crate) fn from_reqwest(
+        url: DisplaySafeUrl,
+        err: reqwest::Error,
+        retries: Option<u32>,
+    ) -> Self {
+        let err = Self::NetworkError(url, WrappedReqwestError::from(err));
+        if let Some(retries) = retries {
+            Self::NetworkErrorWithRetries {
+                err: Box::new(err),
+                retries,
+            }
+        } else {
+            err
+        }
     }
 
     pub(crate) fn from_reqwest_middleware(
@@ -1260,10 +1278,15 @@ async fn read_url(
             .await
             .map_err(|err| Error::from_reqwest_middleware(url.clone(), err))?;
 
-        // Ensure the request was successful.
-        response
-            .error_for_status_ref()
-            .map_err(|err| Error::from_reqwest(url, err))?;
+        let retry_count = response
+            .extensions()
+            .get::<reqwest_retry::RetryCount>()
+            .map(|retries| retries.value());
+
+        // Check the status code.
+        let response = response
+            .error_for_status()
+            .map_err(|err| Error::from_reqwest(url, err, retry_count))?;
 
         let size = response.content_length();
         let stream = response

crates/uv/tests/it/edit.rs

@@ -11477,7 +11477,8 @@ async fn add_unexpected_error_code() -> Result<()> {
     ----- stdout -----
 
     ----- stderr -----
-    error: Failed to fetch: `http://[LOCALHOST]/anyio/`
+    error: Request failed after 3 retries
+      Caused by: Failed to fetch: `http://[LOCALHOST]/anyio/`
       Caused by: HTTP status server error (503 Service Unavailable) for url (http://[LOCALHOST]/anyio/)
     "
     );

crates/uv/tests/it/network.rs

@@ -54,7 +54,8 @@ async fn simple_http_500() {
     ----- stdout -----
 
     ----- stderr -----
-    error: Failed to fetch: `[SERVER]/tqdm/`
+    error: Request failed after 3 retries
+      Caused by: Failed to fetch: `[SERVER]/tqdm/`
       Caused by: HTTP status server error (500 Internal Server Error) for url ([SERVER]/tqdm/)
     ");
 }
@@ -105,6 +106,7 @@ async fn find_links_http_500() {
 
     ----- stderr -----
     error: Failed to read `--find-links` URL: [SERVER]/
+      Caused by: Request failed after 3 retries
       Caused by: Failed to fetch: `[SERVER]/`
       Caused by: HTTP status server error (500 Internal Server Error) for url ([SERVER]/)
     ");
@@ -159,6 +161,7 @@ async fn direct_url_http_500() {
 
     ----- stderr -----
       × Failed to download `tqdm @ [SERVER]/packages/d0/30/dc54f88dd4a2b5dc8a0279bdd7270e735851848b762aeb1c1184ed1f6b14/tqdm-4.67.1-py3-none-any.whl`
+      ├─▶ Request failed after 3 retries
       ├─▶ Failed to fetch: `[SERVER]/packages/d0/30/dc54f88dd4a2b5dc8a0279bdd7270e735851848b762aeb1c1184ed1f6b14/tqdm-4.67.1-py3-none-any.whl`
       ╰─▶ HTTP status server error (500 Internal Server Error) for url ([SERVER]/packages/d0/30/dc54f88dd4a2b5dc8a0279bdd7270e735851848b762aeb1c1184ed1f6b14/tqdm-4.67.1-py3-none-any.whl)
     ");
@@ -243,6 +246,7 @@ async fn python_install_http_500() {
 
     ----- stderr -----
     error: Failed to install cpython-3.10.0-macos-aarch64-none
+      Caused by: Request failed after 3 retries
       Caused by: Failed to download [SERVER]/astral-sh/python-build-standalone/releases/download/20211017/cpython-3.10.0-aarch64-apple-darwin-pgo%2Blto-20211017T1616.tar.zst
       Caused by: HTTP status server error (500 Internal Server Error) for url ([SERVER]/astral-sh/python-build-standalone/releases/download/20211017/cpython-3.10.0-aarch64-apple-darwin-pgo%2Blto-20211017T1616.tar.zst)
     ");