From 59558b13c1bddc1fbb7c3c369e2b7955fa5ddd7a Mon Sep 17 00:00:00 2001 From: konsti Date: Wed, 6 Aug 2025 17:59:17 +0200 Subject: [PATCH] Respect `UV_HTTP_RETRIES` in `uv publish` (#15106) Previously, publish would always use the default retries, now it respects `UV_HTTP_RETRIES` Some awkward error handling to avoid pulling anyhow into uv-publish. --- crates/uv-client/src/base_client.rs | 45 +++++++++++++++++---------- crates/uv-client/src/lib.rs | 3 +- crates/uv-publish/src/lib.rs | 8 +++-- crates/uv/src/commands/project/mod.rs | 5 ++- 4 files changed, 40 insertions(+), 21 deletions(-) diff --git a/crates/uv-client/src/base_client.rs b/crates/uv-client/src/base_client.rs index aab5f5b70..d658406bf 100644 --- a/crates/uv-client/src/base_client.rs +++ b/crates/uv-client/src/base_client.rs @@ -1,12 +1,12 @@ use std::error::Error; use std::fmt::Debug; use std::fmt::Write; +use std::num::ParseIntError; use std::path::Path; use std::sync::Arc; use std::time::Duration; use std::{env, io, iter}; -use anyhow::Context; use anyhow::anyhow; use http::{ HeaderMap, HeaderName, HeaderValue, Method, StatusCode, @@ -22,6 +22,7 @@ use reqwest_retry::policies::ExponentialBackoff; use reqwest_retry::{ DefaultRetryableStrategy, RetryTransientMiddleware, Retryable, RetryableStrategy, }; +use thiserror::Error; use tracing::{debug, trace}; use url::ParseError; use url::Url; @@ -42,7 +43,9 @@ use crate::linehaul::LineHaul; use crate::middleware::OfflineMiddleware; use crate::tls::read_identity; +/// Do not use this value directly outside tests, use [`retries_from_env`] instead. pub const DEFAULT_RETRIES: u32 = 3; + /// Maximum number of redirects to follow before giving up. /// /// This is the default used by [`reqwest`]. @@ -169,23 +172,13 @@ impl<'a> BaseClientBuilder<'a> { self } - /// Read the retry count from [`EnvVars::UV_HTTP_RETRIES`] if set, otherwise, make no change. + /// Read the retry count from [`EnvVars::UV_HTTP_RETRIES`] if set, otherwise use the default + /// retries. /// /// Errors when [`EnvVars::UV_HTTP_RETRIES`] is not a valid u32. - pub fn retries_from_env(self) -> anyhow::Result { - // TODO(zanieb): We should probably parse this in another layer, but there's not a natural - // fit for it right now - if let Some(value) = env::var_os(EnvVars::UV_HTTP_RETRIES) { - Ok(self.retries( - value - .to_string_lossy() - .as_ref() - .parse::() - .context("Failed to parse `UV_HTTP_RETRIES`")?, - )) - } else { - Ok(self) - } + pub fn retries_from_env(mut self) -> Result { + self.retries = retries_from_env()?; + Ok(self) } #[must_use] @@ -982,6 +975,26 @@ fn find_sources(orig: &dyn Error) -> impl Iterator(orig), |&err| find_source(err)) } +// TODO(konsti): Remove once we find a native home for `retries_from_env` +#[derive(Debug, Error)] +pub enum RetryParsingError { + #[error("Failed to parse `UV_HTTP_RETRIES`")] + ParseInt(#[from] ParseIntError), +} + +/// Read the retry count from [`EnvVars::UV_HTTP_RETRIES`] if set, otherwise, make no change. +/// +/// Errors when [`EnvVars::UV_HTTP_RETRIES`] is not a valid u32. +pub fn retries_from_env() -> Result { + // TODO(zanieb): We should probably parse this in another layer, but there's not a natural + // fit for it right now + if let Some(value) = env::var_os(EnvVars::UV_HTTP_RETRIES) { + Ok(value.to_string_lossy().as_ref().parse::()?) + } else { + Ok(DEFAULT_RETRIES) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/uv-client/src/lib.rs b/crates/uv-client/src/lib.rs index e42c86620..f877afb2b 100644 --- a/crates/uv-client/src/lib.rs +++ b/crates/uv-client/src/lib.rs @@ -1,6 +1,7 @@ pub use base_client::{ AuthIntegration, BaseClient, BaseClientBuilder, DEFAULT_RETRIES, ExtraMiddleware, - RedirectClientWithMiddleware, RequestBuilder, UvRetryableStrategy, is_extended_transient_error, + RedirectClientWithMiddleware, RequestBuilder, RetryParsingError, UvRetryableStrategy, + is_extended_transient_error, retries_from_env, }; pub use cached_client::{CacheControl, CachedClient, CachedClientError, DataWithCachePolicy}; pub use error::{Error, ErrorKind, WrappedReqwestError}; diff --git a/crates/uv-publish/src/lib.rs b/crates/uv-publish/src/lib.rs index df97a1c10..9b2b31553 100644 --- a/crates/uv-publish/src/lib.rs +++ b/crates/uv-publish/src/lib.rs @@ -27,8 +27,8 @@ use url::Url; use uv_auth::Credentials; use uv_cache::{Cache, Refresh}; use uv_client::{ - BaseClient, DEFAULT_RETRIES, MetadataFormat, OwnedArchive, RegistryClientBuilder, - RequestBuilder, UvRetryableStrategy, + BaseClient, MetadataFormat, OwnedArchive, RegistryClientBuilder, RequestBuilder, + RetryParsingError, UvRetryableStrategy, retries_from_env, }; use uv_configuration::{KeyringProviderType, TrustedPublishing}; use uv_distribution_filename::{DistFilename, SourceDistExtension, SourceDistFilename}; @@ -78,6 +78,8 @@ pub enum PublishError { }, #[error("Hash is missing in index for {0}")] MissingHash(Box), + #[error(transparent)] + RetryParsing(#[from] RetryParsingError), } /// Failure to get the metadata for a specific file. @@ -397,7 +399,7 @@ pub async fn upload( let mut n_past_retries = 0; let start_time = SystemTime::now(); // N.B. We cannot use the client policy here because it is set to zero retries - let retry_policy = ExponentialBackoff::builder().build_with_max_retries(DEFAULT_RETRIES); + let retry_policy = ExponentialBackoff::builder().build_with_max_retries(retries_from_env()?); loop { let (request, idx) = build_request( file, diff --git a/crates/uv/src/commands/project/mod.rs b/crates/uv/src/commands/project/mod.rs index 73b1f98d2..abe57f37a 100644 --- a/crates/uv/src/commands/project/mod.rs +++ b/crates/uv/src/commands/project/mod.rs @@ -262,6 +262,9 @@ pub(crate) enum ProjectError { #[error(transparent)] Io(#[from] std::io::Error), + #[error(transparent)] + RetryParsing(#[from] uv_client::RetryParsingError), + #[error(transparent)] Anyhow(#[from] anyhow::Error), } @@ -1708,7 +1711,7 @@ pub(crate) async fn resolve_names( let client_builder = BaseClientBuilder::new() .retries_from_env() - .map_err(uv_requirements::Error::ClientError)? + .map_err(|err| uv_requirements::Error::ClientError(err.into()))? .connectivity(network_settings.connectivity) .native_tls(network_settings.native_tls) .keyring(*keyring_provider)