1 commit

Author	SHA1	Message	Date
Manmit Singh	2ce19c2c55	fix(proxy): retry HTTP/2 stream resets instead of 502ing (#1645) ... ## Description Under concurrent load with large request bodies, `/v1/messages` returns **HTTP 502**. A single upstream HTTP/2 stream reset poisons the shared h2 connection and raises `RemoteProtocolError` (`StreamReset`) / `LocalProtocolError` on every other in-flight stream: ``` ERROR [hr_...] Request failed: RemoteProtocolError: <StreamReset stream_id:35, error_code:1, remote_reset:True> ERROR [hr_...] Request failed: LocalProtocolError: 39 INFO event=proxy_inbound_response ... status=502 duration_ms=78712 ``` These are transport errors, but they weren't in the proxy's retry paths — the non-streaming `_retry_request` caught `(ConnectError, TimeoutException, HTTPStatusError)` and the streaming connect loop caught `(ConnectError, ConnectTimeout, PoolTimeout)`. So a stream reset skipped retry entirely and fell through to the broad handler catch as a `502`, with no reconnect. This broadens both retry paths to treat any `httpx.TransportError` — which includes the h2 `Local`/`RemoteProtocolError` — as retryable, so the poisoned connection is dropped and the request re-sent on a fresh one. Closes #1639 > Scope note: the issue also mentions `HEADROOM_HTTP2` being ignored on the `headroom install agent run` launch path. That's a separate config-plumbing gap; I've kept this PR to the 502-cascade fix (which makes the chain self-recover regardless of the env workaround) and am happy to follow up on the env plumbing separately. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made - `headroom/proxy/server.py` (`_retry_request`): the retry `except` now catches `(httpx.TransportError, httpx.HTTPStatusError)` instead of `(ConnectError, TimeoutException, HTTPStatusError)`. `TransportError` is the common base of ConnectError, the timeout family, and the protocol/network errors — so h2 stream resets are retried with backoff. - `headroom/proxy/handlers/streaming.py`: the streaming connect-retry loop and its terminal handler now catch `httpx.TransportError`. The retry runs before any body byte is forwarded to the client (only `build_request` + `send(stream=True)` are inside the loop), so re-sending is safe. On exhaustion the terminal handler still emits a clean `event: error` SSE instead of letting the reset bubble up as a 502. The mid-stream handler was left as-is (already covered by its `except Exception`, and not safe to retry once bytes have been sent). ## 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_h2_stream_reset_retry.py -q 4 passed $ pytest tests/test_proxy_streaming_resilience.py tests/test_mid_turn_steering.py \ tests/test_proxy_streaming_ratelimit_headers.py tests/test_streaming_usage_parser.py \ tests/test_proxy_byte_faithful_forwarding.py -q 87 passed, 1 skipped $ ruff check <changed files> && ruff format --check <changed files> All checks passed! / 3 files already formatted $ mypy headroom/proxy/server.py headroom/proxy/handlers/streaming.py --ignore-missing-imports Success: no issues found in 2 source files ``` ## Real Behavior Proof - Environment: macOS (arm64), Python 3.14 venv, editable install of this branch. - Exact command / steps: ran `pytest tests/test_h2_stream_reset_retry.py` — the tests drive the real `_retry_request` and `_stream_response` with `http_client.post` / `http_client.send` set to raise `httpx.RemoteProtocolError("<StreamReset ...>")` on the first attempt and return a good response on the second. - Observed result: non-streaming — the request is retried and returns the `200` response (`post` awaited twice); on unconditional resets it re-raises after `retry_max_attempts` (no silent hang). Streaming — the reset on `send()` is retried and the upstream SSE (`message_start`…) is forwarded with no `connection_error` event (`send` awaited twice); on repeated resets a clean `event: error` SSE is emitted rather than a crash/502. Before this change the same `RemoteProtocolError` was uncaught and propagated to the `502` handler. - Not tested: a live 10-session concurrent-load repro against a real Anthropic h2 endpoint — reproduced deterministically at the retry boundary with an injected `RemoteProtocolError` instead. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Additional Notes Retrying a stream reset re-sends the (potentially large) body, but that is bounded by the existing `retry_max_attempts` + jittered backoff and only happens before the first client byte — the same contract the existing connect-error retry already relied on. This is complementary to, not a replacement for, an operator forcing HTTP/1.1; it makes the default h2 path self-heal from transient resets. Co-authored-by: JerrettDavis <mxjerrett@gmail.com>	2026-07-07 11:31:06 -05:00