Skip to content

Commit

Permalink
Always attach URL to network errors (#8444)
Browse files Browse the repository at this point in the history
  • Loading branch information
konstin authored Oct 25, 2024
1 parent 58b5fd4 commit 3eda248
Show file tree
Hide file tree
Showing 12 changed files with 130 additions and 83 deletions.
13 changes: 7 additions & 6 deletions crates/uv-client/src/cached_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -468,9 +468,9 @@ impl CachedClient {
.execute(req)
.instrument(info_span!("revalidation_request", url = url.as_str()))
.await
.map_err(ErrorKind::from)?
.map_err(|err| ErrorKind::from_reqwest_middleware(url.clone(), err))?
.error_for_status()
.map_err(ErrorKind::from)?;
.map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?;
match cached
.cache_policy
.after_response(new_cache_policy_builder, &response)
Expand Down Expand Up @@ -500,16 +500,17 @@ impl CachedClient {
&self,
req: Request,
) -> Result<(Response, Option<Box<CachePolicy>>), Error> {
trace!("Sending fresh {} request for {}", req.method(), req.url());
let url = req.url().clone();
trace!("Sending fresh {} request for {}", req.method(), url);
let cache_policy_builder = CachePolicyBuilder::new(&req);
let response = self
.0
.for_host(req.url())
.for_host(&url)
.execute(req)
.await
.map_err(ErrorKind::from)?
.map_err(|err| ErrorKind::from_reqwest_middleware(url.clone(), err))?
.error_for_status()
.map_err(ErrorKind::from)?;
.map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?;
let cache_policy = cache_policy_builder.build(&response);
let cache_policy = if cache_policy.to_archived().is_storable() {
Some(Box::new(cache_policy))
Expand Down
20 changes: 9 additions & 11 deletions crates/uv-client/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,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(err) => {
ErrorKind::WrappedReqwestError(_url, err) => {
if let Some(status) = err.status() {
// If the server doesn't support HEAD requests, we can't check for range
// requests.
Expand Down Expand Up @@ -180,8 +180,8 @@ pub enum ErrorKind {
MetadataNotFound(WheelFilename, String),

/// An error that happened while making a request or in a reqwest middleware.
#[error(transparent)]
WrappedReqwestError(#[from] WrappedReqwestError),
#[error("Failed to fetch: `{0}`")]
WrappedReqwestError(Url, #[source] WrappedReqwestError),

#[error("Received some unexpected JSON from {url}")]
BadJson { source: serde_json::Error, url: Url },
Expand All @@ -208,7 +208,7 @@ pub enum ErrorKind {
CacheWrite(#[source] std::io::Error),

#[error(transparent)]
Io(#[from] std::io::Error),
Io(std::io::Error),

#[error("Cache deserialization failed")]
Decode(#[source] rmp_serde::decode::Error),
Expand All @@ -235,21 +235,19 @@ pub enum ErrorKind {
Offline(String),
}

impl From<reqwest::Error> for ErrorKind {
fn from(error: reqwest::Error) -> Self {
Self::WrappedReqwestError(WrappedReqwestError::from(error))
impl ErrorKind {
pub(crate) fn from_reqwest(url: Url, error: reqwest::Error) -> Self {
Self::WrappedReqwestError(url, WrappedReqwestError::from(error))
}
}

impl From<reqwest_middleware::Error> for ErrorKind {
fn from(err: reqwest_middleware::Error) -> Self {
pub(crate) fn from_reqwest_middleware(url: Url, err: reqwest_middleware::Error) -> Self {
if let reqwest_middleware::Error::Middleware(ref underlying) = err {
if let Some(err) = underlying.downcast_ref::<OfflineError>() {
return Self::Offline(err.url().to_string());
}
}

Self::WrappedReqwestError(WrappedReqwestError(err))
Self::WrappedReqwestError(url, WrappedReqwestError(err))
}
}

Expand Down
7 changes: 5 additions & 2 deletions crates/uv-client/src/flat_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,14 +160,17 @@ impl<'a> FlatIndexClient<'a> {
.header("Accept-Encoding", "gzip")
.header("Accept", "text/html")
.build()
.map_err(ErrorKind::from)?;
.map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?;
let parse_simple_response = |response: Response| {
async {
// Use the response URL, rather than the request URL, as the base for relative URLs.
// This ensures that we handle redirects and other URL transformations correctly.
let url = response.url().clone();

let text = response.text().await.map_err(ErrorKind::from)?;
let text = response
.text()
.await
.map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?;
let SimpleHtml { base, files } = SimpleHtml::parse(&text, &url)
.map_err(|err| Error::from_html_err(err, url.clone()))?;

Expand Down
27 changes: 18 additions & 9 deletions crates/uv-client/src/registry_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,15 +232,15 @@ impl RegistryClient {
}
Err(err) => match err.into_kind() {
// The package could not be found in the remote index.
ErrorKind::WrappedReqwestError(err) => match err.status() {
ErrorKind::WrappedReqwestError(url, err) => match err.status() {
Some(StatusCode::NOT_FOUND) => {}
Some(StatusCode::UNAUTHORIZED) => {
capabilities.set_unauthorized(index.clone());
}
Some(StatusCode::FORBIDDEN) => {
capabilities.set_forbidden(index.clone());
}
_ => return Err(ErrorKind::from(err).into()),
_ => return Err(ErrorKind::WrappedReqwestError(url, err).into()),
},

// The package is unavailable due to a lack of connectivity.
Expand Down Expand Up @@ -323,7 +323,7 @@ impl RegistryClient {
.header("Accept-Encoding", "gzip")
.header("Accept", MediaType::accepts())
.build()
.map_err(ErrorKind::from)?;
.map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?;
let parse_simple_response = |response: Response| {
async {
// Use the response URL, rather than the request URL, as the base for relative URLs.
Expand All @@ -347,14 +347,20 @@ impl RegistryClient {

let unarchived = match media_type {
MediaType::Json => {
let bytes = response.bytes().await.map_err(ErrorKind::from)?;
let bytes = response
.bytes()
.await
.map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?;
let data: SimpleJson = serde_json::from_slice(bytes.as_ref())
.map_err(|err| Error::from_json_err(err, url.clone()))?;

SimpleMetadata::from_files(data.files, package_name, &url)
}
MediaType::Html => {
let text = response.text().await.map_err(ErrorKind::from)?;
let text = response
.text()
.await
.map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?;
SimpleMetadata::from_html(&text, package_name, &url)?
}
};
Expand Down Expand Up @@ -547,7 +553,10 @@ impl RegistryClient {
};

let response_callback = |response: Response| async {
let bytes = response.bytes().await.map_err(ErrorKind::from)?;
let bytes = response
.bytes()
.await
.map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?;

info_span!("parse_metadata21")
.in_scope(|| ResolutionMetadata::parse_metadata(bytes.as_ref()))
Expand All @@ -563,7 +572,7 @@ impl RegistryClient {
.uncached_client(&url)
.get(url.clone())
.build()
.map_err(ErrorKind::from)?;
.map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?;
Ok(self
.cached_client()
.get_serde(req, &cache_entry, cache_control, response_callback)
Expand Down Expand Up @@ -616,7 +625,7 @@ impl RegistryClient {
http::HeaderValue::from_static("identity"),
)
.build()
.map_err(ErrorKind::from)?;
.map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?;

// Copy authorization headers from the HEAD request to subsequent requests
let mut headers = HeaderMap::default();
Expand Down Expand Up @@ -694,7 +703,7 @@ impl RegistryClient {
reqwest::header::HeaderValue::from_static("identity"),
)
.build()
.map_err(ErrorKind::from)?;
.map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?;

// Stream the file, searching for the METADATA.
let read_metadata_stream = |response: Response| {
Expand Down
7 changes: 5 additions & 2 deletions crates/uv-client/src/tls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,15 @@ pub(crate) enum CertificateError {
#[error(transparent)]
Io(#[from] std::io::Error),
#[error(transparent)]
Reqwest(#[from] reqwest::Error),
Reqwest(reqwest::Error),
}

/// Return the `Identity` from the provided file.
pub(crate) fn read_identity(ssl_client_cert: &OsStr) -> Result<Identity, CertificateError> {
let mut buf = Vec::new();
fs_err::File::open(ssl_client_cert)?.read_to_end(&mut buf)?;
Ok(Identity::from_pem(&buf)?)
Identity::from_pem(&buf).map_err(|tls_err| {
debug_assert!(tls_err.is_builder(), "must be a rustls::Error internally");
CertificateError::Reqwest(tls_err)
})
}
3 changes: 2 additions & 1 deletion crates/uv-distribution/src/distribution_database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> {
io::Error::new(
io::ErrorKind::TimedOut,
format!(
"Failed to download distribution due to network timeout. Try increasing UV_HTTP_TIMEOUT (current value: {}s).", self.client.unmanaged.timeout().as_secs()
"Failed to download distribution due to network timeout. Try increasing UV_HTTP_TIMEOUT (current value: {}s).",
self.client.unmanaged.timeout().as_secs()
),
)
} else {
Expand Down
8 changes: 6 additions & 2 deletions crates/uv-publish/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ pub enum PublishPrepareError {
#[derive(Error, Debug)]
pub enum PublishSendError {
#[error("Failed to send POST request")]
ReqwestMiddleware(#[from] reqwest_middleware::Error),
ReqwestMiddleware(#[source] reqwest_middleware::Error),
#[error("Upload failed with status {0}")]
StatusNoBody(StatusCode, #[source] reqwest::Error),
#[error("Upload failed with status code {0}. Server says: {1}")]
Expand Down Expand Up @@ -336,7 +336,11 @@ pub async fn upload(
}

let response = result.map_err(|err| {
PublishError::PublishSend(file.to_path_buf(), registry.clone(), err.into())
PublishError::PublishSend(
file.to_path_buf(),
registry.clone(),
PublishSendError::ReqwestMiddleware(err),
)
})?;

return handle_response(registry, response)
Expand Down
43 changes: 31 additions & 12 deletions crates/uv-publish/src/trusted_publishing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ pub enum TrustedPublishingError {
Var(#[from] VarError),
#[error(transparent)]
Url(#[from] url::ParseError),
#[error(transparent)]
Reqwest(#[from] reqwest::Error),
#[error(transparent)]
ReqwestMiddleware(#[from] reqwest_middleware::Error),
#[error("Failed to fetch: `{0}`")]
Reqwest(Url, #[source] reqwest::Error),
#[error("Failed to fetch: `{0}`")]
ReqwestMiddleware(Url, #[source] reqwest_middleware::Error),
#[error(transparent)]
SerdeJson(#[from] serde_json::error::Error),
#[error(
Expand Down Expand Up @@ -105,8 +105,17 @@ async fn get_audience(
// (RFC 3986).
let audience_url = Url::parse(&format!("https://{}/_/oidc/audience", registry.authority()))?;
debug!("Querying the trusted publishing audience from {audience_url}");
let response = client.get(audience_url).send().await?;
let audience = response.error_for_status()?.json::<Audience>().await?;
let response = client
.get(audience_url.clone())
.send()
.await
.map_err(|err| TrustedPublishingError::ReqwestMiddleware(audience_url.clone(), err))?;
let audience = response
.error_for_status()
.map_err(|err| TrustedPublishingError::Reqwest(audience_url.clone(), err))?
.json::<Audience>()
.await
.map_err(|err| TrustedPublishingError::Reqwest(audience_url.clone(), err))?;
trace!("The audience is `{}`", &audience.audience);
Ok(audience.audience)
}
Expand All @@ -123,11 +132,17 @@ async fn get_oidc_token(
debug!("Querying the trusted publishing OIDC token from {oidc_token_url}");
let authorization = format!("bearer {oidc_token_request_token}");
let response = client
.get(oidc_token_url)
.get(oidc_token_url.clone())
.header(header::AUTHORIZATION, authorization)
.send()
.await?;
let oidc_token: OidcToken = response.error_for_status()?.json().await?;
.await
.map_err(|err| TrustedPublishingError::ReqwestMiddleware(oidc_token_url.clone(), err))?;
let oidc_token: OidcToken = response
.error_for_status()
.map_err(|err| TrustedPublishingError::Reqwest(oidc_token_url.clone(), err))?
.json()
.await
.map_err(|err| TrustedPublishingError::Reqwest(oidc_token_url.clone(), err))?;
Ok(oidc_token.value)
}

Expand All @@ -145,14 +160,18 @@ async fn get_publish_token(
token: oidc_token.to_string(),
};
let response = client
.post(mint_token_url)
.post(mint_token_url.clone())
.body(serde_json::to_vec(&mint_token_payload)?)
.send()
.await?;
.await
.map_err(|err| TrustedPublishingError::ReqwestMiddleware(mint_token_url.clone(), err))?;

// reqwest's implementation of `.json()` also goes through `.bytes()`
let status = response.status();
let body = response.bytes().await?;
let body = response
.bytes()
.await
.map_err(|err| TrustedPublishingError::Reqwest(mint_token_url.clone(), err))?;

if status.is_success() {
let publish_token: PublishToken = serde_json::from_slice(&body)?;
Expand Down
37 changes: 22 additions & 15 deletions crates/uv-python/src/downloads.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ pub enum Error {
InvalidPythonVersion(String),
#[error("Invalid request key (too many parts): {0}")]
TooManyParts(String),
#[error(transparent)]
NetworkError(#[from] WrappedReqwestError),
#[error(transparent)]
NetworkMiddlewareError(#[from] anyhow::Error),
#[error("Failed to download {0}")]
NetworkError(Url, #[source] WrappedReqwestError),
#[error("Failed to download {0}")]
NetworkMiddlewareError(Url, #[source] anyhow::Error),
#[error("Failed to extract archive: {0}")]
ExtractError(String, #[source] uv_extract::Error),
#[error("Failed to hash installation")]
Expand Down Expand Up @@ -619,18 +619,18 @@ impl ManagedPythonDownload {
}
}

impl From<reqwest::Error> for Error {
fn from(error: reqwest::Error) -> Self {
Self::NetworkError(WrappedReqwestError::from(error))
impl Error {
pub(crate) fn from_reqwest(url: Url, err: reqwest::Error) -> Self {
Self::NetworkError(url, WrappedReqwestError::from(err))
}
}

impl From<reqwest_middleware::Error> for Error {
fn from(error: reqwest_middleware::Error) -> Self {
match error {
reqwest_middleware::Error::Middleware(error) => Self::NetworkMiddlewareError(error),
pub(crate) fn from_reqwest_middleware(url: Url, err: reqwest_middleware::Error) -> Self {
match err {
reqwest_middleware::Error::Middleware(error) => {
Self::NetworkMiddlewareError(url, error)
}
reqwest_middleware::Error::Reqwest(error) => {
Self::NetworkError(WrappedReqwestError::from(error))
Self::NetworkError(url, WrappedReqwestError::from(error))
}
}
}
Expand Down Expand Up @@ -701,10 +701,17 @@ async fn read_url(

Ok((Either::Left(reader), Some(size)))
} else {
let response = client.for_host(url).get(url.clone()).send().await?;
let response = client
.for_host(url)
.get(url.clone())
.send()
.await
.map_err(|err| Error::from_reqwest_middleware(url.clone(), err))?;

// Ensure the request was successful.
response.error_for_status_ref()?;
response
.error_for_status_ref()
.map_err(|err| Error::from_reqwest(url.clone(), err))?;

let size = response.content_length();
let stream = response
Expand Down
Loading

0 comments on commit 3eda248

Please sign in to comment.