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

Remove retry wrapper when matching on error kind (#14996) 

## Summary

We often match on `ErrorKind` to figure out how to handle an error
(e.g., to treat a 404 as "Not found" rather than aborting the program).
Unfortunately, if we retry, we wrap the error in a new kind that
includes the retry count. This PR adds an unwrapping mechanism to ensure
that callers always look at the underlying error.

Closes https://github.com/astral-sh/uv/issues/14941.

Closes https://github.com/astral-sh/uv/issues/14989.
This commit is contained in:
Charlie Marsh 2025-07-31 17:00:01 -04:00 committed by GitHub
parent 56677c540a
commit 785595bd35
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 77 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
crates/uv-client/src/cached_client.rs
View File

@ -176,20 +176,12 @@ impl<E: Into<Self> + std::error::Error + 'static> From<CachedClientError<E>> for
CachedClientError::Client {
retries: Some(retries),
err,
} => ErrorKind::RequestWithRetries {
source: Box::new(err.into_kind()),
retries,
}
.into(),
} => Error::new(err.into_kind(), retries),
CachedClientError::Client { retries: None, err } => err,
CachedClientError::Callback {
retries: Some(retries),
err,
} => ErrorKind::RequestWithRetries {
source: Box::new(err.into().into_kind()),
retries,
}
.into(),
} => Error::new(err.into().into_kind(), retries),
CachedClientError::Callback { retries: None, err } => err.into(),
}
}

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

