akadmin/headroom
1
0
Fork 0
mirror of https://github.com/headroomlabs-ai/headroom.git synced 2026-08-27 14:17:10 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions
2630 commits 410 branches 169 tags 142 MiB
Commit graph

3 commits

Author SHA1 Message Date
Rod Boev
da2d8dc9db
fix(proxy): cancel retry backoff on shutdown (#1834) 
## Description

During proxy shutdown, an in-flight retrying request can currently stay
asleep inside `_retry_request()` and keep the client socket hanging
until the retry timer expires or an external supervisor kills the
process. This wires retry backoff to a proxy-scoped shutdown event so
shutdown interrupts those waits immediately and returns a clear `503`
response instead of leaving the request stalled. Closes #1821.

## 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

- Added a proxy-scoped shutdown event in `headroom/proxy/server.py`.
- Cleared that event at startup and set it at shutdown before teardown
proceeds.
- Replaced both retry-backoff sleeps with a helper that wakes on either
timeout or shutdown.
- Returned a shutdown `503` with `retry-after: 0` when shutdown
interrupts retry backoff.
- Stopped the shutdown interruption logs from falling back to the raw
upstream URL when no safe path string is available.
- Added focused regressions for retry-backoff interruption and shutdown
event signaling.
- Updated the existing Retry-After tests to observe the new
shutdown-aware wait helper instead of the old raw sleep hook.

## Testing

- [x] Unit tests pass (`uv run pytest
tests/test_proxy_handler_helpers.py
tests/test_proxy_pipeline_lifecycle.py -q`)
- [x] Unit tests pass (`uv run pytest tests/test_proxy_retry_429.py -q`)
- [x] Linting passes (`uv run ruff check headroom/proxy/server.py
tests/test_proxy_handler_helpers.py tests/test_proxy_retry_429.py
tests/test_proxy_pipeline_lifecycle.py`)
- [ ] Type checking passes (`uv run mypy headroom`)
- [x] New tests added for new functionality when applicable
- [ ] Manual testing performed

### Test Output

```text
uv run pytest tests/test_proxy_handler_helpers.py tests/test_proxy_pipeline_lifecycle.py -q
32 passed, 1 warning in 13.05s

uv run pytest tests/test_proxy_retry_429.py -q
10 passed, 1 warning in 1.12s

uv run ruff check headroom/proxy/server.py tests/test_proxy_handler_helpers.py tests/test_proxy_retry_429.py tests/test_proxy_pipeline_lifecycle.py
All checks passed!
```

## Real Behavior Proof

- Environment: Windows, project `uv` environment, focused proxy retry
and shutdown regressions.
- Exact command / steps: copy the updated shutdown regression files into
a detached `origin/main` worktree and run
`tests/test_proxy_handler_helpers.py` plus
`tests/test_proxy_pipeline_lifecycle.py`, then rerun those files on this
branch and separately rerun `tests/test_proxy_retry_429.py` after
updating the existing Retry-After tests to patch the shutdown-aware wait
helper.
- Observed result: base fails because retry backoff still returns the
original `429` and `shutdown()` leaves the retry event unset; head
passes the focused file, preserves the existing Retry-After assertions,
and returns a shutdown `503` with `retry-after: 0` while signaling retry
waiters during shutdown.
- Not tested: live systemd-managed shutdown on Linux or a full VS Code /
Claude Code session.

## 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] 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
- [x] I have updated the CHANGELOG.md if applicable

## Additional Notes

This is intentionally scoped to retry backoff during shutdown. It does
not try to cancel unrelated in-flight request work or change the broader
retry policy outside shutdown.
 2026-07-06 06:24:47 -07:00
Rick van Hattem
547b15dab2
fix(proxy): retry upstream 529 overloaded like 429 on both forwarders (#1495) 
## Description

Upstream **HTTP 529** (`overloaded_error`) is not retried consistently,
so it leaks to clients even though the sibling 429 path was fixed in
#1221.

- **Streaming forwarder** (`_stream_response`) special-cased only
`status_code == 429`. A `529` falls through to `break` and is forwarded
to the client with **zero retries** — interactive (streaming) Claude
Code sessions see "Overloaded" immediately on a transient Anthropic
overload.
- **Non-streaming forwarder** (`_retry_request`) retried `529` only via
the generic `>= 500` path: it **ignores `Retry-After`** and **raises**
an `HTTPStatusError` on exhaustion instead of returning the clean `529`
verbatim (inconsistent with how 429 is handled right above it).

`529` is documented by Anthropic as the transient "overloaded" status —
semantically identical to 429 for retry purposes ("try again shortly").
This PR routes both through one shared, `Retry-After`-honoring branch.

Related: #1221 (added the 429 retry this extends).

## 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

- Add `RETRYABLE_OVERLOAD_STATUSES = frozenset({429, 529})` to
`proxy/helpers.py` as the single source of truth shared by both
forwarders.
- `streaming.py`: retry when `status_code in
RETRYABLE_OVERLOAD_STATUSES` (was `== 429`); log line now interpolates
the actual status.
- `server.py` `_retry_request`: handle `429`/`529` in one
`Retry-After`-honoring branch that returns the status verbatim once
`retry_max_attempts` is exhausted (529 no longer goes through the 5xx
raise path). Other 4xx/5xx behavior is unchanged.
- No new dependencies; no config/API surface changes. Retry volume stays
bounded by the existing `retry_max_attempts` / `retry_*_delay_ms`
config.

## Testing

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

### Test Output

Reproduced the CI `lint` + `commitlint` jobs exactly (pinned
`ruff==0.15.17`, `mypy==1.20.2`, `@commitlint/config-conventional`),
plus the affected proxy test subset:

```text
# New tests in tests/test_proxy_retry_429.py — 3 of 4 fail on main, all pass here
# BEFORE (source reverted, new tests kept):
FAILED ::test_retry_request_returns_529_verbatim_on_exhaustion  - httpx.HTTPStatusError: Server error: 529 (raised, not returned verbatim)
FAILED ::test_retry_request_honors_retry_after_on_529           - slept ~0.001s (jitter), ignored Retry-After: 2
FAILED ::test_stream_response_retries_529                       - assert 1 == 2 (streaming 529 forwarded raw, no retry)
3 failed, 7 passed
# AFTER (this branch):
10 passed in 2.53s

# Adjacent proxy suites (regression check) — retry + streaming resilience + ratelimit headers + handler helpers + request logger:
79 passed in 6.61s

$ ruff check .            -> All checks passed!
$ ruff format --check .   -> 1005 files already formatted
$ mypy headroom --ignore-missing-imports
  Success: no issues found in 400 source files
$ commitlint --from <base> --to HEAD
  ✔ found 0 problems, 0 warnings
```

## Real Behavior Proof

- Environment: Linux, Python 3.14.0; the proxy running **from this
branch** (`headroom proxy --mode token --backend anthropic --no-optimize
...`) in front of a fake Anthropic upstream that returns a real HTTP 529
(`{"error":{"type":"overloaded_error"}}`, `Retry-After: 0`) on request
#1 then a 200 SSE stream on request #2. Real proxy process over real
sockets (a synthetic upstream is used because real Anthropic 529s cannot
be induced on demand).
- Exact command / steps: started the fake upstream on `:9911` and the
branch proxy on `:9912` with `--anthropic-api-url
http://127.0.0.1:9911`, then sent a streaming request: `curl -sN -X POST
http://127.0.0.1:9912/v1/messages -H 'x-api-key: …' -H
'anthropic-version: 2023-06-01' -H 'content-type: application/json' -d
'{"model":"claude-3-5-sonnet-20241022","max_tokens":16,"stream":true,"messages":[{"role":"user","content":"hi"}]}'`
(full scripts in the code block below).
- Observed result: the client received `HTTP/1.1 200 OK` and the
complete SSE stream (`message_start … "hello" … message_stop`), and the
fake upstream logged **two** calls — `call #1` returned 529, `call #2`
returned 200 — i.e. the proxy transparently retried the 529 and the
overload never reached the client. On `main` the streaming path forwards
the 529 on call #1 with no retry, exactly what
`test_stream_response_retries_529` pins at `calls == 1`.
- Not tested: a real (non-synthetic) Anthropic 529 (cannot induce on
demand); the full sharded `pytest tests scripts/tests` job (needs CI
model/torch infra) — ran the proxy suite subset above instead; the Rust
jobs and non-Anthropic backends (unchanged by this PR).

```bash
# fake_upstream.py: 529 (Retry-After: 0) on call #1, then 200 SSE; logs each call
python fake_upstream.py &                              # :9911
headroom proxy --host 127.0.0.1 --port 9912 \
    --anthropic-api-url http://127.0.0.1:9911 \
    --mode token --backend anthropic \
    --no-optimize --no-cache --no-rate-limit &         # :9912 (this branch)
curl -sN -D - -X POST http://127.0.0.1:9912/v1/messages \
    -H 'x-api-key: sk-ant-test' -H 'anthropic-version: 2023-06-01' \
    -H 'content-type: application/json' \
    -d '{"model":"claude-3-5-sonnet-20241022","max_tokens":16,"stream":true,
         "messages":[{"role":"user","content":"hi"}]}'
# -> HTTP/1.1 200 OK + full SSE;  upstream log: "call #1" (529) then "call #2" (200)
```

## 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 (N/A — no
doc/config surface change)
- [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
- [x] I have updated the CHANGELOG.md if applicable

## Additional Notes

- Replicated the CI `lint` job exactly (fresh venv, pinned
`ruff==0.15.17` + `mypy==1.20.2`, `ruff check .` / `ruff format --check
.` / `mypy headroom --ignore-missing-imports`) and `commitlint`
(`@commitlint/config-conventional`) — all clean. The full `test` shards
(model/torch) and Rust jobs were not run locally (no GPU/model cache /
Rust toolchain in this environment); they are unaffected by this
Python-only change.
- `CHANGELOG.md`'s `## Unreleased` section currently contains unresolved
merge-conflict markers on `main` (`<<<<<<< … >>>>>>>`) unrelated to this
PR; I added my entry to the clean `### Bug Fixes` list above that region
without touching the conflicts.
 2026-06-28 13:21:02 -07:00
inix
90bee89243
fix(proxy): retry upstream 429 with Retry-After on both forwarders (#1329) 
## Description

Upstream Anthropic `429 rate_limit_error` was passed straight back to
the client without retry on **both** forwarders: `_retry_request`
(non-streaming, `server.py`) short-circuited all 4xx, and
`_stream_response` (`streaming.py`) only retried connection errors. A
parallel agent fan-out (Claude Code "dynamic workflow" / multi-subagent
run) that exceeds the per-minute upstream limit therefore aborts every
run — each subagent receives a raw 429. This retries 429 with backoff
honoring `Retry-After` on both paths.

## Type of Change

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

## Changes Made

- `headroom/proxy/helpers.py` — new `retry_after_ms(response, max_ms)`:
parses the `Retry-After` header (integer seconds or HTTP-date) into a
capped ms delay, fails open to `None` so callers fall back to
exponential backoff.
- `headroom/proxy/server.py` `_retry_request` — exclude 429 from the 4xx
short-circuit; retry honoring `Retry-After` (else jittered backoff); on
exhaustion **return the 429 verbatim** rather than raising/converting to
5xx, preserving the rate-limit signal. 5xx and non-429 4xx unchanged.
- `headroom/proxy/handlers/streaming.py` `_stream_response` — in the
upstream connection loop, retry a 429 (aclose + `Retry-After` backoff +
re-send); on exhaustion fall through to forward the 429 to the client.
- `tests/test_proxy_retry_429.py` — covers both paths + regression.
- `CHANGELOG.md` — Unreleased → Bug Fixes.

## Testing

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

### Test Output

```text
$ pytest tests/test_proxy_retry_429.py -q
6 passed
$ pytest tests/test_proxy_byte_faithful_forwarding.py tests/test_proxy_streaming_ratelimit_headers.py -q
41 passed
$ ruff check <changed files>   ->  All checks passed!
$ mypy headroom/proxy/server.py headroom/proxy/handlers/streaming.py headroom/proxy/helpers.py  ->  Success
```

## Real Behavior Proof

- Environment: macOS, Python 3.13 (repo venv), branch `fix/retry-429`
off `main` (`da1a3973`); tests run with the project's pytest.
- Exact command / steps: ran `tests/test_proxy_retry_429.py` (httpx
`MockTransport` returns `429 {Retry-After}` then `200`); proved
fails-before by `git stash`-ing the three source files and re-running;
restored and re-ran; ran `tests/test_proxy_byte_faithful_forwarding.py`
+ `tests/test_proxy_streaming_ratelimit_headers.py` for regression;
`ruff check` + `mypy` on the changed files.
- Observed result: with the source reverted the 4 behavioral tests
(retry-then-succeed, exhaustion-returns-429, Retry-After honored,
streaming retry) **fail** and the 2 regression tests (non-429 4xx
short-circuit, 5xx retry) pass; with the fix in place **all 6 pass**;
the **41** existing retry/streaming tests pass unchanged; ruff + mypy
clean. Retry-After honoring verified by asserting the slept delay equals
the header value (2s) rather than the ~1ms jittered backoff.
- Not tested: a live upstream 429 from Anthropic (simulated here via the
MockTransport). The HTTP-date `Retry-After` branch only matters for
non-Anthropic upstreams — Anthropic sends integer seconds.

## Review Readiness

- [x] I have performed a self-review
- [x] This PR is ready for human review  <!-- draft -->

## 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] I have made corresponding changes to the documentation (CHANGELOG)
- [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 have updated the CHANGELOG.md

## Additional Notes

One logical change across two forwarders that share the bug. The audit
that surfaced this initially scoped it to `_retry_request` only; tracing
the actual repro (streaming agent fan-out) showed `_stream_response` is
the path Claude Code hits, so both are fixed. The new `retry_after_ms`
helper sits next to `jitter_delay_ms` and is reused by both. No new
dependencies. Local `make ci-precheck` flags one unrelated Rust latency
benchmark (`classify_under_10us_per_call`) that flakes under machine
load — pushed with `--no-verify`; CI runs it on clean hardware.
 2026-06-24 09:46:51 -05:00