From 9ca5a16bde81f515454f9bc6ad022caa15e118b8 Mon Sep 17 00:00:00 2001 From: Abhay Singh Date: Tue, 18 Aug 2026 08:51:09 +0530 Subject: [PATCH] fix(proxy/anthropic): coerce present-null usage counters on the buffered backend path (#3084) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description The buffered (non-streaming) Anthropic backend branch in `handle_anthropic_messages` (`headroom/proxy/handlers/anthropic.py`) — the path taken by Bedrock / Vertex / LiteLLM(anthropic) traffic — read the response usage counters with a bare default: ```python output_tokens = usage.get("output_tokens", 0) ... cr_tokens = usage.get("cache_read_input_tokens", 0) cw_tokens = usage.get("cache_creation_input_tokens", 0) ``` A backend can report these counters as JSON `null` (key **present**, value null) rather than omitting them. For a present-null key `dict.get(key, 0)` returns `None`, not the default `0`. That `None` then flowed into: ```python provider_input_tokens=(uncached_input_tokens + cr_tokens + cw_tokens) ``` raising `TypeError: unsupported operand type(s) for +: 'NoneType' and 'NoneType'`, which the outer handler converted into a failed turn (HTTP 500 `api_error`) instead of a normal 200 with zeroed counters. The direct-Anthropic-API branch a few hundred lines down already guards this exact case with `int(usage.get(key, 0) or 0)`, and the surrounding code even comments that a backend may "send null" for `input_tokens` (and None-guards that field). The buffered branch was simply left behind, so the two parallel paths disagreed on null handling. ## Fix Coerce the three counters on the buffered path with `int(usage.get(key, 0) or 0)`, exactly matching the direct-API idiom, so a present-null value becomes `0` instead of `None`. The already-present `input_tokens is not None` guard is unaffected, and its fallback subtraction now operates on coerced ints. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature - [ ] Breaking change - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - `headroom/proxy/handlers/anthropic.py` (buffered backend branch of `handle_anthropic_messages`): coerce `output_tokens`, `cache_read_input_tokens` and `cache_creation_input_tokens` with `int(usage.get(key, 0) or 0)` so a present-null value is treated as `0`, matching the direct-Anthropic path. - `tests/test_backend_nonstreaming_cache_metrics.py`: added `test_anthropic_backend_nonstreaming_present_null_cache_counters_do_not_crash`, driving the buffered backend path with present-null `output_tokens` / `cache_read_input_tokens` / `cache_creation_input_tokens` and asserting a 200 with a recorded `RequestOutcome` whose counters are `0` and whose uncached input comes from the present `input_tokens`. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check`) - [x] Type checking passes (`mypy`) - [x] New tests added ### Test Output ```text tests/test_backend_nonstreaming_cache_metrics.py 7 passed # uvx ruff@0.15.22 check -> All checks passed! # uvx mypy@1.20.2 headroom/proxy/handlers/anthropic.py -> Success: no issues found in 1 source file ``` ## Real Behavior Proof - Environment: Windows 11, Python 3.12.11, project venv, pytest 9.1.1, ruff 0.15.22 and mypy 1.20.2 via uvx. - Exact command / steps: ran the new regression against the unpatched handler and captured the crash (`python -m pytest tests/test_backend_nonstreaming_cache_metrics.py::test_anthropic_backend_nonstreaming_present_null_cache_counters_do_not_crash -x -q` -> `assert 500 == 200` with body `{"type":"error","error":{"type":"api_error","message":"unsupported operand type(s) for +: 'NoneType' and 'NoneType'"}}`); applied the `int(... or 0)` coercion; re-ran the whole file (`python -m pytest tests/test_backend_nonstreaming_cache_metrics.py -q` -> 7 passed); then `uvx ruff@0.15.22 format`, `uvx ruff@0.15.22 check`, and `uvx mypy@1.20.2 headroom/proxy/handlers/anthropic.py`. - Observed result: before the fix a backend response whose usage carries `cache_read_input_tokens: null` (or a null `output_tokens` / `cache_creation_input_tokens`) returned HTTP 500 and recorded no outcome; after the fix the same response returns 200, the counters coerce to `0`, and the `PERF` line reports `cache_read=0 cache_write=0`. - Not tested: a live Bedrock/Vertex session emitting a real null-counter usage block (the null-usage shape is reproduced directly through the mocked backend that the existing suite already uses for this path). ## Runtime Rollout Safety - Rollout-managed feature(s): none. This is the buffered Anthropic response-accounting path behind `handle_anthropic_messages`, not a rollout-channel-gated runtime feature. - Minimum rollout channel: N/A (no rollout-managed behavior). - Stable/default behavior changed: yes, as a bug fix. A backend response with present-null usage counters now completes with a 200 and zeroed counters instead of failing the turn with a 500. Responses with numeric counters are unaffected. - Kill switch / disable path: N/A. There is no behavioral toggle; the change only hardens numeric coercion on the accounting path and does not alter routing, compression, or request forwarding. - Unsafe override required: no. - Qualification impact: Bedrock / Vertex / LiteLLM(anthropic) non-streaming turns that report a null cache/output counter stop 500-ing and are recorded with zeroed counters, matching the direct-Anthropic path. - Rollback path: revert this PR; the buffered path returns to the bare `usage.get(key, 0)` reads. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Checklist - [x] My code follows the project's style guidelines - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective - [x] New and existing unit tests pass locally with my changes - [x] I did **not** edit `CHANGELOG.md`: it is generated by release-please from my Conventional Commit PR title ## Additional Notes This mirrors the recently fixed Gemini CCR-continuation present-null usage bug: the same `dict.get(key, default)` present-null trap, on the parallel Anthropic backend path. Only the buffered (non-streaming) backend branch was affected; the direct-Anthropic and streaming paths already coerce with `or 0`. --- headroom/proxy/handlers/anthropic.py | 11 ++- ...test_backend_nonstreaming_cache_metrics.py | 81 +++++++++++++++++++ 2 files changed, 89 insertions(+), 3 deletions(-) diff --git a/headroom/proxy/handlers/anthropic.py b/headroom/proxy/handlers/anthropic.py index 2da5f9afe..9a5b120a3 100644 --- a/headroom/proxy/handlers/anthropic.py +++ b/headroom/proxy/handlers/anthropic.py @@ -3267,7 +3267,12 @@ class AnthropicHandlerMixin: # Track metrics total_latency = (time.time() - start_time) * 1000 usage = backend_response.body.get("usage", {}) - output_tokens = usage.get("output_tokens", 0) + # A backend may report these counters as JSON null (key + # present, value null), for which ``.get(key, 0)`` returns + # ``None`` rather than the default. Coerce with ``or 0`` so + # the arithmetic below (and ``RequestOutcome``) never sees + # ``None`` — matching the direct-Anthropic path. + output_tokens = int(usage.get("output_tokens", 0) or 0) _backend_name = request_backend.name if request_backend else "anthropic" # Eligible-only denominator for the active @@ -3286,8 +3291,8 @@ class AnthropicHandlerMixin: except Exception: attempted_input_tokens = original_tokens - cr_tokens = usage.get("cache_read_input_tokens", 0) - cw_tokens = usage.get("cache_creation_input_tokens", 0) + cr_tokens = int(usage.get("cache_read_input_tokens", 0) or 0) + cw_tokens = int(usage.get("cache_creation_input_tokens", 0) or 0) cw_5m_tokens, cw_1h_tokens = self._extract_anthropic_cache_ttl_metrics( usage ) diff --git a/tests/test_backend_nonstreaming_cache_metrics.py b/tests/test_backend_nonstreaming_cache_metrics.py index 6eb87e44d..943aec661 100644 --- a/tests/test_backend_nonstreaming_cache_metrics.py +++ b/tests/test_backend_nonstreaming_cache_metrics.py @@ -492,3 +492,84 @@ def test_anthropic_backend_nonstreaming_uncached_falls_back_when_input_tokens_ab # No input_tokens reported and no cache: the live-zone derivation yields the # non-zero token count of the new turn, never the 0 the naive default gave. assert outcome.uncached_input_tokens > 0 + + +def test_anthropic_backend_nonstreaming_present_null_cache_counters_do_not_crash() -> None: + """Present-but-null usage counters must coerce to 0, not crash the turn. + + A backend can send the cache/output counters as JSON ``null`` (key present, + value null) rather than omitting them — the direct-Anthropic path already + guards this with ``int(usage.get(key, 0) or 0)`` and the surrounding code + even acknowledges a backend that "sends null" for ``input_tokens``. The + buffered backend branch, however, read ``usage.get(key, 0)`` for + ``output_tokens`` / ``cache_read_input_tokens`` / ``cache_creation_input_tokens``, + and ``.get`` returns ``None`` for a present-null key (the default applies + only to an absent key). That ``None`` then flowed into + ``uncached_input_tokens + cr_tokens + cw_tokens`` and the prefix-tracker + calls, raising ``TypeError`` that the outer handler turned into a failed + request instead of a normal 200 with zeroed counters. + """ + from headroom.proxy.server import HeadroomProxy + + config = ProxyConfig( + optimize=False, + cache_enabled=False, + rate_limit_enabled=False, + backend="anyllm", + anyllm_provider="anthropic", + ) + body = { + "id": "msg_1", + "type": "message", + "role": "assistant", + "model": "claude-3-5-sonnet-20241022", + "content": [{"type": "text", "text": "hi"}], + "stop_reason": "end_turn", + "usage": { + "input_tokens": 1000, + "output_tokens": None, + "cache_read_input_tokens": None, + "cache_creation_input_tokens": None, + }, + } + backend = _make_anthropic_backend(body) + + captured: list[Any] = [] + orig_record = HeadroomProxy._record_request_outcome + + async def _spy(self, outcome): # noqa: ANN001, ANN202 + captured.append(outcome) + return await orig_record(self, outcome) + + log_handle = _attach_proxy_log_capture() + try: + with ( + patch("headroom.proxy.server.AnyLLMBackend", return_value=backend), + patch.object(HeadroomProxy, "_record_request_outcome", _spy), + ): + app = create_app(config) + with TestClient(app) as client: + resp = client.post( + "/v1/messages", + json={ + "model": "claude-3-5-sonnet-20241022", + "messages": [{"role": "user", "content": "hi"}], + "max_tokens": 64, + }, + headers={"x-api-key": "sk-ant-test", "anthropic-version": "2023-06-01"}, + ) + assert resp.status_code == 200, resp.text[:200] + finally: + _detach_proxy_log_capture(*log_handle) + + assert captured, "expected a recorded RequestOutcome (turn must not have crashed)" + outcome = captured[-1] + # Null counters coerce to 0; the present input_tokens still drives uncached. + assert outcome.output_tokens == 0 + assert outcome.cache_read_tokens == 0 + assert outcome.cache_write_tokens == 0 + assert outcome.uncached_input_tokens == 1000 + + handler = log_handle[0] + cr, cw, chp = _find_perf_record(handler.records) + assert (cr, cw, chp) == (0, 0, 0)