@ -11,19 +11,56 @@ use uv_redacted::DisplaySafeUrl;
use crate::middleware::OfflineError;
use crate::{FlatIndexError, html};
#[derive(Debug, thiserror::Error)]
#[error(transparent)]
#[derive(Debug)]
pub struct Error {
kind: Box<ErrorKind>,
retries: u32,
}
impl Display for Error {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
if self.retries > 0 {
write!(
f,
"Request failed after {retries} retries",
retries = self.retries
)
} else {
Display::fmt(&self.kind, f)
}
}
}
impl std::error::Error for Error {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
if self.retries > 0 {
Some(&self.kind)
} else {
self.kind.source()
}
}
}
impl Error {
/// Convert this error into its [`ErrorKind`] variant.
/// Create a new [`Error`] with the given [`ErrorKind`] and number of retries.
pub fn new(kind: ErrorKind, retries: u32) -> Self {
Self {
kind: Box::new(kind),
retries,
}
}
/// Return the number of retries that were attempted before this error was returned.
pub fn retries(&self) -> u32 {
self.retries
}
/// Convert this error into an [`ErrorKind`].
pub fn into_kind(self) -> ErrorKind {
*self.kind
}
/// Get a reference to the [`ErrorKind`] variant of this error.
/// Return the [`ErrorKind`] of this error.
pub fn kind(&self) -> &ErrorKind {
&self.kind
}
@ -78,7 +115,7 @@ impl Error {
// The server returned a "Method Not Allowed" error, indicating it doesn't support
// HEAD requests, so we can't check for range requests.
ErrorKind::WrappedReqwestError(_url, err) => {
ErrorKind::WrappedReqwestError(_, err) => {
if let Some(status) = err.status() {
// If the server doesn't support HEAD requests, we can't check for range
// requests.
@ -143,6 +180,7 @@ impl From<ErrorKind> for Error {
fn from(kind: ErrorKind) -> Self {
Self {
kind: Box::new(kind),
retries: 0,
}
}
}
@ -265,10 +303,12 @@ pub enum ErrorKind {
}
impl ErrorKind {
/// Create an [`ErrorKind`] from a [`reqwest::Error`].
pub(crate) fn from_reqwest(url: DisplaySafeUrl, error: reqwest::Error) -> Self {
Self::WrappedReqwestError(url, WrappedReqwestError::from(error))
}
/// Create an [`ErrorKind`] from a [`reqwest_middleware::Error`].
pub(crate) fn from_reqwest_middleware(
url: DisplaySafeUrl,
err: reqwest_middleware::Error,

12
crates/uv-client/src/registry_client.rs
View File

@ -549,11 +549,11 @@ impl RegistryClient {
match result {
Ok(metadata) => Ok(SimpleMetadataSearchOutcome::Found(metadata)),
Err(err) => match err.into_kind() {
Err(err) => match err.kind() {
// The package could not be found in the remote index.
ErrorKind::WrappedReqwestError(url, err) => {
let Some(status_code) = err.status() else {
return Err(ErrorKind::WrappedReqwestError(url, err).into());
ErrorKind::WrappedReqwestError(.., reqwest_err) => {
let Some(status_code) = reqwest_err.status() else {
return Err(err);
};
let decision =
status_code_strategy.handle_status_code(status_code, index, capabilities);
@ -562,7 +562,7 @@ impl RegistryClient {
status_code,
StatusCode::UNAUTHORIZED | StatusCode::FORBIDDEN
) {
return Err(ErrorKind::WrappedReqwestError(url, err).into());
return Err(err);
}
}
Ok(SimpleMetadataSearchOutcome::from(decision))
@ -574,7 +574,7 @@ impl RegistryClient {
// The package could not be found in the local index.
ErrorKind::FileNotFound(_) => Ok(SimpleMetadataSearchOutcome::NotFound),
err => Err(err.into()),
_ => Err(err),
},
}
}

4
crates/uv-publish/src/lib.rs
View File

@ -501,7 +501,7 @@ pub async fn check_url(
{
Ok(response) => response,
Err(err) => {
return match err.into_kind() {
return match err.kind() {
uv_client::ErrorKind::PackageNotFound(_) => {
// The package doesn't exist, so we can't have uploaded it.
warn!(
@ -509,7 +509,7 @@ pub async fn check_url(
);
Ok(false)
}
kind => Err(PublishError::CheckUrlIndex(kind.into())),
_ => Err(PublishError::CheckUrlIndex(err)),
};
}
};

35
crates/uv-resolver/src/resolver/provider.rs
View File

@ -199,7 +199,7 @@ impl<Context: BuildContext> ResolverProvider for DefaultResolverProvider<'_, Con
})
.collect(),
)),
Err(err) => match err.into_kind() {
Err(err) => match err.kind() {
uv_client::ErrorKind::PackageNotFound(_) => {
if let Some(flat_index) = flat_index
.and_then(|flat_index| flat_index.get(package_name))
@ -232,7 +232,7 @@ impl<Context: BuildContext> ResolverProvider for DefaultResolverProvider<'_, Con
Ok(VersionsResponse::Offline)
}
}
kind => Err(kind.into()),
_ => Err(err),
},
}
}
@ -246,20 +246,25 @@ impl<Context: BuildContext> ResolverProvider for DefaultResolverProvider<'_, Con
{
Ok(metadata) => Ok(MetadataResponse::Found(metadata)),
Err(err) => match err {
uv_distribution::Error::Client(client) => match client.into_kind() {
uv_client::ErrorKind::Offline(_) => {
Ok(MetadataResponse::Unavailable(MetadataUnavailable::Offline))
uv_distribution::Error::Client(client) => {
let retries = client.retries();
match client.into_kind() {
uv_client::ErrorKind::Offline(_) => {
Ok(MetadataResponse::Unavailable(MetadataUnavailable::Offline))
}
uv_client::ErrorKind::MetadataParseError(_, _, err) => {
Ok(MetadataResponse::Unavailable(
MetadataUnavailable::InvalidMetadata(Arc::new(*err)),
))
}
uv_client::ErrorKind::Metadata(_, err) => {
Ok(MetadataResponse::Unavailable(
MetadataUnavailable::InvalidStructure(Arc::new(err)),
))
}
kind => Err(uv_client::Error::new(kind, retries).into()),
}
uv_client::ErrorKind::MetadataParseError(_, _, err) => {
Ok(MetadataResponse::Unavailable(
MetadataUnavailable::InvalidMetadata(Arc::new(*err)),
))
}
uv_client::ErrorKind::Metadata(_, err) => Ok(MetadataResponse::Unavailable(
MetadataUnavailable::InvalidStructure(Arc::new(err)),
)),
kind => Err(uv_client::Error::from(kind).into()),
},
}
uv_distribution::Error::WheelMetadataVersionMismatch { .. } => {
Ok(MetadataResponse::Unavailable(
MetadataUnavailable::InconsistentMetadata(Arc::new(err)),

4
crates/uv/src/commands/pip/latest.rs
View File

@ -45,11 +45,11 @@ impl LatestClient<'_> {
{
Ok(archives) => archives,
Err(err) => {
return match err.into_kind() {
return match err.kind() {
uv_client::ErrorKind::PackageNotFound(_) => Ok(None),
uv_client::ErrorKind::NoIndex(_) => Ok(None),
uv_client::ErrorKind::Offline(_) => Ok(None),
kind => Err(kind.into()),
_ => Err(err),
};
}
};