Skip to content

feat(core)!: preserve response headers on non-success HTTP errors - #2211

Open
mydreamdoctor wants to merge 1 commit into
0xPlaygrounds:mainfrom
mydreamdoctor:feat/preserve-error-response-headers
Open

feat(core)!: preserve response headers on non-success HTTP errors#2211
mydreamdoctor wants to merge 1 commit into
0xPlaygrounds:mainfrom
mydreamdoctor:feat/preserve-error-response-headers

Conversation

@mydreamdoctor

@mydreamdoctor mydreamdoctor commented Jul 25, 2026

Copy link
Copy Markdown

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 response
headers are discarded at error construction: non_success_status_error()
reads only .status() and .text(), and
http_client::Error::InvalidStatusCodeWithMessage has no slot for them. That
makes rate-limit metadata — most importantly Retry-After on a 429, but also
x-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 as
CompletionError::HttpError(InvalidStatusCodeWithMessage(429, body)) with
provider_response_status() == Some(429) and the body intact, but there is no
route to the Retry-After value — it is gone before the error reaches caller
code. Notably, Rig's own http_client::retry::RetryPolicy::retry(&self, error: &Error, ..) receives the error and therefore also cannot honor
Retry-After today.

This PR captures the response HeaderMap wherever a non-success status error
is built from a live response, and exposes it through the same inspection
surface #1859 established:

  • http_client::Error::InvalidStatusCodeWithMessage gains a third
    Option<Box<HeaderMap>> field (boxed to keep the error size unchanged for
    clippy::result_large_err) and a public non_success_headers() accessor.
  • The bundled reqwest-based clients populate it in non_success_status_error()
    (both the unary and streaming paths); the generic client verify() error
    branches populate it from the response in hand.
  • Every capability error (CompletionError, EmbeddingError,
    TranscriptionError, VerifyError, RerankError, ImageGenerationError,
    AudioGenerationError) gains provider_response_headers() beside the
    existing provider_response_{status,body,json} helpers, via
    impl_provider_response_helpers!.
  • rig-agent's PromptError and StructuredOutputError forward
    provider_response_headers() like the other three helpers.

from_http_response(status, body) intentionally keeps its signature: funnels
that only ever had a status and body construct the error with headers: None,
and the accessor documents that None means "not captured", mirroring
#1859's Option<StatusCode> convention. Broadening capture to the 2xx
error-envelope path (ProviderResponse) is left as a follow-up in the spirit
of #1931 — the helpers' contract already accommodates it.

Fixes #2210

Type of change

  • New feature
  • Breaking change (mechanical: one variant gains a field, see Notes)

Testing

Original validation (upstream main @ 13bfadd):

  • cargo test -p rig-core --all-features --lib986 passed; 0 failed; 3 ignored
  • cargo test -p rig-agent --all-features485 passed; 0 failed; 2 ignored (487 collected)
  • cargo clippy -p rig-core -p rig-agent --all-features --all-targets0 warnings (identical to a clean main baseline; the header field is boxed specifically so result_large_err stays quiet)
  • cargo fmt --all --check — clean
  • cargo doc -p rig-core -p rig-agent --all-features --no-deps — no new warnings

Re-validated after rebase onto main @ 7f1a495: patch applies cleanly;
cargo test -p rig-core1000 passed; 0 failed. (rig-agent's test build
currently fails to compile for us on a clean 7f1a495 checkout — 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::Response carrying
    Retry-After: 20 / x-ratelimit-remaining: 0 on a 429 and asserts status,
    body, and both headers survive.
  • http_client::tests::non_success_headers_absent_when_not_captured
    None for errors built without a response.
  • provider_response assert_funnel! extended for every capability
    error: from_http_response yields provider_response_headers() == None
    (funnels cannot invent headers), while an HttpError built with captured
    headers surfaces Retry-After alongside the preserved 429 status and body.
  • rig-agent:
    prompt_error_provider_response_helpers_forward_captured_headers — forwarding
    through PromptError and StructuredOutputError.

Checklist:

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (doc comments on the new accessors; happy to add a MIGRATING.md note if wanted)
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Notes

Breaking surface (mechanical). InvalidStatusCodeWithMessage is a tuple
variant with public fields, so adding the headers slot is source-breaking for
code that constructs or exhaustively matches it:

// before
Error::InvalidStatusCodeWithMessage(status, message)
// after
Error::InvalidStatusCodeWithMessage(status, message, None)          // construct
Error::InvalidStatusCodeWithMessage(status, message, _) => ...      // match

All in-tree sites (2 match arms, ~15 constructions, mostly tests) are updated
in this PR. Users who only consume errors through Display or the
provider_response_* helpers are unaffected. If you'd rather avoid the break,
the variant could instead become a #[non_exhaustive] struct variant with
constructors — larger diff, future-proof against the next field; happy to
rework it that way if that's the preferred direction.

Why Option<Box<HeaderMap>>: Option distinguishes "not captured" (e.g.
built via from_http_response, which never sees headers) from "captured and
empty"; the Box keeps http_client::Error at its previous size so the
crate-wide clippy::result_large_err profile stays clean (verified: 0 clippy
warnings, same as main).

Scope notes / possible follow-ups:

  • The 2xx error-envelope path (ProviderResponse variant) still reports
    provider_response_headers() == None; provider call sites hold the response
    there, 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.
  • The SSE EventSource connect path still uses InvalidStatusCode (status
    only, no body or headers) — untouched here.
  • A convenience retry_after() -> Option<Duration> parser was deliberately
    left out to keep this minimal; RetryPolicy implementors can read the raw
    header value via non_success_headers().

🤖 Generated with Claude Code

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Response headers (incl. Retry-After) are discarded when non-success HTTP errors are constructed

1 participant