Implement RFC9457 compliant messaging #16199

doddi · 2025-10-09T08:04:09Z

Summary

HTTP1.1 RFC 9112 - HTTP/1.1 section 4 defines the response status code to optionally include a text description (human readable) of the reason for the status code.

RFC9113 - HTTP/2 is the HTTP2 protocol standard and the response status only considers the status code and not the reason phrase, and as such important information can be lost in helping the client determine a route cause of a failure.

As per discussion on this PR the current feeling is that implementing the RFC9457 standard might be the preferred route. This PR makes those changes to aid the discussion which has also been moved to the PEP board

Test Plan

Pulling components that violate our policy over HTTP2 and without any RFC9457 implementation the following message is presented to the user:

With the RFC9457 standard implemented, below you can see the advantage in the extra context as to why the component has been blocked:

zanieb · 2025-10-09T13:42:12Z

Cool!

cc @woodruffw

woodruffw

Thanks @doddi! I did a review pass mostly based on my read of the RFC; I believe @konstin will do one more for uv's idioms (in which case any of his feedback on idioms overrides mine) 🙂

woodruffw · 2025-10-10T17:34:13Z

crates/uv-client/src/cached_client.rs

+    // Try to read the response body and parse it as problem details
+    match response.bytes().await {
+        Ok(bytes) => ProblemDetails::from_json(&bytes).ok(),
+        Err(_) => None,


Thinking out loud: is ignoring a malformed details response the right thing here? I think it is, but maybe it makes sense to emit a warning? I could see it being useful to notify upstream servers that they're emitting a malformed payload.

I probably wouldn't add a "warning" since it's not something the user can fix, but it might be worth logging.

woodruffw · 2025-10-10T17:36:28Z

crates/uv-client/src/cached_client.rs

+                .headers()
+                .get("content-type")
+                .and_then(|ct| ct.to_str().ok())
+                .map(|ct| ct.starts_with("application/problem+json"))


Should this be an equality check rather than starts_with? RFC 9457 says that problem detail responses are identified by exactly the application/problem+json content-type, so we probably shouldn't accept application/problem+json+abc or similar.

woodruffw · 2025-10-10T17:39:02Z

crates/uv-client/src/cached_client.rs

+    if !content_type.starts_with("application/problem+json") {
+        return None;
+    }


IMO it's okay to remove this check, since we do it at the callsite. We can document that a caller-upheld invariant of this API is that the content-type should be right.

(Alternatively we could keep this check, but remove it from the callsite. But that might be a little harder with the lifetimes/consumption.)

woodruffw · 2025-10-10T17:40:23Z

crates/uv-client/src/error.rs

+    /// Parse Problem Details from JSON bytes
+    pub fn from_json(bytes: &[u8]) -> Result<Self, serde_json::Error> {
+        serde_json::from_slice(bytes)
+    }


My 0.02c would be for removing this, and just calling serde_json::from_slice directly wherever we use it 🙂

woodruffw · 2025-10-10T17:41:57Z

crates/uv-client/src/error.rs

+    pub extensions: std::collections::HashMap<String, serde_json::Value>,
+}
+
+/// Default problem type URI as per RFC 9457


Nitpick, nonblocking: serde defaults sometimes benefit from inlining:

Suggested change

/// Default problem type URI as per RFC 9457

/// Default problem type URI as per RFC 9457

#[inline]

woodruffw · 2025-10-10T17:44:36Z

crates/uv-client/src/error.rs

+    /// Additional problem-specific extension fields
+    #[serde(flatten)]
+    pub extensions: std::collections::HashMap<String, serde_json::Value>,


serde will ignore unknown fields by default, so my 0.02c would be for removing this until we actually need to process extension fields. It'll also make the parsing slightly faster (not that that matters much on the error path):

Suggested change

/// Additional problem-specific extension fields

#[serde(flatten)]

pub extensions: std::collections::HashMap<String, serde_json::Value>,

(The tests should confirm that this works 🙂)

woodruffw · 2025-10-10T17:51:29Z

crates/uv-client/src/error.rs

