diff --git a/crates/uv-client/src/cached_client.rs b/crates/uv-client/src/cached_client.rs index 5e2428de2bcb6..d6eddcd95dd68 100644 --- a/crates/uv-client/src/cached_client.rs +++ b/crates/uv-client/src/cached_client.rs @@ -20,25 +20,6 @@ use crate::{ rkyvutil::OwnedArchive, }; -/// Extract problem details from an HTTP response if it has the correct content type -/// -/// Note: This consumes the response body, so it should only be called when there's an error status. -async fn extract_problem_details(response: Response) -> Option { - match response.bytes().await { - Ok(bytes) => match serde_json::from_slice(&bytes) { - Ok(details) => Some(details), - Err(err) => { - warn!("Failed to parse problem details: {err}"); - None - } - }, - Err(err) => { - warn!("Failed to read response body for problem details: {err}"); - None - } - } -} - /// A trait the generalizes (de)serialization at a high level. /// /// The main purpose of this trait is to make the `CachedClient` work for @@ -549,18 +530,7 @@ impl CachedClient { // Check for HTTP error status and extract problem details if available if let Err(status_error) = response.error_for_status_ref() { - // Clone the response to extract problem details before the error consumes it - let problem_details = if response - .headers() - .get("content-type") - .and_then(|ct| ct.to_str().ok()) - .map(|ct| ct == "application/problem+json") - .unwrap_or(false) - { - extract_problem_details(response).await - } else { - None - }; + let problem_details = ProblemDetails::try_from_response(response).await; return Err(ErrorKind::from_reqwest_with_problem_details( url.clone(), status_error, @@ -637,17 +607,7 @@ impl CachedClient { .map(|retries| retries.value()); if let Err(status_error) = response.error_for_status_ref() { - let problem_details = if response - .headers() - .get("content-type") - .and_then(|ct| ct.to_str().ok()) - .map(|ct| ct.starts_with("application/problem+json")) - .unwrap_or(false) - { - extract_problem_details(response).await - } else { - None - }; + let problem_details = ProblemDetails::try_from_response(response).await; return Err(Error::new( ErrorKind::from_reqwest_with_problem_details(url, status_error, problem_details), retry_count.unwrap_or_default(), diff --git a/crates/uv-client/src/error.rs b/crates/uv-client/src/error.rs index 2cf891c5c948c..dc41a8366fb61 100644 --- a/crates/uv-client/src/error.rs +++ b/crates/uv-client/src/error.rs @@ -1,9 +1,11 @@ use async_http_range_reader::AsyncHttpRangeReaderError; use async_zip::error::ZipError; +use reqwest::Response; use serde::Deserialize; use std::fmt::{Display, Formatter}; use std::ops::Deref; use std::path::PathBuf; +use tracing::warn; use uv_cache::Error as CacheError; use uv_distribution_filename::{WheelFilename, WheelFilenameError}; @@ -44,6 +46,46 @@ fn default_problem_type() -> String { } impl ProblemDetails { + /// The content type for RFC 9457 Problem Details responses. + pub const CONTENT_TYPE: &str = "application/problem+json"; + + /// Try to parse a response body as RFC 9457 Problem Details. + /// + /// Returns `None` if parsing fails. The caller is responsible for checking + /// that the content type is `application/problem+json` before calling this. + pub fn try_from_response_body(body: &[u8]) -> Option { + match serde_json::from_slice(body) { + Ok(details) => Some(details), + Err(err) => { + warn!("Failed to parse problem details: {err}"); + None + } + } + } + + /// Try to extract RFC 9457 Problem Details from an HTTP response. + /// + /// Returns `None` if the content type is not `application/problem+json` + /// or if parsing fails. Only consumes the response body if the content + /// type matches. + pub async fn try_from_response(response: Response) -> Option { + let is_problem = response + .headers() + .get(reqwest::header::CONTENT_TYPE) + .and_then(|ct| ct.to_str().ok()) + .is_some_and(|ct| ct == Self::CONTENT_TYPE); + if !is_problem { + return None; + } + match response.bytes().await { + Ok(bytes) => Self::try_from_response_body(&bytes), + Err(err) => { + warn!("Failed to read response body for problem details: {err}"); + None + } + } + } + /// Get a human-readable description of the problem pub fn description(&self) -> Option { match self { @@ -680,4 +722,15 @@ mod tests { Some("You do not have enough credit.".to_string()) ); } + + #[test] + fn test_try_from_response_body() { + let body = r#"{"type": "about:blank", "status": 400, "title": "Bad Request", "detail": "Missing required field `name`"}"#; + let problem = ProblemDetails::try_from_response_body(body.as_bytes()) + .expect("should parse problem details"); + assert_eq!( + problem.description().unwrap(), + "Server message: Bad Request, Missing required field `name`" + ); + } } diff --git a/crates/uv-client/src/lib.rs b/crates/uv-client/src/lib.rs index 220e3f35ab990..f758dad72b336 100644 --- a/crates/uv-client/src/lib.rs +++ b/crates/uv-client/src/lib.rs @@ -4,7 +4,7 @@ pub use base_client::{ RetryParsingError, RetryState, UvRetryableStrategy, }; pub use cached_client::{CacheControl, CachedClient, CachedClientError, DataWithCachePolicy}; -pub use error::{Error, ErrorKind, WrappedReqwestError}; +pub use error::{Error, ErrorKind, ProblemDetails, WrappedReqwestError}; pub use flat_index::{FlatIndexClient, FlatIndexEntries, FlatIndexEntry, FlatIndexError}; pub use linehaul::LineHaul; pub use registry_client::{ diff --git a/crates/uv-publish/src/lib.rs b/crates/uv-publish/src/lib.rs index f06257d6f01c5..ac86ebee785d0 100644 --- a/crates/uv-publish/src/lib.rs +++ b/crates/uv-publish/src/lib.rs @@ -130,6 +130,8 @@ pub enum PublishSendError { StatusNoBody(StatusCode, #[source] reqwest::Error), #[error("Server returned status code {0}. Server says: {1}")] Status(StatusCode, String), + #[error("Server returned status code {0}. {1}")] + StatusProblemDetails(StatusCode, String), #[error( "POST requests are not supported by the endpoint, are you using the simple index URL instead of the upload URL?" )] @@ -1450,6 +1452,18 @@ async fn handle_response(registry: &Url, response: Response) -> Result<(), Publi )); } + // Try to parse as RFC 9457 Problem Details (e.g., from pyx). + if content_type.as_deref() == Some(uv_client::ProblemDetails::CONTENT_TYPE) + && let Some(problem) = + uv_client::ProblemDetails::try_from_response_body(upload_error.as_bytes()) + && let Some(description) = problem.description() + { + return Err(PublishSendError::StatusProblemDetails( + status_code, + description, + )); + } + // Raced uploads of the same file are handled by the caller. Err(PublishSendError::Status( status_code, @@ -2172,4 +2186,73 @@ mod tests { " ); } + + /// PyPI returns `application/json` with a `code` field. + #[tokio::test] + async fn upload_error_pypi_json() { + let mock_server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/final")) + .respond_with( + ResponseTemplate::new(400) + .insert_header("content-type", "application/json") + .set_body_raw( + r#"{"message": "The server could not comply with the request since it is either malformed or otherwise incorrect.\n\n\nError: Use 'source' as Python version for an sdist.\n\n", "code": "400 Error: Use 'source' as Python version for an sdist.", "title": "Bad Request"}"#, + "application/json", + ), + ) + .mount(&mock_server) + .await; + + let err = mock_server_upload(&mock_server).await.unwrap_err(); + + let mut capture = String::new(); + write_error_chain(&err, &mut capture, "error", AnsiColors::Red).unwrap(); + + let capture = capture.replace(&mock_server.uri(), "[SERVER]"); + let capture = anstream::adapter::strip_str(&capture); + assert_snapshot!( + &capture, + @" + error: Failed to publish `../../test/links/tqdm-4.66.1-py3-none-manylinux_2_12_x86_64.manylinux2010_x86_64.musllinux_1_1_x86_64.whl` to [SERVER]/final + Caused by: Server returned status code 400 Bad Request. Server says: 400 Error: Use 'source' as Python version for an sdist. + " + ); + } + + /// pyx returns `application/problem+json` with RFC 9457 Problem Details. + #[tokio::test] + async fn upload_error_problem_details() { + let mock_server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/final")) + .respond_with( + ResponseTemplate::new(400) + .insert_header( + "content-type", + uv_client::ProblemDetails::CONTENT_TYPE, + ) + .set_body_raw( + r#"{"type": "about:blank", "status": 400, "title": "Bad Request", "detail": "Missing required field `name`"}"#, + uv_client::ProblemDetails::CONTENT_TYPE, + ), + ) + .mount(&mock_server) + .await; + + let err = mock_server_upload(&mock_server).await.unwrap_err(); + + let mut capture = String::new(); + write_error_chain(&err, &mut capture, "error", AnsiColors::Red).unwrap(); + + let capture = capture.replace(&mock_server.uri(), "[SERVER]"); + let capture = anstream::adapter::strip_str(&capture); + assert_snapshot!( + &capture, + @" + error: Failed to publish `../../test/links/tqdm-4.66.1-py3-none-manylinux_2_12_x86_64.manylinux2010_x86_64.musllinux_1_1_x86_64.whl` to [SERVER]/final + Caused by: Server returned status code 400 Bad Request. Server message: Bad Request, Missing required field `name` + " + ); + } } diff --git a/crates/uv/tests/it/publish.rs b/crates/uv/tests/it/publish.rs index e2a18b76237bd..adb4775266af2 100644 --- a/crates/uv/tests/it/publish.rs +++ b/crates/uv/tests/it/publish.rs @@ -612,3 +612,75 @@ async fn gitlab_trusted_publishing_testpypi_id_token() { " ); } + +/// PyPI returns `application/json` errors with a `code` field. +#[tokio::test] +async fn upload_error_pypi_json() { + let context = TestContext::new("3.12"); + let server = MockServer::start().await; + + Mock::given(method("POST")) + .and(path("/upload")) + .respond_with(ResponseTemplate::new(400).set_body_raw( + r#"{"message": "Error", "code": "400 Use 'source' as Python version for an sdist.", "title": "Bad Request"}"#, + "application/json", + )) + .mount(&server) + .await; + + uv_snapshot!(context.filters(), context.publish() + .arg("-u") + .arg("dummy") + .arg("-p") + .arg("dummy") + .arg("--publish-url") + .arg(format!("{}/upload", server.uri())) + .arg(dummy_wheel()), @" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + Publishing 1 file to http://[LOCALHOST]/upload + Uploading ok-1.0.0-py3-none-any.whl ([SIZE]) + error: Failed to publish `[WORKSPACE]/test/links/ok-1.0.0-py3-none-any.whl` to http://[LOCALHOST]/upload + Caused by: Server returned status code 400 Bad Request. Server says: 400 Use 'source' as Python version for an sdist. + " + ); +} + +/// pyx returns `application/problem+json` errors with RFC 9457 Problem Details. +#[tokio::test] +async fn upload_error_problem_details() { + let context = TestContext::new("3.12"); + let server = MockServer::start().await; + + Mock::given(method("POST")) + .and(path("/upload")) + .respond_with(ResponseTemplate::new(400).set_body_raw( + r#"{"type": "about:blank", "status": 400, "title": "Bad Request", "detail": "Missing required field `name`"}"#, + "application/problem+json", + )) + .mount(&server) + .await; + + uv_snapshot!(context.filters(), context.publish() + .arg("-u") + .arg("dummy") + .arg("-p") + .arg("dummy") + .arg("--publish-url") + .arg(format!("{}/upload", server.uri())) + .arg(dummy_wheel()), @" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + Publishing 1 file to http://[LOCALHOST]/upload + Uploading ok-1.0.0-py3-none-any.whl ([SIZE]) + error: Failed to publish `[WORKSPACE]/test/links/ok-1.0.0-py3-none-any.whl` to http://[LOCALHOST]/upload + Caused by: Server returned status code 400 Bad Request. Server message: Bad Request, Missing required field `name` + " + ); +}