fix(combo): failover when upstream SSE is truncated mid-lifecycle - #7545
diegosouzapw merged 4 commits into
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Thanks for tracking down log One issue: this branch predates two fixes that already merged into release/v3.8.49 — issue #7285 (OpenAI-shape stream truncated without We're going to rebase your fix onto the current file — keeping the #7285/#1382 logic intact and adding your |
User log 1784230812441-bf3789: a combo target returned an SSE stream that carried bytes but never sent a recognised terminator (`data: [DONE]`, `message_stop`, `message_delta` with `stop_reason`, or a `finish_reason`) and never produced a single parseable SSE frame. The streaming quality validator's generic done-branch gate only checked `!sawAnyBytes`, so any byte at all — even unparseable garbage — passed the stream through. The combo did not fail over to the next target and the downstream SSE client hung waiting for events that never arrived. Rebuilt against the current release/v3.8.49 tip instead of the original branch diff: the original diff predates and deletes two fixes already merged to release — issue diegosouzapw#7285 (`OpenAiLifecycleFlags` / `applyOpenAiLifecycleEvent`, the OpenAI-shape "truncated without finish_reason" failover branch) and issue diegosouzapw#1382 (`SseLifecycleFlags .hasRealContent`, the Claude real-content vs. empty-content_block nuance). Both are preserved untouched here. Two new flags are tracked in parallel to that existing machinery instead of replacing it: * sawStructuredSSE — any parseable `event:` or `data:` frame was seen, even one carrying no recognised content (ping/metadata) — keeps the diegosouzapw#3399/diegosouzapw#3685 pass-through contract for those streams. * sawTerminator — a recognised terminator arrived: `data: [DONE]`, an OpenAI `finish_reason` (mirrors `openAi.hasTerminalMarker`), a Claude `message_stop`/`message_delta` with `stop_reason` (mirrors `sse.hasLifecycleEnd`), or a terminal `usage`-only chunk (new). The generic done-branch gate now requires neither flag to be true before marking the stream invalid, replacing the old `!sawAnyBytes` check (now dead and removed). The diegosouzapw#7285 and diegosouzapw#1382 branches are untouched. Tests added in tests/unit/validate-response-quality.test.ts (adapted from the original branch, same scenarios): 1. incomplete lifecycle (the bug) -> invalid 2. `[DONE]` only -> valid (regression guard for diegosouzapw#3685) 3. `event: ping` only -> valid (regression guard for diegosouzapw#3399) 4. OpenAI `finish_reason`-only chunk (no `[DONE]`) -> valid, isolates the new finish_reason check Full touched-area regression set verified green (51/51): the new tests plus combo-streaming-openai-no-finish-reason-7285, streaming-empty- content-block-1382, combo-quality-validator-reasoning, masked-200- exhaustion-fallback-6427, combo-streaming-empty-content-failover, combo-empty-content-failover-5085, combo-response-validation-failover, and combo-response-validation. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
cc1c735 to
3b91f9c
Compare
Resolves conflict in open-sse/services/combo/validateQuality.ts: keep both the release's isStreamingUpstreamError() early-return and this PR's terminal usage-only-chunk detection, error check taking priority. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
…lpers (complexity gate on parseAccumulatedSse) Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
…ity-gate compensation) Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
6ca3531
into
diegosouzapw:release/v3.8.49
|
Merged into |
…egosouzapw#7545) * fix(combo): failover when upstream SSE is truncated mid-lifecycle User log 1784230812441-bf3789: a combo target returned an SSE stream that carried bytes but never sent a recognised terminator (`data: [DONE]`, `message_stop`, `message_delta` with `stop_reason`, or a `finish_reason`) and never produced a single parseable SSE frame. The streaming quality validator's generic done-branch gate only checked `!sawAnyBytes`, so any byte at all — even unparseable garbage — passed the stream through. The combo did not fail over to the next target and the downstream SSE client hung waiting for events that never arrived. Rebuilt against the current release/v3.8.49 tip instead of the original branch diff: the original diff predates and deletes two fixes already merged to release — issue diegosouzapw#7285 (`OpenAiLifecycleFlags` / `applyOpenAiLifecycleEvent`, the OpenAI-shape "truncated without finish_reason" failover branch) and issue diegosouzapw#1382 (`SseLifecycleFlags .hasRealContent`, the Claude real-content vs. empty-content_block nuance). Both are preserved untouched here. Two new flags are tracked in parallel to that existing machinery instead of replacing it: * sawStructuredSSE — any parseable `event:` or `data:` frame was seen, even one carrying no recognised content (ping/metadata) — keeps the diegosouzapw#3399/diegosouzapw#3685 pass-through contract for those streams. * sawTerminator — a recognised terminator arrived: `data: [DONE]`, an OpenAI `finish_reason` (mirrors `openAi.hasTerminalMarker`), a Claude `message_stop`/`message_delta` with `stop_reason` (mirrors `sse.hasLifecycleEnd`), or a terminal `usage`-only chunk (new). The generic done-branch gate now requires neither flag to be true before marking the stream invalid, replacing the old `!sawAnyBytes` check (now dead and removed). The diegosouzapw#7285 and diegosouzapw#1382 branches are untouched. Tests added in tests/unit/validate-response-quality.test.ts (adapted from the original branch, same scenarios): 1. incomplete lifecycle (the bug) -> invalid 2. `[DONE]` only -> valid (regression guard for diegosouzapw#3685) 3. `event: ping` only -> valid (regression guard for diegosouzapw#3399) 4. OpenAI `finish_reason`-only chunk (no `[DONE]`) -> valid, isolates the new finish_reason check Full touched-area regression set verified green (51/51): the new tests plus combo-streaming-openai-no-finish-reason-7285, streaming-empty- content-block-1382, combo-quality-validator-reasoning, masked-200- exhaustion-fallback-6427, combo-streaming-empty-content-failover, combo-empty-content-failover-5085, combo-response-validation-failover, and combo-response-validation. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> * refactor(combo): extract consumeSseLine + isTerminalUsageOnlyChunk helpers (complexity gate on parseAccumulatedSse) Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> * refactor(combo): move parseJsonRecord to module scope (finish complexity-gate compensation) Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --------- Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
…egosouzapw#7545) * fix(combo): failover when upstream SSE is truncated mid-lifecycle User log 1784230812441-bf3789: a combo target returned an SSE stream that carried bytes but never sent a recognised terminator (`data: [DONE]`, `message_stop`, `message_delta` with `stop_reason`, or a `finish_reason`) and never produced a single parseable SSE frame. The streaming quality validator's generic done-branch gate only checked `!sawAnyBytes`, so any byte at all — even unparseable garbage — passed the stream through. The combo did not fail over to the next target and the downstream SSE client hung waiting for events that never arrived. Rebuilt against the current release/v3.8.49 tip instead of the original branch diff: the original diff predates and deletes two fixes already merged to release — issue diegosouzapw#7285 (`OpenAiLifecycleFlags` / `applyOpenAiLifecycleEvent`, the OpenAI-shape "truncated without finish_reason" failover branch) and issue diegosouzapw#1382 (`SseLifecycleFlags .hasRealContent`, the Claude real-content vs. empty-content_block nuance). Both are preserved untouched here. Two new flags are tracked in parallel to that existing machinery instead of replacing it: * sawStructuredSSE — any parseable `event:` or `data:` frame was seen, even one carrying no recognised content (ping/metadata) — keeps the diegosouzapw#3399/diegosouzapw#3685 pass-through contract for those streams. * sawTerminator — a recognised terminator arrived: `data: [DONE]`, an OpenAI `finish_reason` (mirrors `openAi.hasTerminalMarker`), a Claude `message_stop`/`message_delta` with `stop_reason` (mirrors `sse.hasLifecycleEnd`), or a terminal `usage`-only chunk (new). The generic done-branch gate now requires neither flag to be true before marking the stream invalid, replacing the old `!sawAnyBytes` check (now dead and removed). The diegosouzapw#7285 and diegosouzapw#1382 branches are untouched. Tests added in tests/unit/validate-response-quality.test.ts (adapted from the original branch, same scenarios): 1. incomplete lifecycle (the bug) -> invalid 2. `[DONE]` only -> valid (regression guard for diegosouzapw#3685) 3. `event: ping` only -> valid (regression guard for diegosouzapw#3399) 4. OpenAI `finish_reason`-only chunk (no `[DONE]`) -> valid, isolates the new finish_reason check Full touched-area regression set verified green (51/51): the new tests plus combo-streaming-openai-no-finish-reason-7285, streaming-empty- content-block-1382, combo-quality-validator-reasoning, masked-200- exhaustion-fallback-6427, combo-streaming-empty-content-failover, combo-empty-content-failover-5085, combo-response-validation-failover, and combo-response-validation. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> * refactor(combo): extract consumeSseLine + isTerminalUsageOnlyChunk helpers (complexity gate on parseAccumulatedSse) Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> * refactor(combo): move parseJsonRecord to module scope (finish complexity-gate compensation) Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --------- Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
Summary
Fix combo silent-stop fallback: when a combo target's upstream SSE stream
ends mid-lifecycle without a recognised terminator (
data: [DONE],message_stop,finish_reason, or terminalusage), the combo nowfails over to the next target instead of returning a half-finished
stream that hangs the downstream SSE parser.
Before this fix the streaming quality validator in
open-sse/services/combo/validateQuality.tshad a!sawAnyBytesgatethat any byte — even unparseable garbage or a partial
data:payload —satisfied. The validator returned
valid: true, the combo passed thetruncated stream through, and OpenCode (or any OpenAI-compatible SSE
client) hung waiting for events that never came.
The gate is now tightened to require the stream to have either seen a
recognised terminator (
sawTerminator) or any parseable SSE framing(
sawStructuredSSE). Only when neither is true does the stream getclassified as invalid for combo failover. The #3399/#3685 pass-through
contract for ping/metadata/[DONE]-only streams is preserved.
Related Issues
1784230812441-bf3789; no public GitHub issue exists)Streaming combo returns "[Proxy Error] The upstream API returned an empty response" instead of failing over #3685 (incomplete Claude lifecycle pass-through)
Validation
node --import tsx/esm --test tests/unit/validate-response-quality.test.ts— 12/12 passnode --import tsx/esm --test tests/unit/combo-quality-validator-reasoning.test.ts— passnode --import tsx/esm --test tests/unit/masked-200-exhaustion-fallback-6427.test.ts— passnode --import tsx/esm --test tests/unit/combo-streaming-empty-content-failover.test.ts tests/unit/combo-empty-content-failover-5085.test.ts tests/unit/combo-response-validation-failover.test.ts tests/unit/combo-response-validation.test.ts— 20/20 passprettier --writeon touched files (clean)eslinton touched files (clean)npm run lint(full repo)npm run test:unit(full repo)npm run test:coverage>= 60%for statements, lines, functions, and branchesTests Added Or Updated
tests/unit/validate-response-quality.test.ts— 4 new regressiontests:
bytes with no terminator and no structured SSE) →valid: false(the bug, reproduced from the user log)data: [DONE]only (no content) → stillvalid: true(regression guard for Streaming combo returns "[Proxy Error] The upstream API returned an empty response" instead of failing over #3685)
event: pingonly (no content, no terminator) → stillvalid: true(regression guard for fix: server-side context cache pinning, stop proxy message leaks, persist context_cache_protection toggle #3399)
finish_reason-only chunk (no content delta, no trailing[DONE]) →valid: true(isolates the newfinish_reasoncheck)Coverage Notes
The touched files (
open-sse/services/combo/validateQuality.tsandtests/unit/validate-response-quality.test.ts) are covered by the newtest cases and the existing
combo-streaming-empty-content-failoversuite. The new flags (
sawTerminator,sawStructuredSSE) and thetightened done-branch guard are exercised by all four new tests; the
existing #3685 tests continue to confirm the pass-through contract for
incomplete Claude lifecycles.
Reviewer Notes
validateResponseQuality(lines ~106–327). Non-streaming path isuntouched. All three combo call sites
(
combo.ts::handleComboChat,combo.ts::handleRoundRobinCombo,runtimeUnits.ts) share the validator, so the fix propagatesautomatically.
#3399(ping/metadatapass-through) and
#3685(incomplete Claude lifecyclepass-through) contracts are explicitly preserved via
sawStructuredSSEand
sawTerminator; see the comment block at the flag declarationin
validateQuality.ts.parseAccumulatedSse:data: [DONE]is now marked as a terminator (previously skipped)finish_reasonin any choice delta is now trackedusage-only chunks (nochoices, not a Responses APIevent) are now tracked
message_stopandmessage_deltawithstop_reasonareco-located with the existing
hasLifecycleEndupdates(more streams are now classified as invalid); the four new tests are
designed to catch any over-tightening regression. The 27 existing
validator / combo-quality / failover tests still pass.
four new tests reproduce the user's exact failure mode at the unit
level.