headroom/tests/test_proxy_retry_429.py
Rod Boev da2d8dc9db
Some checks failed
Docker / docker-build (map[name:amd64 platform:linux/amd64 runs_on:ubuntu-24.04], map[bake_target:runtime name:]) (push) Waiting to run
Docker / docker-build (map[name:amd64 platform:linux/amd64 runs_on:ubuntu-24.04], map[bake_target:runtime-code name:code]) (push) Waiting to run
Docker / docker-build (map[name:amd64 platform:linux/amd64 runs_on:ubuntu-24.04], map[bake_target:runtime-code-nonroot name:code-nonroot]) (push) Waiting to run
Docker / docker-build (map[name:amd64 platform:linux/amd64 runs_on:ubuntu-24.04], map[bake_target:runtime-code-slim name:code-slim]) (push) Waiting to run
Docker / docker-build (map[name:amd64 platform:linux/amd64 runs_on:ubuntu-24.04], map[bake_target:runtime-code-slim-nonroot name:code-slim-nonroot]) (push) Waiting to run
CI / changes (push) Failing after 50s
Docker / docker-build (map[name:amd64 platform:linux/amd64 runs_on:ubuntu-24.04], map[bake_target:runtime-nonroot name:nonroot]) (push) Waiting to run
Docker / docker-build (map[name:amd64 platform:linux/amd64 runs_on:ubuntu-24.04], map[bake_target:runtime-slim name:slim]) (push) Waiting to run
Docker / docker-build (map[name:amd64 platform:linux/amd64 runs_on:ubuntu-24.04], map[bake_target:runtime-slim-nonroot name:slim-nonroot]) (push) Waiting to run
Docker / docker-build (map[name:arm64 platform:linux/arm64 runs_on:ubuntu-24.04-arm], map[bake_target:runtime name:]) (push) Waiting to run
Docker / docker-build (map[name:arm64 platform:linux/arm64 runs_on:ubuntu-24.04-arm], map[bake_target:runtime-code name:code]) (push) Waiting to run
Docker / docker-build (map[name:arm64 platform:linux/arm64 runs_on:ubuntu-24.04-arm], map[bake_target:runtime-code-nonroot name:code-nonroot]) (push) Waiting to run
Docker / docker-build (map[name:arm64 platform:linux/arm64 runs_on:ubuntu-24.04-arm], map[bake_target:runtime-code-slim name:code-slim]) (push) Waiting to run
Docker / docker-build (map[name:arm64 platform:linux/arm64 runs_on:ubuntu-24.04-arm], map[bake_target:runtime-code-slim-nonroot name:code-slim-nonroot]) (push) Waiting to run
Docker / docker-build (map[name:arm64 platform:linux/arm64 runs_on:ubuntu-24.04-arm], map[bake_target:runtime-nonroot name:nonroot]) (push) Waiting to run
CI / commitlint (push) Failing after 2s
Docker / docker-build (map[name:arm64 platform:linux/arm64 runs_on:ubuntu-24.04-arm], map[bake_target:runtime-slim name:slim]) (push) Waiting to run
Docker / docker-build (map[name:arm64 platform:linux/arm64 runs_on:ubuntu-24.04-arm], map[bake_target:runtime-slim-nonroot name:slim-nonroot]) (push) Waiting to run
Docker / docker-manifest (map[bake_target:runtime name:]) (push) Blocked by required conditions
Docker / docker-manifest (map[bake_target:runtime-code name:code]) (push) Blocked by required conditions
Docker / docker-manifest (map[bake_target:runtime-code-nonroot name:code-nonroot]) (push) Blocked by required conditions
CI / lint (push) Failing after 4s
CI / build-wheel (push) Failing after 2s
Docker / docker-manifest (map[bake_target:runtime-code-slim name:code-slim]) (push) Blocked by required conditions
Docker / docker-manifest (map[bake_target:runtime-code-slim-nonroot name:code-slim-nonroot]) (push) Blocked by required conditions
Docker / docker-manifest (map[bake_target:runtime-nonroot name:nonroot]) (push) Blocked by required conditions
Docker / docker-manifest (map[bake_target:runtime-slim name:slim]) (push) Blocked by required conditions
Docker / docker-manifest (map[bake_target:runtime-slim-nonroot name:slim-nonroot]) (push) Blocked by required conditions
CI / prefetch-model (push) Failing after 4s
Docker / promote-latest (push) Blocked by required conditions
CI / test-agno (push) Failing after 4s
CI / build (push) Failing after 3s
CI / test-extras (push) Failing after 7s
CI / workflow-validation (push) Failing after 4s
CI / docker-native-e2e (push) Failing after 4s
Deploy Documentation / validate (push) Failing after 6s
Deploy Documentation / deploy (push) Failing after 6s
CI / test-dashboard-ui (push) Failing after 3s
CI / test (1) (push) Failing after 3s
CI / test (2) (push) Failing after 5s
CI / test (3) (push) Failing after 5s
CI / test (4) (push) Failing after 5s
Init E2E / docker-init-e2e (push) Failing after 6s
Install Native E2E / install-native (ubuntu-latest) (push) Failing after 6s
Merge Conflicts / merge-conflicts (push) Failing after 5s
Release Please / release-please (push) Failing after 4s
Security / CodeQL (javascript-typescript) (push) Failing after 31s
Security / CodeQL (python) (push) Failing after 28s
Security / Dependency audit (pip-audit) (push) Failing after 1m3s
Wrap E2E / docker-wrap-e2e (push) Failing after 21s
Wrap Native E2E / wrap-native (ubuntu-latest) (push) Failing after 6s
Security / Secret scan (gitleaks) (push) Failing after 36s
Init Native E2E / init-native (ubuntu-latest, copilot) (push) Failing after 12m32s
Init Native E2E / init-native (ubuntu-latest, codex) (push) Failing after 12m35s
Init Native E2E / init-native (ubuntu-latest, claude) (push) Failing after 12m36s
CI / windows-native-wrapper (push) Has been cancelled
CI / macos-native-wrapper (push) Has been cancelled
Init Native E2E / init-native (macos-latest, claude) (push) Has been cancelled
Init Native E2E / init-native (macos-latest, codex) (push) Has been cancelled
Init Native E2E / init-native (macos-latest, copilot) (push) Has been cancelled
Install Native E2E / install-native (macos-latest) (push) Has been cancelled
Wrap Native E2E / wrap-native (macos-latest) (push) Has been cancelled
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

