Commit graph

2 commits

Author SHA1 Message Date
Abhay Singh
2b5ee7cde8
fix(proxy/anthropic): None-guard usage token counts on the direct buffered path (#2434)
## Description

The direct (non-backend) Anthropic buffered `/v1/messages` path reads
token counts from the response usage to record metrics and update the
prefix tracker:

```python
usage = resp_json.get("usage", {})
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)
...
uncached_input_tokens = usage.get("input_tokens", 0)
```

`.get(key, default)` only falls back when the key is **absent**. When a
key is present with a **null** value, `.get` returns `None`. The direct
Anthropic API always sends integer usage, but this same handler serves
any Anthropic-compatible upstream reached through a custom
`ANTHROPIC_TARGET_API_URL` gateway (the scenario `install apply` now
supports), and such a gateway can emit null counts on a stopped or empty
turn.

Those `None`s then reach `max(0, expected_cached - cr_tokens)` in the
cache-bust block and the int-typed `RequestOutcome` / metrics recorder,
so a single such response raises an uncaught `TypeError` and 502s the
request. This is the same class as the Gemini crash fixed in #2347 and
the OpenAI chat path.

## Fix

Coerce the four counts with `int(... or 0)` at the direct-path
usage-extraction site, matching `_extract_anthropic_cache_ttl_metrics`
(which already guards its TTL buckets this way) and the Gemini fix. A
normal integer usage is unchanged; only a null (or absent) value now
becomes 0.

## Type of Change

- [x] Bug fix (non-breaking change that fixes an issue)
- [ ] New feature (non-breaking change that adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
- [ ] Performance improvement
- [ ] Code refactoring (no functional changes)

## Changes Made

- `headroom/proxy/handlers/anthropic.py`: `int(... or 0)`-guard
`output_tokens` / `cache_read_input_tokens` /
`cache_creation_input_tokens` / `input_tokens` at the direct
buffered-path usage-extraction site.
- `tests/test_proxy/test_anthropic_buffered_timeout.py`: regression
driving a buffered `/v1/messages` request whose upstream usage reports
null counts, asserting a 200 instead of a 502.

## Testing

- [x] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check .`)
- [x] Type checking passes (`mypy headroom`)
- [x] New tests added for new functionality
- [ ] Manual testing performed

### Test Output

```text
$ python -m pytest tests/test_proxy/test_anthropic_buffered_timeout.py -q
# all pass

# with the fix reverted, the new test fails (the null-usage response 502s):
$ git stash push -- headroom/proxy/handlers/anthropic.py
$ python -m pytest "tests/test_proxy/test_anthropic_buffered_timeout.py::test_anthropic_messages_buffered_survives_null_usage_counts" -q
1 failed  (TypeError: unsupported operand type(s) for +: 'NoneType' and 'NoneType')

$ uvx ruff@0.15.17 check headroom/proxy/handlers/anthropic.py tests/test_proxy/test_anthropic_buffered_timeout.py
All checks passed!
```

## Real Behavior Proof

- Environment: Windows 11, Python 3.12, project venv (`uv sync --extra
proxy`), `uvx ruff@0.15.17` / `uvx mypy@1.20.2`, pytest in the venv.
- Exact command / steps: ran the new FastAPI `TestClient` regression,
which drives the real direct buffered `/v1/messages` handler with
`proxy._retry_request` returning a 200 whose `usage` has null
`input_tokens` / `output_tokens` / `cache_read_input_tokens` /
`cache_creation_input_tokens`; then reverted only `anthropic.py` and
re-ran.
- Observed result: with the fix the request returns 200; with the fix
reverted the same request 502s with `TypeError: unsupported operand
type(s) for +: 'NoneType' and 'NoneType'`. Ran against the actual
handler via the app.
- Not tested: a live third-party Anthropic-compatible gateway emitting
null usage.

## 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
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] New and existing unit tests pass locally with my changes
- [ ] I have updated the CHANGELOG.md if applicable

Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
2026-08-12 00:07:51 -05:00
Rod Boev
3be2526b76
fix(proxy): add an Anthropic buffered read-timeout override (#1331)
## Description

Buffered Anthropic `/v1/messages` requests still use Headroom's generic
300-second read timeout, which can produce proxy-generated `502
ReadTimeout` errors on long turns. This adds a dedicated buffered
Anthropic timeout, keeps it applied across CCR and memory continuations
plus batch paths, and makes the direct server entrypoint enforce the
same positive-integer contract as the Click CLI. Closes #1261.

## Type of Change

- [x] Bug fix (non-breaking change that fixes an issue)

## Changes Made

- Added `anthropic_buffered_request_timeout_seconds` for buffered
Anthropic reads.
- Routed `/v1/messages`, CCR continuation, memory continuation, batch
create, batch passthrough, and batch results through that timeout.
- Enforced the same positive-integer validation for
`HEADROOM_ANTHROPIC_BUFFERED_REQUEST_TIMEOUT_SECONDS` and
`--anthropic-buffered-request-timeout-seconds` in both startup paths.
- Added focused regressions and updated `CHANGELOG.md`.

## Testing

- [x] `uv run pytest tests/test_proxy/test_anthropic_buffered_timeout.py
tests/test_cli_proxy_improvements.py::TestNewEnvVarWiring`
- [x] `uv run ruff check .`
- [x] `uv run ruff format . --check`

### Test Output

```text
$ uv run pytest tests/test_proxy/test_anthropic_buffered_timeout.py tests/test_cli_proxy_improvements.py::TestNewEnvVarWiring
17 passed in 3.42s

$ uv run ruff check .
All checks passed!

$ uv run ruff format . --check
966 files already formatted
```

## Real Behavior Proof

- Environment: local FastAPI `TestClient` with stubbed retry and HTTP
client seams
- Exact command / steps: run `uv run pytest
tests/test_proxy/test_anthropic_buffered_timeout.py
tests/test_cli_proxy_improvements.py::TestNewEnvVarWiring`; the tests
build `ProxyConfig(request_timeout_seconds=7, connect_timeout_seconds=3,
anthropic_buffered_request_timeout_seconds=19)`, drive `/v1/messages`,
`/v1/messages/batches`, `/v1/messages/batches/{batch_id}/results`, a CCR
continuation, and a memory continuation through `TestClient`, then
verify `HEADROOM_ANTHROPIC_BUFFERED_REQUEST_TIMEOUT_SECONDS=0` falls
back to `600`, `--anthropic-buffered-request-timeout-seconds 0` is
rejected, and default proxy timeouts stay `read=300` and `write=300`
- Observed result: buffered Anthropic paths use
`httpx.Timeout(connect=3, read=19, write=7, pool=3)`, continuation
requests stay on that same budget, invalid zero-valued startup config is
rejected or ignored back to the default, and unrelated proxy timeout
defaults stay unchanged
- Not tested: live upstream Anthropic latency beyond the focused
stubbed-timeout regression

## 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 added tests that prove the fix
- [x] New and existing unit tests pass locally with my changes
- [x] I have updated the CHANGELOG.md if applicable
2026-06-23 22:46:31 -05:00