mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-10 14:27:00 -04:00
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
## 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.
227 lines
8.3 KiB
Python
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
|