Skip to content

fix(sentry): drop expected upstream provider errors - #809

Open
sjawhar wants to merge 5 commits into
LuthienResearch:mainfrom
trajectory-labs-pbc:fix/sentry-expected-upstream-errors
Open

fix(sentry): drop expected upstream provider errors#809
sjawhar wants to merge 5 commits into
LuthienResearch:mainfrom
trajectory-labs-pbc:fix/sentry-expected-upstream-errors

Conversation

@sjawhar

@sjawhar sjawhar commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

Problem

With Sentry enabled, every provider rate-limit and overload response becomes an unhandled Sentry error. In our production deployment RateLimitError: Error code: 429 accumulated 56 unhandled events in three days, alongside 529 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_error converts AnthropicStatusError into a BackendAPIError, backend_api_error_handler returns 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:request with mechanism: {"type": "anthropic", "handled": false} before our handler ever sees the exception. Nothing downstream can un-capture it.

Fix

_sentry_before_send drops APIStatusError whose status_code is 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

  • Four new tests in test_sentry_scrubbing.py build real APIStatusErrors from httpx responses: 429 and 529 are dropped, 400 is kept, and an ordinary TypeError from 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.py passes 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 in test_config_registry.py); all three pass when run with GATEWAY_PORT/POSTGRES_PORT/REDIS_PORT unset, 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 AnthropicStatusError identically regardless of status code (_handle_anthropic_error / _build_error_event both log at warning and convert to BackendAPIError for any status code), and these three are the provider legitimately rejecting something the client sent, not something the proxy built:

  • LUTHIEN-6 (1,080 events, by far the largest single issue in the whole quota drain): a client sent message content with a field Anthropic's schema rejects (400, e.g. messages.297.content.1.text.parsed_output: Extra inputs are not permitted). Confirmed parsed_output is not a field this proxy or any policy injects — it's client-originated.
  • LUTHIEN-2 (125 events): a client requested a model name Anthropic doesn't have (404, e.g. model: opus-4.6).
  • LUTHIEN-D (28 events): a client-supplied bearer token that Anthropic itself rejected as invalid (401) — passthrough-credential mode forwards the client's own token unchanged; the proxy didn't mint it.

_EXPECTED_UPSTREAM_STATUS_CODES now 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.

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
legion-implementer Bot force-pushed the fix/sentry-expected-upstream-errors branch from 2b311f8 to e538774 Compare August 29, 2026 23:25
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
legion-implementer Bot force-pushed the fix/sentry-expected-upstream-errors branch from e538774 to 7fcb643 Compare August 30, 2026 00:11
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
legion-implementer Bot force-pushed the fix/sentry-expected-upstream-errors branch from 7fcb643 to 7363c63 Compare August 30, 2026 00:20
…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.
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.

1 participant