feat(core)!: preserve response headers on non-success HTTP errors - #2211
Open
mydreamdoctor wants to merge 1 commit into
Open
feat(core)!: preserve response headers on non-success HTTP errors#2211mydreamdoctor wants to merge 1 commit into
mydreamdoctor wants to merge 1 commit into
Conversation
Capture the response HeaderMap when a non-success status error is
constructed from a live HTTP response, so rate-limit metadata such as
Retry-After stays recoverable by callers' backoff logic.
- http_client::Error::InvalidStatusCodeWithMessage gains an
Option<HeaderMap> third field and a non_success_headers() accessor.
- The bundled reqwest-based clients populate it in
non_success_status_error(); the client verify() paths populate it from
the response in hand.
- Capability errors gain provider_response_headers() beside the existing
provider_response_{status,body,json} helpers; rig-agent's PromptError
and StructuredOutputError forward it.
BREAKING CHANGE: matches on InvalidStatusCodeWithMessage need a third
pattern field; constructions need an Option<HeaderMap> argument.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
When a provider returns a non-success HTTP status, Rig preserves the status and
the raw body (via #1859's
provider_response_*helpers), but the responseheaders are discarded at error construction:
non_success_status_error()reads only
.status()and.text(), andhttp_client::Error::InvalidStatusCodeWithMessagehas no slot for them. Thatmakes rate-limit metadata — most importantly
Retry-Afteron a 429, but alsox-ratelimit-*— structurally unrecoverable by the caller.We hit this while wiring provider-aware backoff: a stub server returning
429+Retry-After: 20+ a JSON error body surfaces asCompletionError::HttpError(InvalidStatusCodeWithMessage(429, body))withprovider_response_status() == Some(429)and the body intact, but there is noroute to the
Retry-Aftervalue — it is gone before the error reaches callercode. Notably, Rig's own
http_client::retry::RetryPolicy::retry(&self, error: &Error, ..)receives the error and therefore also cannot honorRetry-Aftertoday.This PR captures the response
HeaderMapwherever a non-success status erroris built from a live response, and exposes it through the same inspection
surface #1859 established:
http_client::Error::InvalidStatusCodeWithMessagegains a thirdOption<Box<HeaderMap>>field (boxed to keep the error size unchanged forclippy::result_large_err) and a publicnon_success_headers()accessor.non_success_status_error()(both the unary and streaming paths); the generic client
verify()errorbranches populate it from the response in hand.
CompletionError,EmbeddingError,TranscriptionError,VerifyError,RerankError,ImageGenerationError,AudioGenerationError) gainsprovider_response_headers()beside theexisting
provider_response_{status,body,json}helpers, viaimpl_provider_response_helpers!.rig-agent'sPromptErrorandStructuredOutputErrorforwardprovider_response_headers()like the other three helpers.from_http_response(status, body)intentionally keeps its signature: funnelsthat only ever had a status and body construct the error with
headers: None,and the accessor documents that
Nonemeans "not captured", mirroring#1859's
Option<StatusCode>convention. Broadening capture to the 2xxerror-envelope path (
ProviderResponse) is left as a follow-up in the spiritof #1931 — the helpers' contract already accommodates it.
Fixes #2210
Type of change
Testing
Original validation (upstream
main@13bfadd):cargo test -p rig-core --all-features --lib— 986 passed; 0 failed; 3 ignoredcargo test -p rig-agent --all-features— 485 passed; 0 failed; 2 ignored (487 collected)cargo clippy -p rig-core -p rig-agent --all-features --all-targets— 0 warnings (identical to a cleanmainbaseline; the header field is boxed specifically soresult_large_errstays quiet)cargo fmt --all --check— cleancargo doc -p rig-core -p rig-agent --all-features --no-deps— no new warningsRe-validated after rebase onto
main@7f1a495: patch applies cleanly;cargo test -p rig-core— 1000 passed; 0 failed. (rig-agent's test buildcurrently fails to compile for us on a clean
7f1a495checkout — E0282/E0432,identical with and without this patch, so it appears unrelated to this change;
happy to re-run once that builds again in case it's environmental.)
New tests:
http_client::tests::non_success_status_error_preserves_response_headers—drives the real construction path with a
reqwest::ResponsecarryingRetry-After: 20/x-ratelimit-remaining: 0on a 429 and asserts status,body, and both headers survive.
http_client::tests::non_success_headers_absent_when_not_captured—Nonefor errors built without a response.provider_responseassert_funnel!extended for every capabilityerror:
from_http_responseyieldsprovider_response_headers() == None(funnels cannot invent headers), while an
HttpErrorbuilt with capturedheaders surfaces
Retry-Afteralongside the preserved 429 status and body.rig-agent:prompt_error_provider_response_helpers_forward_captured_headers— forwardingthrough
PromptErrorandStructuredOutputError.Checklist:
Notes
Breaking surface (mechanical).
InvalidStatusCodeWithMessageis a tuplevariant with public fields, so adding the headers slot is source-breaking for
code that constructs or exhaustively matches it:
All in-tree sites (2 match arms, ~15 constructions, mostly tests) are updated
in this PR. Users who only consume errors through
Displayor theprovider_response_*helpers are unaffected. If you'd rather avoid the break,the variant could instead become a
#[non_exhaustive]struct variant withconstructors — larger diff, future-proof against the next field; happy to
rework it that way if that's the preferred direction.
Why
Option<Box<HeaderMap>>:Optiondistinguishes "not captured" (e.g.built via
from_http_response, which never sees headers) from "captured andempty"; the
Boxkeepshttp_client::Errorat its previous size so thecrate-wide
clippy::result_large_errprofile stays clean (verified: 0 clippywarnings, same as
main).Scope notes / possible follow-ups:
ProviderResponsevariant) still reportsprovider_response_headers() == None; provider call sites hold the responsethere, so threading headers through is a mechanical broadening in the spirit
of Follow-up to #1859: broaden provider error-response inspection coverage + guard against regressions #1931.
EventSourceconnect path still usesInvalidStatusCode(statusonly, no body or headers) — untouched here.
retry_after() -> Option<Duration>parser was deliberatelyleft out to keep this minimal;
RetryPolicyimplementors can read the rawheader value via
non_success_headers().🤖 Generated with Claude Code