diff --git a/crates/uv-python/src/downloads.rs b/crates/uv-python/src/downloads.rs index 2b0a7e669..ad516d096 100644 --- a/crates/uv-python/src/downloads.rs +++ b/crates/uv-python/src/downloads.rs @@ -12,7 +12,7 @@ use futures::TryStreamExt; use itertools::Itertools; use once_cell::sync::OnceCell; use owo_colors::OwoColorize; -use reqwest_retry::RetryPolicy; +use reqwest_retry::{RetryError, RetryPolicy}; use serde::Deserialize; use thiserror::Error; use tokio::io::{AsyncRead, AsyncWriteExt, BufWriter, ReadBuf}; @@ -111,6 +111,33 @@ pub enum Error { }, } +impl Error { + // Return the number of attempts that were made to complete this request before this error was + // returned. Note that e.g. 3 retries equates to 4 attempts. + // + // It's easier to do arithmetic with "attempts" instead of "retries", because if you have + // nested retry loops you can just add up all the attempts directly, while adding up the + // retries requires +1/-1 adjustments. + fn attempts(&self) -> u32 { + // Unfortunately different variants of `Error` track retry counts in different ways. We + // could consider unifying the variants we handle here in `Error::from_reqwest_middleware` + // instead, but both approaches will be fragile as new variants get added over time. + if let Error::NetworkErrorWithRetries { retries, .. } = self { + return retries + 1; + } + // TODO(jack): let-chains are stable as of Rust 1.88. We should use them here as soon as + // our rust-version is high enough. + if let Error::NetworkMiddlewareError(_, anyhow_error) = self { + if let Some(RetryError::WithRetries { retries, .. }) = + anyhow_error.downcast_ref::() + { + return retries + 1; + } + } + 1 + } +} + #[derive(Debug, PartialEq, Eq, Clone, Hash)] pub struct ManagedPythonDownload { key: PythonInstallationKey, @@ -695,7 +722,8 @@ impl ManagedPythonDownload { pypy_install_mirror: Option<&str>, reporter: Option<&dyn Reporter>, ) -> Result { - let mut n_past_retries = 0; + let mut total_attempts = 0; + let mut retried_here = false; let start_time = SystemTime::now(); let retry_policy = client.retry_policy(); loop { @@ -710,25 +738,41 @@ impl ManagedPythonDownload { reporter, ) .await; - if result - .as_ref() - .err() - .is_some_and(|err| is_extended_transient_error(err)) - { - let retry_decision = retry_policy.should_retry(start_time, n_past_retries); - if let reqwest_retry::RetryDecision::Retry { execute_after } = retry_decision { - debug!( - "Transient failure while handling response for {}; retrying...", - self.key() - ); - let duration = execute_after - .duration_since(SystemTime::now()) - .unwrap_or_else(|_| Duration::default()); - tokio::time::sleep(duration).await; - n_past_retries += 1; - continue; + let result = match result { + Ok(download_result) => Ok(download_result), + Err(err) => { + // Inner retry loops (e.g. `reqwest-retry` middleware) might make more than one + // attempt per error we see here. + total_attempts += err.attempts(); + // We currently interpret e.g. "3 retries" to mean we should make 4 attempts. + let n_past_retries = total_attempts - 1; + if is_extended_transient_error(&err) { + let retry_decision = retry_policy.should_retry(start_time, n_past_retries); + if let reqwest_retry::RetryDecision::Retry { execute_after } = + retry_decision + { + debug!( + "Transient failure while handling response for {}; retrying...", + self.key() + ); + let duration = execute_after + .duration_since(SystemTime::now()) + .unwrap_or_else(|_| Duration::default()); + tokio::time::sleep(duration).await; + retried_here = true; + continue; // Retry. + } + } + if retried_here { + Err(Error::NetworkErrorWithRetries { + err: Box::new(err), + retries: n_past_retries, + }) + } else { + Err(err) + } } - } + }; return result; } }