Skip to content

Commit 215e729

Browse files
committed
Always attach URL to network errors
Due to the enduring problems with #8144 and related issues, I've opted for a more systemic approach and switched all reqwest errors to not use the implicit-try-fallthrough `#[from]`, but an explicit `#[source]` with a URL attached (except for when there is only one or no URL). This guarantees that we get the URL that failed, and helps identifying the responsible code path.
1 parent 0dd4d01 commit 215e729

File tree

12 files changed

+144
-83
lines changed

12 files changed

+144
-83
lines changed

crates/uv-client/src/cached_client.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -468,9 +468,9 @@ impl CachedClient {
468468
.execute(req)
469469
.instrument(info_span!("revalidation_request", url = url.as_str()))
470470
.await
471-
.map_err(ErrorKind::from)?
471+
.map_err(|err| ErrorKind::from_reqwest_middleware(url.clone(), err))?
472472
.error_for_status()
473-
.map_err(ErrorKind::from)?;
473+
.map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?;
474474
match cached
475475
.cache_policy
476476
.after_response(new_cache_policy_builder, &response)
@@ -500,16 +500,17 @@ impl CachedClient {
500500
&self,
501501
req: Request,
502502
) -> Result<(Response, Option<Box<CachePolicy>>), Error> {
503-
trace!("Sending fresh {} request for {}", req.method(), req.url());
503+
let url = req.url().clone();
504+
trace!("Sending fresh {} request for {}", req.method(), url);
504505
let cache_policy_builder = CachePolicyBuilder::new(&req);
505506
let response = self
506507
.0
507-
.for_host(req.url())
508+
.for_host(&url)
508509
.execute(req)
509510
.await
510-
.map_err(ErrorKind::from)?
511+
.map_err(|err| ErrorKind::from_reqwest_middleware(url.clone(), err))?
511512
.error_for_status()
512-
.map_err(ErrorKind::from)?;
513+
.map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?;
513514
let cache_policy = cache_policy_builder.build(&response);
514515
let cache_policy = if cache_policy.to_archived().is_storable() {
515516
Some(Box::new(cache_policy))

crates/uv-client/src/error.rs

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ impl Error {
7373

7474
// The server returned a "Method Not Allowed" error, indicating it doesn't support
7575
// HEAD requests, so we can't check for range requests.
76-
ErrorKind::WrappedReqwestError(err) => {
76+
ErrorKind::WrappedReqwestError(_url, err) => {
7777
if let Some(status) = err.status() {
7878
// If the server doesn't support HEAD requests, we can't check for range
7979
// requests.
@@ -180,8 +180,8 @@ pub enum ErrorKind {
180180
MetadataNotFound(WheelFilename, String),
181181

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

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

210210
#[error(transparent)]
211-
Io(#[from] std::io::Error),
211+
Io(std::io::Error),
212212

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

238-
impl From<reqwest::Error> for ErrorKind {
239-
fn from(error: reqwest::Error) -> Self {
240-
Self::WrappedReqwestError(WrappedReqwestError::from(error))
238+
impl ErrorKind {
239+
pub(crate) fn from_reqwest(url: Url, error: reqwest::Error) -> Self {
240+
Self::WrappedReqwestError(url, WrappedReqwestError::from(error))
241241
}
242-
}
243242

244-
impl From<reqwest_middleware::Error> for ErrorKind {
245-
fn from(err: reqwest_middleware::Error) -> Self {
243+
pub(crate) fn from_reqwest_middleware(url: Url, err: reqwest_middleware::Error) -> Self {
246244
if let reqwest_middleware::Error::Middleware(ref underlying) = err {
247245
if let Some(err) = underlying.downcast_ref::<OfflineError>() {
248246
return Self::Offline(err.url().to_string());
249247
}
250248
}
251249

252-
Self::WrappedReqwestError(WrappedReqwestError(err))
250+
Self::WrappedReqwestError(url, WrappedReqwestError(err))
253251
}
254252
}
255253

crates/uv-client/src/flat_index.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,14 +160,17 @@ impl<'a> FlatIndexClient<'a> {
160160
.header("Accept-Encoding", "gzip")
161161
.header("Accept", "text/html")
162162
.build()
163-
.map_err(ErrorKind::from)?;
163+
.map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?;
164164
let parse_simple_response = |response: Response| {
165165
async {
166166
// Use the response URL, rather than the request URL, as the base for relative URLs.
167167
// This ensures that we handle redirects and other URL transformations correctly.
168168
let url = response.url().clone();
169169

170-
let text = response.text().await.map_err(ErrorKind::from)?;
170+
let text = response
171+
.text()
172+
.await
173+
.map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?;
171174
let SimpleHtml { base, files } = SimpleHtml::parse(&text, &url)
172175
.map_err(|err| Error::from_html_err(err, url.clone()))?;
173176

crates/uv-client/src/registry_client.rs

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -232,15 +232,15 @@ impl RegistryClient {
232232
}
233233
Err(err) => match err.into_kind() {
234234
// The package could not be found in the remote index.
235-
ErrorKind::WrappedReqwestError(err) => match err.status() {
235+
ErrorKind::WrappedReqwestError(url, err) => match err.status() {
236236
Some(StatusCode::NOT_FOUND) => {}
237237
Some(StatusCode::UNAUTHORIZED) => {
238238
capabilities.set_unauthorized(index.clone());
239239
}
240240
Some(StatusCode::FORBIDDEN) => {
241241
capabilities.set_forbidden(index.clone());
242242
}
243-
_ => return Err(ErrorKind::from(err).into()),
243+
_ => return Err(ErrorKind::WrappedReqwestError(url, err).into()),
244244
},
245245

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

348348
let unarchived = match media_type {
349349
MediaType::Json => {
350-
let bytes = response.bytes().await.map_err(ErrorKind::from)?;
350+
let bytes = response
351+
.bytes()
352+
.await
353+
.map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?;
351354
let data: SimpleJson = serde_json::from_slice(bytes.as_ref())
352355
.map_err(|err| Error::from_json_err(err, url.clone()))?;
353356

354357
SimpleMetadata::from_files(data.files, package_name, &url)
355358
}
356359
MediaType::Html => {
357-
let text = response.text().await.map_err(ErrorKind::from)?;
360+
let text = response
361+
.text()
362+
.await
363+
.map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?;
358364
SimpleMetadata::from_html(&text, package_name, &url)?
359365
}
360366
};
@@ -547,7 +553,10 @@ impl RegistryClient {
547553
};
548554

549555
let response_callback = |response: Response| async {
550-
let bytes = response.bytes().await.map_err(ErrorKind::from)?;
556+
let bytes = response
557+
.bytes()
558+
.await
559+
.map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?;
551560

552561
info_span!("parse_metadata21")
553562
.in_scope(|| ResolutionMetadata::parse_metadata(bytes.as_ref()))
@@ -563,7 +572,7 @@ impl RegistryClient {
563572
.uncached_client(&url)
564573
.get(url.clone())
565574
.build()
566-
.map_err(ErrorKind::from)?;
575+
.map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?;
567576
Ok(self
568577
.cached_client()
569578
.get_serde(req, &cache_entry, cache_control, response_callback)
@@ -616,7 +625,7 @@ impl RegistryClient {
616625
http::HeaderValue::from_static("identity"),
617626
)
618627
.build()
619-
.map_err(ErrorKind::from)?;
628+
.map_err(|err| ErrorKind::from_reqwest(url.clone(), err))?;
620629

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

699708
// Stream the file, searching for the METADATA.
700709
let read_metadata_stream = |response: Response| {

crates/uv-client/src/tls.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,15 @@ pub(crate) enum CertificateError {
77
#[error(transparent)]
88
Io(#[from] std::io::Error),
99
#[error(transparent)]
10-
Reqwest(#[from] reqwest::Error),
10+
Reqwest(reqwest::Error),
1111
}
1212

1313
/// Return the `Identity` from the provided file.
1414
pub(crate) fn read_identity(ssl_client_cert: &OsStr) -> Result<Identity, CertificateError> {
1515
let mut buf = Vec::new();
1616
fs_err::File::open(ssl_client_cert)?.read_to_end(&mut buf)?;
17-
Ok(Identity::from_pem(&buf)?)
17+
Identity::from_pem(&buf).map_err(|tls_err| {
18+
debug_assert!(tls_err.is_builder(), "must be a rustls::Error internally");
19+
CertificateError::Reqwest(tls_err)
20+
})
1821
}

crates/uv-distribution/src/distribution_database.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,8 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> {
8787
io::Error::new(
8888
io::ErrorKind::TimedOut,
8989
format!(
90-
"Failed to download distribution due to network timeout. Try increasing UV_HTTP_TIMEOUT (current value: {}s).", self.client.unmanaged.timeout().as_secs()
90+
"Failed to download distribution due to network timeout. Try increasing UV_HTTP_TIMEOUT (current value: {}s).",
91+
self.client.unmanaged.timeout().as_secs()
9192
),
9293
)
9394
} else {

crates/uv-publish/src/lib.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ pub enum PublishPrepareError {
7979
#[derive(Error, Debug)]
8080
pub enum PublishSendError {
8181
#[error("Failed to send POST request")]
82-
ReqwestMiddleware(#[from] reqwest_middleware::Error),
82+
ReqwestMiddleware(#[source] reqwest_middleware::Error),
8383
#[error("Upload failed with status {0}")]
8484
StatusNoBody(StatusCode, #[source] reqwest::Error),
8585
#[error("Upload failed with status code {0}. Server says: {1}")]
@@ -336,7 +336,11 @@ pub async fn upload(
336336
}
337337

338338
let response = result.map_err(|err| {
339-
PublishError::PublishSend(file.to_path_buf(), registry.clone(), err.into())
339+
PublishError::PublishSend(
340+
file.to_path_buf(),
341+
registry.clone(),
342+
PublishSendError::ReqwestMiddleware(err),
343+
)
340344
})?;
341345

342346
return handle_response(registry, response)

crates/uv-publish/src/trusted_publishing.rs

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@ pub enum TrustedPublishingError {
1717
Var(#[from] VarError),
1818
#[error(transparent)]
1919
Url(#[from] url::ParseError),
20-
#[error(transparent)]
21-
Reqwest(#[from] reqwest::Error),
22-
#[error(transparent)]
23-
ReqwestMiddleware(#[from] reqwest_middleware::Error),
20+
#[error("Failed to fetch {0}")]
21+
Reqwest(Url, #[source] reqwest::Error),
22+
#[error("Failed to fetch {0}")]
23+
ReqwestMiddleware(Url, #[source] reqwest_middleware::Error),
2424
#[error(transparent)]
2525
SerdeJson(#[from] serde_json::error::Error),
2626
#[error(
@@ -105,8 +105,17 @@ async fn get_audience(
105105
// (RFC 3986).
106106
let audience_url = Url::parse(&format!("https://{}/_/oidc/audience", registry.authority()))?;
107107
debug!("Querying the trusted publishing audience from {audience_url}");
108-
let response = client.get(audience_url).send().await?;
109-
let audience = response.error_for_status()?.json::<Audience>().await?;
108+
let response = client
109+
.get(audience_url.clone())
110+
.send()
111+
.await
112+
.map_err(|err| TrustedPublishingError::ReqwestMiddleware(audience_url.clone(), err))?;
113+
let audience = response
114+
.error_for_status()
115+
.map_err(|err| TrustedPublishingError::Reqwest(audience_url.clone(), err))?
116+
.json::<Audience>()
117+
.await
118+
.map_err(|err| TrustedPublishingError::Reqwest(audience_url.clone(), err))?;
110119
trace!("The audience is `{}`", &audience.audience);
111120
Ok(audience.audience)
112121
}
@@ -123,11 +132,17 @@ async fn get_oidc_token(
123132
debug!("Querying the trusted publishing OIDC token from {oidc_token_url}");
124133
let authorization = format!("bearer {oidc_token_request_token}");
125134
let response = client
126-
.get(oidc_token_url)
135+
.get(oidc_token_url.clone())
127136
.header(header::AUTHORIZATION, authorization)
128137
.send()
129-
.await?;
130-
let oidc_token: OidcToken = response.error_for_status()?.json().await?;
138+
.await
139+
.map_err(|err| TrustedPublishingError::ReqwestMiddleware(oidc_token_url.clone(), err))?;
140+
let oidc_token: OidcToken = response
141+
.error_for_status()
142+
.map_err(|err| TrustedPublishingError::Reqwest(oidc_token_url.clone(), err))?
143+
.json()
144+
.await
145+
.map_err(|err| TrustedPublishingError::Reqwest(oidc_token_url.clone(), err))?;
131146
Ok(oidc_token.value)
132147
}
133148

@@ -145,14 +160,18 @@ async fn get_publish_token(
145160
token: oidc_token.to_string(),
146161
};
147162
let response = client
148-
.post(mint_token_url)
163+
.post(mint_token_url.clone())
149164
.body(serde_json::to_vec(&mint_token_payload)?)
150165
.send()
151-
.await?;
166+
.await
167+
.map_err(|err| TrustedPublishingError::ReqwestMiddleware(mint_token_url.clone(), err))?;
152168

153169
// reqwest's implementation of `.json()` also goes through `.bytes()`
154170
let status = response.status();
155-
let body = response.bytes().await?;
171+
let body = response
172+
.bytes()
173+
.await
174+
.map_err(|err| TrustedPublishingError::Reqwest(mint_token_url.clone(), err))?;
156175

157176
if status.is_success() {
158177
let publish_token: PublishToken = serde_json::from_slice(&body)?;

crates/uv-python/src/downloads.rs

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,10 @@ pub enum Error {
3939
InvalidPythonVersion(String),
4040
#[error("Invalid request key (too many parts): {0}")]
4141
TooManyParts(String),
42-
#[error(transparent)]
43-
NetworkError(#[from] WrappedReqwestError),
44-
#[error(transparent)]
45-
NetworkMiddlewareError(#[from] anyhow::Error),
42+
#[error("Failed to download {0}")]
43+
NetworkError(Url, #[source] WrappedReqwestError),
44+
#[error("Failed to download {0}")]
45+
NetworkMiddlewareError(Url, #[source] anyhow::Error),
4646
#[error("Failed to extract archive: {0}")]
4747
ExtractError(String, #[source] uv_extract::Error),
4848
#[error("Failed to hash installation")]
@@ -612,18 +612,18 @@ impl ManagedPythonDownload {
612612
}
613613
}
614614

615-
impl From<reqwest::Error> for Error {
616-
fn from(error: reqwest::Error) -> Self {
617-
Self::NetworkError(WrappedReqwestError::from(error))
615+
impl Error {
616+
pub(crate) fn from_reqwest(url: Url, err: reqwest::Error) -> Self {
617+
Self::NetworkError(url, WrappedReqwestError::from(err))
618618
}
619-
}
620619

621-
impl From<reqwest_middleware::Error> for Error {
622-
fn from(error: reqwest_middleware::Error) -> Self {
623-
match error {
624-
reqwest_middleware::Error::Middleware(error) => Self::NetworkMiddlewareError(error),
620+
pub(crate) fn from_reqwest_middleware(url: Url, err: reqwest_middleware::Error) -> Self {
621+
match err {
622+
reqwest_middleware::Error::Middleware(error) => {
623+
Self::NetworkMiddlewareError(url, error)
624+
}
625625
reqwest_middleware::Error::Reqwest(error) => {
626-
Self::NetworkError(WrappedReqwestError::from(error))
626+
Self::NetworkError(url, WrappedReqwestError::from(error))
627627
}
628628
}
629629
}
@@ -694,10 +694,17 @@ async fn read_url(
694694

695695
Ok((Either::Left(reader), Some(size)))
696696
} else {
697-
let response = client.client().get(url.clone()).send().await?;
697+
let response = client
698+
.client()
699+
.get(url.clone())
700+
.send()
701+
.await
702+
.map_err(|err| Error::from_reqwest_middleware(url.clone(), err))?;
698703

699704
// Ensure the request was successful.
700-
response.error_for_status_ref()?;
705+
response
706+
.error_for_status_ref()
707+
.map_err(|err| Error::from_reqwest(url.clone(), err))?;
701708

702709
let size = response.content_length();
703710
let stream = response

0 commit comments

Comments
 (0)