227 lines
8.3 KiB
Python

"""Upstream 429 rate-limit retry + Retry-After honoring (fixes #1221).
Both the non-streaming (``server.py:_retry_request``) and streaming
(``streaming.py:_stream_response``) forwarders must retry an upstream 429 with
backoff instead of passing it straight back to the client, since a parallel
agent fan-out that exceeds the per-minute limit otherwise aborts every run.
"""
from __future__ import annotations
import asyncio
import httpx
from headroom.proxy.server import ProxyConfig, create_app
class _RateLimitTransport(httpx.AsyncBaseTransport):
"""Returns ``fail_status`` for the first ``fail_times`` calls, then 200.
Records ``calls`` so a test can assert whether a retry happened.
"""
def __init__(
self,
*,
fail_status: int = 429,
fail_times: int = 1,
retry_after: str | None = None,
sse: bool = False,
) -> None:
self.fail_status = fail_status
self.fail_times = fail_times
self.retry_after = retry_after
self.sse = sse
self.calls = 0
async def handle_async_request(self, request: httpx.Request) -> httpx.Response:
self.calls += 1
async for _ in request.stream: # drain the request body
pass
if self.calls <= self.fail_times:
headers = {"retry-after": self.retry_after} if self.retry_after is not None else {}
return httpx.Response(
self.fail_status,
headers=headers,
json={"type": "error", "error": {"type": "rate_limit_error"}},
)
if self.sse:
body = b'event: message_stop\ndata: {"type":"message_stop"}\n\n'
return httpx.Response(200, headers={"content-type": "text/event-stream"}, content=body)
return httpx.Response(
200,
json={
"id": "msg_1",
"type": "message",
"role": "assistant",
"content": [{"type": "text", "text": "ok"}],
"usage": {"input_tokens": 1, "output_tokens": 1},
},
)
def _proxy_with(transport: _RateLimitTransport, *, max_attempts: int = 3):
config = ProxyConfig(
optimize=False,
cache_enabled=False,
rate_limit_enabled=False,
cost_tracking_enabled=False,
log_requests=False,
ccr_inject_tool=False,
ccr_handle_responses=False,
ccr_context_tracking=False,
image_optimize=False,
retry_enabled=True,
retry_max_attempts=max_attempts,
retry_base_delay_ms=1,
retry_max_delay_ms=5000,
)
proxy = create_app(config).state.proxy
proxy.http_client = httpx.AsyncClient(transport=transport)
return proxy
# --- non-streaming: _retry_request ---------------------------------------
def test_retry_request_retries_429_then_succeeds() -> None:
transport = _RateLimitTransport(fail_status=429, fail_times=1, retry_after="0")
proxy = _proxy_with(transport)
resp = asyncio.run(proxy._retry_request("POST", "https://up/v1/messages", {}, {"messages": []}))
assert resp.status_code == 200
assert transport.calls == 2 # one 429 + one success — the retry happened
def test_retry_request_returns_429_verbatim_on_exhaustion() -> None:
# Always 429: must return the 429 to the client, NOT raise / convert to 5xx.
transport = _RateLimitTransport(fail_status=429, fail_times=99, retry_after="0")
proxy = _proxy_with(transport, max_attempts=3)
resp = asyncio.run(proxy._retry_request("POST", "https://up/v1/messages", {}, {"messages": []}))
assert resp.status_code == 429
assert transport.calls == 3 # exhausted all attempts
def test_retry_request_honors_retry_after(monkeypatch) -> None:
slept: list[float] = []
async def _fake_wait(self, seconds: float) -> bool: # type: ignore[no-untyped-def]
slept.append(seconds)
return False
monkeypatch.setattr(
"headroom.proxy.server.HeadroomProxy._wait_for_retry_delay_or_shutdown", _fake_wait
)
transport = _RateLimitTransport(fail_status=429, fail_times=1, retry_after="2")
proxy = _proxy_with(transport)
asyncio.run(proxy._retry_request("POST", "https://up/v1/messages", {}, {"messages": []}))
# Retry-After: 2s honored (not the ~1-10ms jittered exponential backoff).
assert slept and abs(slept[0] - 2.0) < 0.01
def test_retry_request_does_not_retry_other_4xx() -> None:
transport = _RateLimitTransport(fail_status=400, fail_times=99)
proxy = _proxy_with(transport)
resp = asyncio.run(proxy._retry_request("POST", "https://up/v1/messages", {}, {"messages": []}))
assert resp.status_code == 400
assert transport.calls == 1 # 4xx (non-429) still short-circuits — no retry
def test_retry_request_still_retries_5xx() -> None:
transport = _RateLimitTransport(fail_status=503, fail_times=1)
proxy = _proxy_with(transport)
resp = asyncio.run(proxy._retry_request("POST", "https://up/v1/messages", {}, {"messages": []}))
assert resp.status_code == 200
assert transport.calls == 2 # 5xx retry path unchanged
# --- streaming: _stream_response -----------------------------------------
def test_stream_response_retries_429() -> None:
transport = _RateLimitTransport(fail_status=429, fail_times=1, retry_after="0", sse=True)
proxy = _proxy_with(transport)
asyncio.run(
proxy._stream_response(
"https://up/v1/messages",
{},
{"messages": []},
"anthropic",
"claude-3",
"r1",
0,
0,
0,
[],
{},
0.0,
)
)
assert transport.calls == 2 # streaming 429 retried, not forwarded raw
# --- 529 overloaded: same transient-retry path as 429 --------------------
#
# 529 is Anthropic's ``overloaded_error``. Like 429 it means "try again
# shortly", so both forwarders must retry it honoring Retry-After. Before this
# fix the streaming path forwarded a 529 to the client raw (zero retries), and
# _retry_request retried it only via the generic 5xx path — raising on
# exhaustion instead of returning the 529 verbatim, and ignoring Retry-After.
def test_retry_request_retries_529_then_succeeds() -> None:
transport = _RateLimitTransport(fail_status=529, fail_times=1, retry_after="0")
proxy = _proxy_with(transport)
resp = asyncio.run(proxy._retry_request("POST", "https://up/v1/messages", {}, {"messages": []}))
assert resp.status_code == 200
assert transport.calls == 2 # one 529 + one success — the retry happened
def test_retry_request_returns_529_verbatim_on_exhaustion() -> None:
# Always 529: must return the 529 to the client, NOT raise / convert to 5xx.
transport = _RateLimitTransport(fail_status=529, fail_times=99, retry_after="0")
proxy = _proxy_with(transport, max_attempts=3)
resp = asyncio.run(proxy._retry_request("POST", "https://up/v1/messages", {}, {"messages": []}))
assert resp.status_code == 529
assert transport.calls == 3 # exhausted all attempts, returned verbatim
def test_retry_request_honors_retry_after_on_529(monkeypatch) -> None:
slept: list[float] = []
async def _fake_wait(self, seconds: float) -> bool: # type: ignore[no-untyped-def]
slept.append(seconds)
return False
monkeypatch.setattr(
"headroom.proxy.server.HeadroomProxy._wait_for_retry_delay_or_shutdown", _fake_wait
)
transport = _RateLimitTransport(fail_status=529, fail_times=1, retry_after="2")
proxy = _proxy_with(transport)
asyncio.run(proxy._retry_request("POST", "https://up/v1/messages", {}, {"messages": []}))
# Retry-After: 2s honored for 529 just like 429.
assert slept and abs(slept[0] - 2.0) < 0.01
def test_stream_response_retries_529() -> None:
# The gap this PR closes: an interactive (streaming) session hitting a 529
# used to get "Overloaded" surfaced immediately, with no retry.
transport = _RateLimitTransport(fail_status=529, fail_times=1, retry_after="0", sse=True)
proxy = _proxy_with(transport)
asyncio.run(
proxy._stream_response(
"https://up/v1/messages",
{},
{"messages": []},
"anthropic",
"claude-3",
"r1",
0,
0,
0,
[],
{},
0.0,
)
)
assert transport.calls == 2 # streaming 529 retried, not forwarded raw