headroom/tests/test_anthropic_buffered_sse.py

Ignoring revisions in .git-blame-ignore-revs. Click here to bypass and see the normal blame view.

319 lines
11 KiB
Python
Raw Permalink Normal View History

fix(proxy/anthropic): stop answering a non-streaming turn with an event stream (#3142) ## Description Closes #3130. Unifies #3131 (@Joaovsales) and #3132 (@taiseii), which landed within hours of each other on the same bug. Neither is redundant — **#3131 contributed the clearest statement of the contract; #3132 contributed the reconstruction that can actually be trusted to satisfy it.** This takes both. A caller that sent `stream: false` was handed a `text/event-stream` body at HTTP 200. The reply was complete — 8756 bytes, a valid upstream `request-id` — it was simply wearing a wire format the SDK cannot parse, so the turn was lost. **On root cause.** #3130 says outright: *"I could not pin down why the upstream answered a `stream`-less request with an event stream."* I think this does. At `v0.35.0` the CCR path flips the body to `stream: false` and never touches the client's `Accept` header — I checked the tag and the count of Accept rewrites at that site is **zero**. So upstream receives a self-contradicting request: *"answer as JSON"* in the body, *"I only accept SSE"* in the headers. Both reporters (#3130, #3140) show `server: cloudflare` / `cf-ray`, and both describe it as intermittent — consistent with an edge honouring `Accept` under retry. #3102 fixed that for the CCR flip; this PR moves the rewrite to the buffered boundary **every** non-streaming request reaches, so the client's own non-streaming retry is covered too. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made **From #3131 — the contract.** `headroom/proxy/nonstream_sse_policy.py`: a pure module with a behaviour matrix and `should_recover_sse_reply` as a single predicate. The three negative arms are deliberate — a streaming caller wants SSE, a JSON content-type is already correct, a non-200 carries an upstream error the client should see verbatim. **From #3132 — the reconstruction.** `require_complete=True` demands `message_start`, a terminal `message_stop`, every opened block closed, no in-band `error` event, and no delta the reconstructor cannot replay. Anything short of that is a 502. Three things only #3132 had, each load-bearing: - **`index` is stripped from rebuilt content blocks.** The parser writes it (`streaming.py:425`) and a client persists the reconstructed turn and echoes it back — at which point Anthropic 400s with `content.0.text.index: Extra inputs are not permitted`. `_strip_streaming_only_content_fields` (`anthropic.py:185`) already documents this exact failure. That inbound stripper would mask it *while the proxy is in the path*, but the client's stored history is still polluted. - **SSE framing is normalized and `data:` no longer requires the optional space.** The old `startswith("data: ")` skipped a spec-valid stream **entirely** — zero events parsed, which is literally what the report describes (*"0 stream events received"*). - **Detection sniffs the body**, so a mislabeled or absent content-type is still caught. **Reconciled where they disagreed:** - *Headers.* #3131 hand-rolled a framing list; this uses the established `sanitize_forwarded_response_headers`. That already strips `connection`, `keep-alive` and `server` alongside the content-* family — and per the comment at `helpers.py:325`, leaving `transfer-encoding` on a rebuilt body is what produced an empty HTTP 200 in #3019. #3131's list would have left three of those on. #3132's `cf-*` filter is kept. - *Detection.* The body sniff arrives as `body_is_event_stream`, so the policy module stays pure — the sniff needs the response object and the handler owns that. - Dropped #3131's `json_reply_headers` and its test class; everything else from both PRs is retained. ## Testing - [x] Unit tests pass (`pytest`) - [x] Integration tests pass ### Test Output ```text tests/test_nonstream_sse_policy.py 18 passed (from #3131) tests/test_anthropic_buffered_sse.py 18 passed (from #3132) 36 passed Regression sweep (-k "stream or sse or ccr or anthropic or proxy or buffered or usage"): 2982 passed, 181 skipped, 0 failed in 153.41s ruff check: All checks passed ruff format --check: 527 files already formatted ``` Both contributors' suites are kept whole and both pass unmodified against the merged implementation, which is the useful signal here — they were written independently against different implementations. ## Real Behavior Proof - Environment: macOS (darwin 25.4.0), Python 3.12.13, worktree off `main`, `_core.abi3.so` copied in. - Exact command / steps: applied #3132 as the engine, layered #3131's policy module over it, rewired the decision site to the predicate, then ran both suites and a 2982-test sweep concentrated on everything touching the shared SSE parser. - Observed result: 36/36 across both contributed suites, 2982 passed / 0 failed on the sweep. The sweep matters more than usual here — `_parse_sse_to_response` is shared with the streaming path's usage accounting, and `require_complete` defaults to `False` specifically so existing callers keep the lenient reconstruction they were written against. Nothing regressed. - Not tested: no live upstream. I could not reproduce the upstream answering a `stream`-less request with SSE against real `api.anthropic.com` — that is the condition #3130 reports as intermittent and load-dependent, and the Accept explanation above remains a well-supported hypothesis rather than something I observed. The fix does not depend on it: whatever the upstream returns, a caller that did not ask for streaming is no longer handed SSE. ## Runtime Rollout Safety - Rollout-managed feature(s): None. - Minimum rollout channel: n/a - Stable/default behavior changed: Yes, deliberately, in two places. A non-streaming turn answered with SSE is now reconstructed as JSON instead of relayed; an SSE reply that cannot be faithfully reconstructed is now a 502 instead of an unparseable 200. Both are the point. `require_complete` defaults to `False`, so streaming callers of the shared parser are untouched. - Kill switch / disable path: none by design — relaying a body the client cannot parse has no legitimate mode. - Unsafe override required: No. - Qualification impact: A truncated upstream stream now surfaces as an explicit 502 rather than a short-but-successful turn. More visible failures, fewer silent ones. - Rollback path: Revert the commit. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Additional Notes If this lands, #3131 and #3132 should be closed as superseded — both authors are credited via `Co-authored-by:` and their tests ship intact. I would not close either before a maintainer agrees this unification is the direction, since it discards a design decision from each. **Wider context, not fixed here:** #3130 and #3140 both report against **0.35.0**, and `main` already carries a stack of fixes for this symptom class that has never shipped — #3102 (Accept), #3092, #3091, #3094, #3101, #3069, #3084, #3124, #3134. All of them are gated behind #3067 `chore: release 0.36.0`. Every closed lookalike (#3019, #3055, #3071, #3040, #2952) was fixed into that same unreleased window. Merging this PR does not help either reporter until 0.36.0 ships; **cutting that release is the higher-leverage action.** The interim workaround for anyone on 0.35.0 is `HEADROOM_NO_CCR=1` — the buffered flip is gated on `_has_headroom_retrieve_tool`, and `no_ccr` stops the tool being injected, so the flip never engages. Note `headroom wrap` has no `--no-ccr` flag in 0.35.0, so it has to be the env var. Co-authored-by: Tejas Chopra <tejas@Tejass-MacBook-Pro.local> Co-authored-by: João Souto <73318835+Joaovsales@users.noreply.github.com> Co-authored-by: taiseii <37083727+taiseii@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-08-19 20:45:38 -07:00
"""A non-streaming turn must never be answered with an event stream (#3130).
Claude Code retries a failed streaming turn as ``stream: false``. The buffered
Anthropic path forwarded the upstream response headers wholesale, so when the
upstream answered that JSON request with ``content-type: text/event-stream``
the SDK got a wire format it never asked for and lost a complete, already-paid
turn:
API returned an empty or malformed response (HTTP 200) ... content-type
event-stream, body is an event stream (the non-streaming request was
answered with a stream), 8756 bytes
Two defects, fixed on both sides:
* the request went out contradicting itself ``stream: false`` in the body,
``Accept: text/event-stream`` in the headers (the narrow CCR-only rewrite
from #3078 never covered a client-originated non-stream turn), and
* the response was relayed verbatim instead of being adapted to the JSON the
caller asked for.
Reconstruction is deliberately strict: a partial stream must fail loudly as a
502 rather than be handed back as a successful and silently truncated
message.
"""
from __future__ import annotations
import json
import pytest
fastapi = pytest.importorskip("fastapi")
httpx = pytest.importorskip("httpx")
from fastapi.testclient import TestClient # noqa: E402
from headroom.proxy.server import ProxyConfig, create_app # noqa: E402
COMPLETE_SSE = (
"event: message_start\n"
'data: {"type":"message_start","message":{"id":"msg_1","type":"message",'
'"role":"assistant","model":"claude-sonnet-4-6","content":[],'
'"stop_reason":null,"stop_sequence":null,"usage":{"input_tokens":10,'
'"output_tokens":1,"cache_read_input_tokens":2,'
'"cache_creation_input_tokens":3}}}\n\n'
"event: content_block_start\n"
'data: {"type":"content_block_start","index":0,'
'"content_block":{"type":"text","text":""}}\n\n'
"event: content_block_delta\n"
'data: {"type":"content_block_delta","index":0,'
'"delta":{"type":"text_delta","text":"hello"}}\n\n'
"event: content_block_stop\n"
'data: {"type":"content_block_stop","index":0}\n\n'
"event: message_delta\n"
'data: {"type":"message_delta","delta":{"stop_reason":"end_turn",'
'"stop_sequence":null},"usage":{"output_tokens":5}}\n\n'
"event: message_stop\n"
'data: {"type":"message_stop"}\n\n'
)
# Everything up to — but not including — the terminal event.
TRUNCATED_SSE = COMPLETE_SSE.split("event: message_delta")[0]
ERROR_SSE = (
COMPLETE_SSE.split("event: message_delta")[0] + "event: error\n"
'data: {"type":"error","error":{"type":"overloaded_error",'
'"message":"Overloaded"}}\n\n'
)
JSON_REPLY = {
"id": "msg_1",
"type": "message",
"role": "assistant",
"model": "claude-sonnet-4-6",
"content": [{"type": "text", "text": "hello"}],
"stop_reason": "end_turn",
"usage": {
"input_tokens": 10,
"output_tokens": 5,
"cache_read_input_tokens": 0,
"cache_creation_input_tokens": 0,
},
}
def _config() -> ProxyConfig:
return ProxyConfig(
optimize=False,
cache_enabled=False,
rate_limit_enabled=False,
memory_enabled=False,
ccr_inject_tool=False,
ccr_handle_responses=False,
ccr_context_tracking=False,
image_optimize=False,
)
def _drive(
*,
upstream: httpx.Response,
accept: str | None = "text/event-stream",
stream: bool = False,
) -> tuple[httpx.Response, dict[str, object]]:
"""Run one turn against a canned upstream reply.
Returns the client-facing response and what went upstream.
"""
seen: dict[str, object] = {}
app = create_app(_config())
with TestClient(app) as client:
proxy = client.app.state.proxy
async def _fake_retry(method, url, headers, body, stream=False, **kwargs): # noqa: ANN001
sent = json.loads(body) if isinstance(body, (str, bytes)) else body
seen["stream"] = sent.get("stream")
seen["headers"] = dict(headers or {})
return upstream
proxy._retry_request = _fake_retry # type: ignore[assignment]
headers = {"x-api-key": "test-key", "anthropic-version": "2023-06-01"}
if accept is not None:
headers["accept"] = accept
resp = client.post(
"/v1/messages",
json={
"model": "claude-sonnet-4-6",
"max_tokens": 64,
"stream": stream,
"messages": [{"role": "user", "content": "go"}],
},
headers=headers,
)
return resp, seen
def _sse_response(body: str, **extra_headers: str) -> httpx.Response:
headers = {"content-type": "text/event-stream", **extra_headers}
return httpx.Response(200, content=body.encode(), headers=headers)
def _accepts(headers: dict) -> list[str]:
return [v for k, v in headers.items() if k.lower() == "accept"]
# --------------------------------------------------------------------------- #
# Request side: a stream:false body must not carry an SSE-only Accept
# --------------------------------------------------------------------------- #
def test_non_stream_turn_asks_upstream_for_json() -> None:
_, seen = _drive(upstream=httpx.Response(200, json=JSON_REPLY))
assert seen["stream"] is False
assert _accepts(seen["headers"]) == ["application/json"] # type: ignore[arg-type]
def test_non_stream_turn_replaces_rather_than_appends_accept() -> None:
_, seen = _drive(upstream=httpx.Response(200, json=JSON_REPLY), accept="TEXT/EVENT-STREAM")
values = _accepts(seen["headers"]) # type: ignore[arg-type]
assert values == ["application/json"]
assert not any("event-stream" in v.lower() for v in values)
def test_non_stream_turn_without_client_accept_still_asks_for_json() -> None:
_, seen = _drive(upstream=httpx.Response(200, json=JSON_REPLY), accept=None)
assert _accepts(seen["headers"]) == ["application/json"] # type: ignore[arg-type]
# --------------------------------------------------------------------------- #
# Response side: SSE at 200 for a JSON request is adapted, not relayed
# --------------------------------------------------------------------------- #
def test_event_stream_answer_is_adapted_to_json() -> None:
resp, _ = _drive(upstream=_sse_response(COMPLETE_SSE))
assert resp.status_code == 200
assert "application/json" in resp.headers["content-type"]
body = resp.json()
assert body["type"] == "message"
assert body["content"] == [{"type": "text", "text": "hello"}]
assert body["stop_reason"] == "end_turn"
def test_adapted_reply_preserves_usage_for_accounting() -> None:
resp, _ = _drive(upstream=_sse_response(COMPLETE_SSE))
usage = resp.json()["usage"]
assert usage["input_tokens"] == 10
assert usage["output_tokens"] == 5
assert usage["cache_read_input_tokens"] == 2
assert usage["cache_creation_input_tokens"] == 3
def test_adapted_reply_carries_no_streaming_only_index() -> None:
"""``index`` is a response-delta field; Anthropic rejects it on replay."""
resp, _ = _drive(upstream=_sse_response(COMPLETE_SSE))
assert all("index" not in block for block in resp.json()["content"])
def test_adapted_reply_drops_cdn_and_framing_headers() -> None:
resp, _ = _drive(
upstream=_sse_response(
COMPLETE_SSE,
**{
"server": "cloudflare",
"cf-ray": "abc123",
"cf-cache-status": "DYNAMIC",
"request-id": "req_011CeC1JTMS8egPL3FBteQay",
"anthropic-ratelimit-requests-remaining": "42",
},
)
)
lowered = {k.lower() for k in resp.headers}
assert "server" not in lowered
assert not any(k.startswith("cf-") for k in lowered)
# Provenance the caller legitimately needs survives.
assert resp.headers["request-id"] == "req_011CeC1JTMS8egPL3FBteQay"
assert resp.headers["anthropic-ratelimit-requests-remaining"] == "42"
def test_truncated_event_stream_fails_loudly() -> None:
"""A partial stream is not a successful short answer."""
resp, _ = _drive(upstream=_sse_response(TRUNCATED_SSE))
assert resp.status_code == 502
assert "application/json" in resp.headers["content-type"]
assert resp.json()["error"]["type"] == "upstream_protocol_error"
def test_error_event_is_not_reported_as_success() -> None:
resp, _ = _drive(upstream=_sse_response(ERROR_SSE))
assert resp.status_code == 502
assert resp.json()["error"]["type"] == "upstream_protocol_error"
def test_plain_json_reply_is_untouched() -> None:
resp, _ = _drive(upstream=httpx.Response(200, json=JSON_REPLY))
assert resp.status_code == 200
assert "application/json" in resp.headers["content-type"]
assert resp.json()["content"] == [{"type": "text", "text": "hello"}]
# --------------------------------------------------------------------------- #
# Strict reconstruction, exercised directly
# --------------------------------------------------------------------------- #
@pytest.fixture()
def proxy():
from headroom.proxy.server import HeadroomProxy
return HeadroomProxy(_config())
def test_strict_mode_requires_a_terminal_event(proxy) -> None:
assert proxy._parse_sse_to_response(TRUNCATED_SSE, "anthropic", require_complete=True) is None
def test_strict_mode_rejects_an_error_event(proxy) -> None:
assert proxy._parse_sse_to_response(ERROR_SSE, "anthropic", require_complete=True) is None
def test_strict_mode_rejects_an_unclosed_block(proxy) -> None:
unclosed = COMPLETE_SSE.replace(
'event: content_block_stop\ndata: {"type":"content_block_stop","index":0}\n\n', ""
)
assert proxy._parse_sse_to_response(unclosed, "anthropic", require_complete=True) is None
def test_strict_mode_rejects_an_unknown_delta_type(proxy) -> None:
"""A future delta Headroom cannot replay must not pass as complete."""
unknown = COMPLETE_SSE.replace('"type":"text_delta","text":"hello"', '"type":"future_delta"')
assert proxy._parse_sse_to_response(unknown, "anthropic", require_complete=True) is None
def test_strict_mode_reads_crlf_framed_events(proxy) -> None:
parsed = proxy._parse_sse_to_response(
COMPLETE_SSE.replace("\n", "\r\n"), "anthropic", require_complete=True
)
assert parsed is not None
assert parsed["content"] == [{"type": "text", "text": "hello"}]
def test_strict_mode_keeps_stop_sequence_and_type(proxy) -> None:
parsed = proxy._parse_sse_to_response(COMPLETE_SSE, "anthropic", require_complete=True)
assert parsed is not None
assert parsed["type"] == "message"
assert parsed["stop_sequence"] is None
def test_permissive_mode_is_unchanged_for_existing_callers(proxy) -> None:
"""Streaming callers keep the lenient reconstruction they rely on."""
parsed = proxy._parse_sse_to_response(TRUNCATED_SSE, "anthropic")
assert parsed is not None
assert parsed["content"][0]["text"] == "hello"
def test_event_stream_under_a_vague_content_type_is_still_adapted() -> None:
"""A gateway may relay the stream without declaring it (#3130)."""
resp, _ = _drive(
upstream=httpx.Response(
200,
content=COMPLETE_SSE.encode(),
headers={"content-type": "application/octet-stream"},
)
)
assert resp.status_code == 200
assert "application/json" in resp.headers["content-type"]
assert resp.json()["content"] == [{"type": "text", "text": "hello"}]