Retry streaming Python and bin download errors (#15567)
When there is an error during the streaming download and unpack for Python interpreter and bin installs, we would previously fail, causing a lot of CI flakes on GitHub Actions. The problem was that the error is not one of the extended IO errors we were previously handling, but a regular reqwest error, nested below layers of errors of other crates processing the stream, including some IO errors. We now handle nested reqwest errors, too. This surfaced another problem: Our manual retry loop couldn't inform the retry middleware that it already performed the limit of retries, and that the middleware should not retry anymore. While too many retries are more a problem for debugging than for the user, this causes confusing error output. To work around this, we disable the retries in the client and handle all retry errors in our loop. Fixes https://github.com/astral-sh/uv/issues/14171 Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
This commit is contained in:
@@ -12,6 +12,7 @@ use futures::TryStreamExt;
|
||||
use itertools::Itertools;
|
||||
use once_cell::sync::OnceCell;
|
||||
use owo_colors::OwoColorize;
|
||||
use reqwest_retry::policies::ExponentialBackoff;
|
||||
use reqwest_retry::{RetryError, RetryPolicy};
|
||||
use serde::Deserialize;
|
||||
use thiserror::Error;
|
||||
@@ -21,7 +22,7 @@ use tokio_util::either::Either;
|
||||
use tracing::{debug, instrument};
|
||||
use url::Url;
|
||||
|
||||
use uv_client::{BaseClient, WrappedReqwestError, is_extended_transient_error};
|
||||
use uv_client::{BaseClient, WrappedReqwestError, is_transient_network_error};
|
||||
use uv_distribution_filename::{ExtensionError, SourceDistExtension};
|
||||
use uv_extract::hash::Hasher;
|
||||
use uv_fs::{Simplified, rename_with_retry};
|
||||
@@ -930,6 +931,7 @@ impl ManagedPythonDownload {
|
||||
pub async fn fetch_with_retry(
|
||||
&self,
|
||||
client: &BaseClient,
|
||||
retry_policy: &ExponentialBackoff,
|
||||
installation_dir: &Path,
|
||||
scratch_dir: &Path,
|
||||
reinstall: bool,
|
||||
@@ -940,7 +942,6 @@ impl ManagedPythonDownload {
|
||||
let mut total_attempts = 0;
|
||||
let mut retried_here = false;
|
||||
let start_time = SystemTime::now();
|
||||
let retry_policy = client.retry_policy();
|
||||
loop {
|
||||
let result = self
|
||||
.fetch(
|
||||
@@ -961,7 +962,7 @@ impl ManagedPythonDownload {
|
||||
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) {
|
||||
if is_transient_network_error(&err) {
|
||||
let retry_decision = retry_policy.should_retry(start_time, n_past_retries);
|
||||
if let reqwest_retry::RetryDecision::Retry { execute_after } =
|
||||
retry_decision
|
||||
|
||||
@@ -5,10 +5,11 @@ use std::str::FromStr;
|
||||
|
||||
use indexmap::IndexMap;
|
||||
use ref_cast::RefCast;
|
||||
use reqwest_retry::policies::ExponentialBackoff;
|
||||
use tracing::{debug, info};
|
||||
|
||||
use uv_cache::Cache;
|
||||
use uv_client::BaseClientBuilder;
|
||||
use uv_client::{BaseClientBuilder, retries_from_env};
|
||||
use uv_pep440::{Prerelease, Version};
|
||||
use uv_platform::{Arch, Libc, Os, Platform};
|
||||
use uv_preview::Preview;
|
||||
@@ -228,12 +229,17 @@ impl PythonInstallation {
|
||||
let scratch_dir = installations.scratch();
|
||||
let _lock = installations.lock().await?;
|
||||
|
||||
let client = client_builder.build();
|
||||
// Python downloads are performing their own retries to catch stream errors, disable the
|
||||
// default retries to avoid the middleware from performing uncontrolled retries.
|
||||
let retry_policy =
|
||||
ExponentialBackoff::builder().build_with_max_retries(retries_from_env()?);
|
||||
let client = client_builder.clone().retries(0).build();
|
||||
|
||||
info!("Fetching requested Python...");
|
||||
let result = download
|
||||
.fetch_with_retry(
|
||||
&client,
|
||||
&retry_policy,
|
||||
installations_dir,
|
||||
&scratch_dir,
|
||||
false,
|
||||
|
||||
@@ -99,6 +99,9 @@ pub enum Error {
|
||||
|
||||
#[error(transparent)]
|
||||
InvalidEnvironment(#[from] environment::InvalidEnvironment),
|
||||
|
||||
#[error(transparent)]
|
||||
RetryParsing(#[from] uv_client::RetryParsingError),
|
||||
}
|
||||
|
||||
impl Error {
|
||||
|
||||
Reference in New Issue
Block a user