Python
/
uv
mirror of https://github.com/astral-sh/uv
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Refactor uv retrayble strategy to use a single code path 

Refactoring that allows uv's retryable strategy to return Some(Retryable::Fatal), which is also helpful for #16245.

We need to change the way we're reporting retries to avoid both the retry middleware and our own retry context to report the retry numbers.
This commit is contained in:
konstin 2025-12-12 15:36:06 +01:00
parent a2d64aa224
commit 7571ffa385
4 changed files with 72 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

76
crates/uv-client/src/base_client.rs
View File

@ -20,8 +20,8 @@ use reqwest::{Client, ClientBuilder, IntoUrl, Proxy, Request, Response, multipar
use reqwest_middleware::{ClientWithMiddleware, Middleware};
use reqwest_retry::policies::ExponentialBackoff;
use reqwest_retry::{
DefaultRetryableStrategy, RetryTransientMiddleware, Retryable, RetryableStrategy,
default_on_request_error,
RetryTransientMiddleware, Retryable, RetryableStrategy, default_on_request_error,
default_on_request_success,
};
use thiserror::Error;
use tracing::{debug, trace};
@ -1015,21 +1015,15 @@ impl<'a> RequestBuilder<'a> {
}
}
/// Extends [`DefaultRetryableStrategy`], to log transient request failures and additional retry cases.
/// An extension over [`DefaultRetryableStrategy`] that logs transient request failures and
/// adds additional retry cases.
pub struct UvRetryableStrategy;
impl RetryableStrategy for UvRetryableStrategy {
fn handle(&self, res: &Result<Response, reqwest_middleware::Error>) -> Option<Retryable> {
// Use the default strategy and check for additional transient error cases.
let retryable = match DefaultRetryableStrategy.handle(res) {
None | Some(Retryable::Fatal)
if res
.as_ref()
.is_err_and(|err| is_transient_network_error(err)) =>
{
Some(Retryable::Transient)
}
default => default,
let retryable = match res {
Ok(success) => default_on_request_success(success),
Err(err) => retryable_on_request_failure(err),
};
// Log on transient errors
@ -1055,11 +1049,13 @@ impl RetryableStrategy for UvRetryableStrategy {
/// Whether the error looks like a network error that should be retried.
///
/// There are two cases that the default retry strategy is missing:
/// This is an extension over [`reqwest_middleware::default_on_request_failure`], which is missing
/// a number of cases:
/// * Inside the reqwest or reqwest-middleware error is an `io::Error` such as a broken pipe
/// * When streaming a response, a reqwest error may be hidden several layers behind errors
/// of different crates processing the stream, including `io::Error` layers.
pub fn is_transient_network_error(err: &(dyn Error + 'static)) -> bool {
/// of different crates processing the stream, including `io::Error` layers
/// * Any `h2` error
fn retryable_on_request_failure(err: &(dyn Error + 'static)) -> Option<Retryable> {
// First, try to show a nice trace log
if let Some((Some(status), Some(url))) = find_source::<WrappedReqwestError>(&err)
.map(|request_err| (request_err.status(), request_err.url()))
@ -1074,29 +1070,32 @@ pub fn is_transient_network_error(err: &(dyn Error + 'static)) -> bool {
// crates
let mut current_source = Some(err);
while let Some(source) = current_source {
if let Some(reqwest_err) = source.downcast_ref::<WrappedReqwestError>() {
has_known_error = true;
if let reqwest_middleware::Error::Reqwest(reqwest_err) = &**reqwest_err {
if default_on_request_error(reqwest_err) == Some(Retryable::Transient) {
trace!("Retrying nested reqwest middleware error");
return true;
}
if is_retryable_status_error(reqwest_err) {
trace!("Retrying nested reqwest middleware status code error");
return true;
}
}
// Handle different kinds of reqwest error nesting not accessible by downcast.
let reqwest_err = if let Some(reqwest_err) = source.downcast_ref::<reqwest::Error>() {
Some(reqwest_err)
} else if let Some(reqwest_err) = source
.downcast_ref::<WrappedReqwestError>()
.and_then(|err| err.inner())
{
Some(reqwest_err)
} else if let Some(reqwest_middleware::Error::Reqwest(reqwest_err)) =
source.downcast_ref::<reqwest_middleware::Error>()
{
Some(reqwest_err)
} else {
None
};
trace!("Cannot retry nested reqwest middleware error");
} else if let Some(reqwest_err) = source.downcast_ref::<reqwest::Error>() {
if let Some(reqwest_err) = reqwest_err {
has_known_error = true;
// Ignore the default retry strategy returning fatal.
if default_on_request_error(reqwest_err) == Some(Retryable::Transient) {
trace!("Retrying nested reqwest error");
return true;
return Some(Retryable::Transient);
}
if is_retryable_status_error(reqwest_err) {
trace!("Retrying nested reqwest status code error");
return true;
return Some(Retryable::Transient);
}
trace!("Cannot retry nested reqwest error");
@ -1104,7 +1103,7 @@ pub fn is_transient_network_error(err: &(dyn Error + 'static)) -> bool {
// All h2 errors look like errors that should be retried
// https://github.com/astral-sh/uv/issues/15916
trace!("Retrying nested h2 error");
return true;
return Some(Retryable::Transient);
} else if let Some(io_err) = source.downcast_ref::<io::Error>() {
has_known_error = true;
let retryable_io_err_kinds = [
@ -1121,7 +1120,7 @@ pub fn is_transient_network_error(err: &(dyn Error + 'static)) -> bool {
];
if retryable_io_err_kinds.contains(&io_err.kind()) {
trace!("Retrying error: `{}`", io_err.kind());
return true;
return Some(Retryable::Transient);
}
trace!(
@ -1136,7 +1135,12 @@ pub fn is_transient_network_error(err: &(dyn Error + 'static)) -> bool {
if !has_known_error {
trace!("Cannot retry error: Neither an IO error nor a reqwest error");
}
false
None
}
pub fn is_transient_network_error(err: &(dyn Error + 'static)) -> bool {
retryable_on_request_failure(err) == Some(Retryable::Transient)
}
/// Whether the error is a status code error that is retryable.
@ -1397,7 +1401,7 @@ mod tests {
.await;
let middleware_retry =
DefaultRetryableStrategy.handle(&response) == Some(Retryable::Transient);
UvRetryableStrategy.handle(&response) == Some(Retryable::Transient);
let response = client
.get(format!("{}/{}", server.uri(), status))

27
crates/uv-client/src/error.rs
View File

@ -425,13 +425,33 @@ impl WrappedReqwestError {
problem_details: Option<ProblemDetails>,
) -> Self {
Self {
error,
error: Self::filter_retries_from_error(error),
problem_details: problem_details.map(Box::new),
}
}
/// Drop `RetryError::WithRetries` to avoid reporting the number of retries twice.
///
/// We attach the number of errors outside by adding the retry counts from the retry middleware
/// and from uv's outer retry loop for streaming bodies. Stripping the inner count from the
/// error context avoids showing two numbers.
fn filter_retries_from_error(error: reqwest_middleware::Error) -> reqwest_middleware::Error {
match error {
reqwest_middleware::Error::Middleware(error) => {
match error.downcast::<reqwest_retry::RetryError>() {
Ok(
reqwest_retry::RetryError::WithRetries { err, .. }
| reqwest_retry::RetryError::Error(err),
) => err,
Err(error) => reqwest_middleware::Error::Middleware(error),
}
}
error @ reqwest_middleware::Error::Reqwest(_) => error,
}
}
/// Return the inner [`reqwest::Error`] from the error chain, if it exists.
fn inner(&self) -> Option<&reqwest::Error> {
pub fn inner(&self) -> Option<&reqwest::Error> {
match &self.error {
reqwest_middleware::Error::Reqwest(err) => Some(err),
reqwest_middleware::Error::Middleware(err) => err.chain().find_map(|err| {
@ -495,6 +515,7 @@ impl WrappedReqwestError {
impl From<reqwest::Error> for WrappedReqwestError {
fn from(error: reqwest::Error) -> Self {
Self {
// No need to filter retries as this error does not have retries.
error: error.into(),
problem_details: None,
}
@ -504,7 +525,7 @@ impl From<reqwest::Error> for WrappedReqwestError {
impl From<reqwest_middleware::Error> for WrappedReqwestError {
fn from(error: reqwest_middleware::Error) -> Self {
Self {
error,
error: Self::filter_retries_from_error(error),
problem_details: None,
}
}

7
crates/uv-python/src/downloads.rs
View File

@ -131,15 +131,12 @@ impl Error {
if let Self::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 Self::NetworkMiddlewareError(_, anyhow_error) = self {
if let Some(RetryError::WithRetries { retries, .. }) =
if let Self::NetworkMiddlewareError(_, anyhow_error) = self
&& let Some(RetryError::WithRetries { retries, .. }) =
anyhow_error.downcast_ref::<RetryError>()
{
return retries + 1;
}
}
1
}
}

8
crates/uv/tests/it/network.rs
View File

@ -83,8 +83,8 @@ async fn simple_io_err() {
----- stdout -----
----- stderr -----
error: Failed to fetch: `[SERVER]/tqdm/`
Caused by: Request failed after 3 retries
error: Request failed after 3 retries
Caused by: Failed to fetch: `[SERVER]/tqdm/`
Caused by: error sending request for url ([SERVER]/tqdm/)
Caused by: client error (SendRequest)
Caused by: connection closed before message completed
@ -141,8 +141,8 @@ async fn find_links_io_error() {
----- stderr -----
error: Failed to read `--find-links` URL: [SERVER]/
Caused by: Failed to fetch: `[SERVER]/`
Caused by: Request failed after 3 retries
Caused by: Failed to fetch: `[SERVER]/`
Caused by: error sending request for url ([SERVER]/)
Caused by: client error (SendRequest)
Caused by: connection closed before message completed
@ -200,8 +200,8 @@ async fn direct_url_io_error() {
----- stderr -----
× Failed to download `tqdm @ [SERVER]/packages/d0/30/dc54f88dd4a2b5dc8a0279bdd7270e735851848b762aeb1c1184ed1f6b14/tqdm-4.67.1-py3-none-any.whl`
├─▶ Failed to fetch: `[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`
├─▶ error sending request for url ([SERVER]/packages/d0/30/dc54f88dd4a2b5dc8a0279bdd7270e735851848b762aeb1c1184ed1f6b14/tqdm-4.67.1-py3-none-any.whl)
├─▶ client error (SendRequest)
╰─▶ connection closed before message completed