+        if let Some(detail) = &self.detail {
+            detail.clone()
+        } else if let Some(title) = &self.title {
+            title.clone()


Should we render both of these, if present? I can imagine {title}: {detail} being useful.

woodruffw · 2025-10-10T17:52:36Z

crates/uv-client/src/error.rs

+        } else if let Some(title) = &self.title {
+            title.clone()
+        } else {
+            format!("HTTP error {}", self.status.unwrap_or(0))


I think we want to specialize here too -- if status isn't given we should probably take it from the parent request, or maybe we could say "unknown error (server provided no information)" and rely on the error layer above us rendering the status code.

konstin

Can you add a test that shows the error message in context? index_has_no_requires_python and the network.rs have reference structures.

konstin · 2025-10-14T11:04:06Z

crates/uv-client/src/error.rs

+            }
+            _ => {
+                // If no detail, title, or status is provided, return a generic message
+                "An error occurred".to_string()


We should return None for this case and fall back to the default case in WrappedReqwestError instead of using our own string.

konstin · 2025-10-14T11:08:41Z

crates/uv-client/src/cached_client.rs

+    if let Ok(bytes) = response.bytes().await {
+        serde_json::from_slice(&bytes).ok()
+    } else {
+        warn!("Failed to read response body for problem details");


We should show the inner error in this warning (at least for serde), so it's clear why e.g. the data didn't match the schema.

konstin · 2025-10-14T11:17:53Z

crates/uv-client/src/error.rs

            f.write_str("Could not connect, are you offline?")
+        } else if let Some(problem_details) = &self.problem_details {
+            // Show problem details if available
+            write!(f, "{}", problem_details.description())


Here we need to add something like Server says: so it's clear that that's from the remote and not uv

konstin · 2025-10-14T15:56:40Z

For the clippy failure, we can box the large variant.

doddi · 2025-10-16T09:33:19Z

Add an it test

936ff82

For the clippy failure, we can box the large variant.

7ed1074 although I see the CI still failing for clippy error on windows for a different area. Can someone help me understand this?

konstin · 2025-10-16T09:48:23Z

Some OS types are larger on Windows than on Unix, which can push the error type size over the clippy threshold just for Windows. You can run Windows clippy locally with cargo-xwin and cargo xwin clippy, but usually just Boxing the struct that grew in the change in an error enum and rerunning CI does the trick.

konstin · 2025-10-16T19:09:59Z

I fixed the linter errors, I have experience with them.

konstin

Thank you!

konstin · 2025-10-16T19:42:11Z

I've changed the boxing to a different location because is_transient_network_error relies on having specific error types in the error chains and it considers Boxed type different from the base type there.

doddi · 2025-10-16T19:43:22Z

I've changed the boxing to a different location because is_transient_network_error relies on having specific error types in the error chains and it considers Boxed type different from the base type there.

Thank you @konstin, I was struggling to understand this error being new to the code base

woodruffw · 2025-10-16T19:54:36Z

Thanks @doddi!

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [astral-sh/uv](https://github.com/astral-sh/uv) | patch | `0.9.3` -> `0.9.5` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>astral-sh/uv (astral-sh/uv)</summary> ### [`v0.9.5`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#095) [Compare Source](astral-sh/uv@0.9.4...0.9.5) Released on 2025-10-21. This release contains an upgrade to `astral-tokio-tar`, which addresses a vulnerability in tar extraction on malformed archives with mismatching size information between the ustar header and PAX extensions. While the `astral-tokio-tar` advisory has been graded as "high" due its potential broader impact, the *specific* impact to uv is **low** due to a lack of novel attacker capability. Specifically, uv only processes tar archives from source distributions, which already possess the capability for full arbitrary code execution by design, meaning that an attacker gains no additional capabilities through `astral-tokio-tar`. Regardless, we take the hypothetical risk of parser differentials very seriously. Out of an abundance of caution, we have assigned this upgrade an advisory: <GHSA-w476-p2h3-79g9> ##### Security - Upgrade `astral-tokio-tar` to 0.5.6 to address a parsing differential ([#16387](astral-sh/uv#16387)) ##### Enhancements - Add required environment marker example to hint ([#16244](astral-sh/uv#16244)) - Fix typo in MissingTopLevel warning ([#16351](astral-sh/uv#16351)) - Improve 403 Forbidden error message to indicate package may not exist ([#16353](astral-sh/uv#16353)) - Add a hint on `uv pip install` failure if the `--system` flag is used to select an externally managed interpreter ([#16318](astral-sh/uv#16318)) ##### Bug fixes - Fix backtick escaping for PowerShell ([#16307](astral-sh/uv#16307)) ##### Documentation - Document metadata consistency expectation ([#15683](astral-sh/uv#15683)) - Remove outdated aarch64 musl note ([#16385](astral-sh/uv#16385)) ### [`v0.9.4`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#094) [Compare Source](astral-sh/uv@0.9.3...0.9.4) Released on 2025-10-17. ##### Enhancements - Add CUDA 13.0 support ([#16321](astral-sh/uv#16321)) - Add auto-detection for Intel GPU on Windows ([#16280](astral-sh/uv#16280)) - Implement display of RFC 9457 HTTP error contexts ([#16199](astral-sh/uv#16199)) ##### Bug fixes - Avoid obfuscating pyx tokens in `uv auth token` output ([#16345](astral-sh/uv#16345)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

doddi added 2 commits October 9, 2025 08:51

Add RFC9457 compliant messaging

c9d5ceb

cargo formatting

ec19910

zanieb requested a review from konstin October 9, 2025 13:43

woodruffw self-requested a review October 9, 2025 13:44

woodruffw reviewed Oct 10, 2025

View reviewed changes

konstin added the enhancement New feature or improvement to existing functionality label Oct 13, 2025

doddi added 9 commits October 14, 2025 10:07

Add logging to failed parsing of problem detail

5a3a59d

Test for exact match of 'application/problem+json'

63175fb

Remove redundant content-type check

bab16f2

unwrap serde conversion

c1c453c

inline macro

0ab6ed1

ignore extensions

9f252c4

Update error messaging description

550a3c4

formatting

192b066

clippy

2713cc7

konstin reviewed Oct 14, 2025

View reviewed changes

extra logging and clearer errors

5f01168

Add an it test

936ff82

Boxing to fix the clippy error (large variant)

b9175d5

doddi force-pushed the RFC9457-problem-messages branch from 7ed1074 to b9175d5 Compare October 16, 2025 15:44

Fix linter problems

e9b6067

konstin approved these changes Oct 16, 2025

View reviewed changes

Box in a different place to not break is_transient_network_error

02820c8

konstin enabled auto-merge (squash) October 16, 2025 19:42

konstin merged commit c12e8bb into astral-sh:main Oct 16, 2025
99 checks passed

BrewTestBot mentioned this pull request Oct 18, 2025

uv 0.9.4 Homebrew/homebrew-core#250214

Merged

	/// Default problem type URI as per RFC 9457
	/// Default problem type URI as per RFC 9457
	#[inline]

	/// Additional problem-specific extension fields
	#[serde(flatten)]
	pub extensions: std::collections::HashMap<String, serde_json::Value>,

Implement RFC9457 compliant messaging #16199

Implement RFC9457 compliant messaging #16199

Conversation

doddi commented Oct 9, 2025

Summary

Test Plan

Uh oh!

zanieb commented Oct 9, 2025

Uh oh!

woodruffw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

konstin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

konstin Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

konstin Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

konstin commented Oct 14, 2025

Uh oh!

doddi commented Oct 16, 2025

Uh oh!

konstin commented Oct 16, 2025

Uh oh!

konstin commented Oct 16, 2025

Uh oh!

konstin left a comment

Choose a reason for hiding this comment

Uh oh!

konstin commented Oct 16, 2025

Uh oh!

doddi commented Oct 16, 2025

Uh oh!

konstin Oct 14, 2025 •

edited

Loading

konstin Oct 14, 2025 •

edited

Loading