fix(sentry): drop expected upstream provider errors - #809
Open
sjawhar wants to merge 5 commits into
Open
Conversation
The Sentry Anthropic integration captures provider throttling and availability errors (429, 529, 5xx) unhandled at the SDK call site, before the pipeline converts them into a BackendAPIError response for the client. Those are the backend telling us to slow down, not proxy defects, and they bury real failures: 56 unhandled 429 events in three days in our deployment. _sentry_before_send now drops them, matching on the status code rather than the exception class. Client errors such as 400 still report.
Broadens _EXPECTED_UPSTREAM_STATUS_CODES from throttling/availability codes
(408/429/500/502/503/504/529) to also include 400, 404, and 401.
These are still the Anthropic SDK auto-capturing at the call site before our
handler runs, and the pipeline already treats every AnthropicStatusError
identically regardless of status code (_handle_anthropic_error /
_build_error_event log at warning and convert to BackendAPIError) — the
proxy is a transparent passthrough here, not the cause of the rejection:
- LUTHIEN-6 (1,080 events): a client sent message content with a field
Anthropic's schema rejects (400).
- LUTHIEN-2 (125 events): a client requested a model name Anthropic
doesn't have (404).
- LUTHIEN-D (28 events): a client-supplied bearer token Anthropic itself
rejected as invalid (401) — the proxy forwarded the credential
unchanged, it did not mint it.
A status code outside this set (e.g. 403) still reports, so a genuinely new
upstream failure mode stays visible rather than being silently swallowed.
No behavior change to proxying itself — this only changes what Sentry
captures.
legion-implementer Bot
pushed a commit
to trajectory-labs-pbc/luthien-proxy
that referenced
this pull request
Aug 29, 2026
Policy hooks can mutate or replace the outgoing request, and UPSTREAM_HEADERS/policy-context injection can alter it before it reaches Anthropic, so a proxy/policy-generated invalid request rejected by Anthropic with 400/401/404 was being silently dropped from Sentry as an 'expected' upstream error. Tag the Sentry scope (PASSTHROUGH_TAG) at the actual upstream call boundary in _AnthropicPolicyIO with whether the request is proven unmodified (no policy-hook mutation/replacement, no header/context injection), and gate the 400/401/404 drop in _sentry_before_send on that tag. 429/408/5xx/529 remain dropped unconditionally (provider-side by definition). Addresses thermonuclear-deep-review consensus-High finding on PR LuthienResearch#809.
legion-implementer
Bot
force-pushed
the
fix/sentry-expected-upstream-errors
branch
from
August 29, 2026 23:25
2b311f8 to
e538774
Compare
Policy hooks can mutate or replace the outgoing request, and UPSTREAM_HEADERS/policy-context injection can alter it before it reaches Anthropic, so a proxy/policy-generated invalid request rejected by Anthropic with 400/401/404 was being silently dropped from Sentry as an 'expected' upstream error. Tag the Sentry scope (PASSTHROUGH_TAG) at the actual upstream call boundary in _AnthropicPolicyIO with whether the request is proven unmodified (no policy-hook mutation/replacement, no header/context injection), and gate the 400/401/404 drop in _sentry_before_send on that tag. 429/408/5xx/529 remain dropped unconditionally (provider-side by definition). Addresses thermonuclear-deep-review consensus-High finding on PR LuthienResearch#809.
legion-implementer
Bot
force-pushed
the
fix/sentry-expected-upstream-errors
branch
from
August 30, 2026 00:11
e538774 to
7fcb643
Compare
PASSTHROUGH_TAG previously only proved the request body/headers were untouched, and gated 400/401/404 alike. In client-key auth mode, resolve_anthropic_client forwards the operator's shared ANTHROPIC_API_KEY (user_credential=None) instead of anything the client sent — an unmodified body there proves nothing about whose credential caused a 401, so an invalid operator credential could be silently dropped as an 'expected client fault'. Add a separate CREDENTIAL_PASSTHROUGH_TAG (true when user_credential is not None) tagged alongside PASSTHROUGH_TAG at the same upstream call boundary. _is_expected_upstream_error now requires only PASSTHROUGH_TAG to drop 400/404 (content-driven, credential- independent — an unmodified body proves the client caused these regardless of auth mode) and BOTH tags to drop a 401 (credential- driven). Folding credential provenance into a single combined bool would have also stopped dropping 400/404 in client-key mode, which was PR LuthienResearch#809's original Sentry-noise target.
legion-implementer
Bot
force-pushed
the
fix/sentry-expected-upstream-errors
branch
from
August 30, 2026 00:20
7fcb643 to
7363c63
Compare
…atus codes Thermonuclear pass-3 finding: the example-based tests for _is_expected_upstream_error only covered 429/529 of the seven provider-side status codes and scattered named cases for 400/404/401, so silently dropping a status out of _PROVIDER_SIDE_STATUS_CODES (or adding one without a test) wouldn't fail anything. Adds three parametrized test classes driven directly from the production status-code sets (_PROVIDER_SIDE_STATUS_CODES, _CONTENT_DEPENDENT_STATUS_CODES, _CREDENTIAL_DEPENDENT_STATUS_CODES): every provider-side status with no tags, 400/404 across all 9 passthrough x credential tag-state combinations (pinning that the credential tag is irrelevant for them), and 401 across all 9 combinations (only both-True drops). The existing named regression tests that pin specific incidents (client-key-mode 401, proxy-modified 400) are kept unchanged. Also adds _REQUIRED_TAGS_BY_STATUS, a dict built from those same sets mapping each status to the tags it requires, so the decision table is discoverable directly in the source (the reviewer's optional suggestion) rather than only in a docstring. _is_expected_upstream_error now does a single lookup + all() instead of three branches; the mapping's construction from the existing sets keeps one source of truth per category. No behavior change.
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.
Problem
With Sentry enabled, every provider rate-limit and overload response becomes an unhandled Sentry error. In our production deployment
RateLimitError: Error code: 429accumulated 56 unhandled events in three days, alongside529 overloaded_error, and it sits at the top of the issue stream above defects that are actually ours.These are not proxy defects. A 429 is the backend applying backpressure, and the pipeline already handles it deliberately:
_handle_anthropic_errorconvertsAnthropicStatusErrorinto aBackendAPIError,backend_api_error_handlerreturns it to the client in Anthropic error format, and the caller retries.Root cause
The Sentry SDK's Anthropic integration is auto-enabled and captures at the SDK call site, so the event is created inside
anthropic/_base_client.py:requestwithmechanism: {"type": "anthropic", "handled": false}before our handler ever sees the exception. Nothing downstream can un-capture it.Fix
_sentry_before_senddropsAPIStatusErrorwhosestatus_codeis one of 408, 429, 500, 502, 503, 504, 529 — throttling and availability, the cases the pipeline converts and the caller retries. The check is on the status code rather than the exception class so that an SDK renaming or adding a status subclass cannot silently start reporting again.Verification
test_sentry_scrubbing.pybuild realAPIStatusErrors fromhttpxresponses: 429 and 529 are dropped, 400 is kept, and an ordinaryTypeErrorfrom our own code is kept. The two drop tests fail before this change and pass after; the two keep tests pass throughout, so they pin the behavior that must not regress.uv run pytest tests/luthien_proxy/unit_tests/test_sentry_scrubbing.pypasses at this commit (49 passed).dev_checks.sh: shellcheck, ruff format, ruff lint, ruff docstrings, and pyright all clean. The pytest stage shows the same three machine-local failures documented in fix(sentry): never capture opentelemetry.context detach noise #807 and fix(pipeline): serialize stream events with model_dump(mode="json") #808 (test_onboard.py::test_find_docker_ports_respects_env_vars, two intest_config_registry.py); all three pass when run withGATEWAY_PORT/POSTGRES_PORT/REDIS_PORTunset, so they are this box's environment rather than this change.Update: also dropping 400/404/401
This PR originally kept 4xx codes other than 429 on the theory that a 400 means an actionable malformed request. Three more Sentry issues showed that isn't the whole picture — the pipeline already treats every
AnthropicStatusErroridentically regardless of status code (_handle_anthropic_error/_build_error_eventboth log at warning and convert toBackendAPIErrorfor any status code), and these three are the provider legitimately rejecting something the client sent, not something the proxy built:400, e.g.messages.297.content.1.text.parsed_output: Extra inputs are not permitted). Confirmedparsed_outputis not a field this proxy or any policy injects — it's client-originated.404, e.g.model: opus-4.6).401) — passthrough-credential mode forwards the client's own token unchanged; the proxy didn't mint it._EXPECTED_UPSTREAM_STATUS_CODESnow includes 400, 401, 404 alongside the original throttling/availability codes. Three new drop-tests (test_drops_upstream_bad_request_error,_not_found_error,_authentication_error) plus a new keep-test for a status code outside the set (test_keeps_upstream_status_code_outside_expected_set, using 403) so a genuinely new upstream failure mode stays visible.Silences: LUTHIEN-6, LUTHIEN-2, LUTHIEN-D (new), plus the original LUTHIEN-8/rate-limit and overload noise this PR already covered.