Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 2 additions & 42 deletions crates/uv-client/src/cached_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<ProblemDetails> {
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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
Expand Down
53 changes: 53 additions & 0 deletions crates/uv-client/src/error.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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<Self> {
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<Self> {
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<String> {
match self {
Expand Down Expand Up @@ -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`"
);
}
}
2 changes: 1 addition & 1 deletion crates/uv-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down
83 changes: 83 additions & 0 deletions crates/uv-publish/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Comment on lines 131 to +134
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ProblemDetails message already includes a "Server says" style prefix.

#[error(
"POST requests are not supported by the endpoint, are you using the simple index URL instead of the upload URL?"
)]
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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`
"
);
}
}
72 changes: 72 additions & 0 deletions crates/uv/tests/it/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`
"
);
}
Loading