From 468b19eea62e6835048d059a5b4469399a0065f1 Mon Sep 17 00:00:00 2001 From: konstin Date: Tue, 16 Dec 2025 17:30:10 +0100 Subject: [PATCH] Review --- crates/uv-bin-install/src/lib.rs | 19 ++++++++++++------- crates/uv-client/src/base_client.rs | 16 +++++++++++----- crates/uv-client/src/cached_client.rs | 14 ++++++++++---- crates/uv-publish/src/lib.rs | 7 +++++-- crates/uv-python/src/downloads.rs | 7 +++++-- 5 files changed, 43 insertions(+), 20 deletions(-) diff --git a/crates/uv-bin-install/src/lib.rs b/crates/uv-bin-install/src/lib.rs index d3caff2eb..a289d76a2 100644 --- a/crates/uv-bin-install/src/lib.rs +++ b/crates/uv-bin-install/src/lib.rs @@ -148,13 +148,15 @@ 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. - fn attempts(&self) -> u32 { + /// Return the number of retries that were made to complete this request before this error was + /// returned. + /// + /// Note that e.g. 3 retries equates to 4 attempts. + fn retries(&self) -> u32 { if let Self::RetriedError { retries, .. } = self { - return retries + 1; + return *retries; } - 1 + 0 } } @@ -240,7 +242,7 @@ async fn download_and_unpack_with_retry( download_url: &Url, cache_entry: &CacheEntry, ) -> Result { - let mut retry_state = RetryState::new(*retry_policy, download_url.clone()); + let mut retry_state = RetryState::start(*retry_policy, download_url.clone()); loop { let result = download_and_unpack( @@ -259,7 +261,10 @@ async fn download_and_unpack_with_retry( let result = match result { Ok(path) => Ok(path), Err(err) => { - if retry_state.should_retry(&err, err.attempts()).await { + if retry_state + .handle_retry_and_backoff(&err, err.retries()) + .await + { continue; } diff --git a/crates/uv-client/src/base_client.rs b/crates/uv-client/src/base_client.rs index 556727114..bfba6a47d 100644 --- a/crates/uv-client/src/base_client.rs +++ b/crates/uv-client/src/base_client.rs @@ -1148,7 +1148,8 @@ pub struct RetryState { } impl RetryState { - pub fn new(retry_policy: ExponentialBackoff, url: impl Into) -> Self { + /// Initialize the [`RetryState`] and start the backoff timer. + pub fn start(retry_policy: ExponentialBackoff, url: impl Into) -> Self { Self { retry_policy, start_time: SystemTime::now(), @@ -1164,16 +1165,21 @@ impl RetryState { self.total_retries } - /// Whether request should be retried. Waits with backoff if required. + /// Determines whether request should be retried and waits with backoff if it's retried. /// /// Takes the number of retries from nested layers associated with the specific `err` type as /// `error_retries`. - pub async fn should_retry(&mut self, err: &(dyn Error + 'static), error_retries: u32) -> bool { - // Check if the middleware already performed retries + /// + /// Returns `true` if the request should be retried. + pub async fn handle_retry_and_backoff( + &mut self, + err: &(dyn Error + 'static), + error_retries: u32, + ) -> bool { + // If the middleware performed any retries, consider them in our budget. self.total_retries += error_retries; match retryable_on_request_failure(err) { Some(Retryable::Transient) => { - // If middleware already retried, consider that in our retry budget let retry_decision = self .retry_policy .should_retry(self.start_time, self.total_retries); diff --git a/crates/uv-client/src/cached_client.rs b/crates/uv-client/src/cached_client.rs index 4eb51e801..34a22a4a9 100644 --- a/crates/uv-client/src/cached_client.rs +++ b/crates/uv-client/src/cached_client.rs @@ -690,7 +690,7 @@ impl CachedClient { cache_control: CacheControl<'_>, response_callback: Callback, ) -> Result> { - let mut retry_state = RetryState::new(self.uncached().retry_policy(), req.url().clone()); + let mut retry_state = RetryState::start(self.uncached().retry_policy(), req.url().clone()); loop { let fresh_req = req.try_clone().expect("HTTP request must be cloneable"); let result = self @@ -700,7 +700,10 @@ impl CachedClient { match result { Ok(ok) => return Ok(ok), Err(err) => { - if retry_state.should_retry(err.error(), err.retries()).await { + if retry_state + .handle_retry_and_backoff(err.error(), err.retries()) + .await + { continue; } return Err(err.with_retries(retry_state.total_retries())); @@ -723,7 +726,7 @@ impl CachedClient { cache_control: CacheControl<'_>, response_callback: Callback, ) -> Result> { - let mut retry_state = RetryState::new(self.uncached().retry_policy(), req.url().clone()); + let mut retry_state = RetryState::start(self.uncached().retry_policy(), req.url().clone()); loop { let fresh_req = req.try_clone().expect("HTTP request must be cloneable"); let result = self @@ -733,7 +736,10 @@ impl CachedClient { match result { Ok(ok) => return Ok(ok), Err(err) => { - if retry_state.should_retry(err.error(), err.retries()).await { + if retry_state + .handle_retry_and_backoff(err.error(), err.retries()) + .await + { continue; } return Err(err.with_retries(retry_state.total_retries())); diff --git a/crates/uv-publish/src/lib.rs b/crates/uv-publish/src/lib.rs index 5980af37b..73bf165c1 100644 --- a/crates/uv-publish/src/lib.rs +++ b/crates/uv-publish/src/lib.rs @@ -468,7 +468,7 @@ pub async fn upload( download_concurrency: &Semaphore, reporter: Arc, ) -> Result { - let mut retry_state = RetryState::new(retry_policy, registry.clone()); + let mut retry_state = RetryState::start(retry_policy, registry.clone()); loop { let (request, idx) = build_upload_request( @@ -496,7 +496,10 @@ pub async fn upload( } else { 0 }; - if retry_state.should_retry(&err, middleware_retries).await { + if retry_state + .handle_retry_and_backoff(&err, middleware_retries) + .await + { continue; } return Err(PublishError::PublishSend( diff --git a/crates/uv-python/src/downloads.rs b/crates/uv-python/src/downloads.rs index 2e9edf964..33f059625 100644 --- a/crates/uv-python/src/downloads.rs +++ b/crates/uv-python/src/downloads.rs @@ -1105,7 +1105,7 @@ impl ManagedPythonDownload { pypy_install_mirror: Option<&str>, reporter: Option<&dyn Reporter>, ) -> Result { - let mut retry_state = RetryState::new( + let mut retry_state = RetryState::start( *retry_policy, self.download_url(python_install_mirror, pypy_install_mirror)?, ); @@ -1125,7 +1125,10 @@ impl ManagedPythonDownload { match result { Ok(download_result) => return Ok(download_result), Err(err) => { - if retry_state.should_retry(&err, err.retries()).await { + if retry_state + .handle_retry_and_backoff(&err, err.retries()) + .await + { continue; } return if retry_state.total_retries() > 0 {