diff --git a/open-sse/utils/streamHandler.ts b/open-sse/utils/streamHandler.ts index e3f4ebaa1c..809c93ac55 100644 --- a/open-sse/utils/streamHandler.ts +++ b/open-sse/utils/streamHandler.ts @@ -2,6 +2,7 @@ import { trackPendingRequest } from "@/lib/usageDb"; import { STREAM_IDLE_TIMEOUT_MS } from "../config/constants.ts"; import { FORMATS } from "../translator/formats.ts"; import { PENDING_REQUEST_CLEARED_MARKER } from "./stream.ts"; +import { createStreamContentWatcher, type StreamContentWatcher } from "./streamReadiness.ts"; // Stream handler with disconnect detection - shared for all providers @@ -464,10 +465,52 @@ export function createNoopAbortWritable(): { * Create transform stream with disconnect detection * Wraps existing transform stream and adds abort capability */ +/** + * Why a finished upstream stream should still be reported as a failure, or null + * when the close was clean. Two distinct silent-close shapes: + * + * - **#7699, no terminal marker.** Scoped to Claude (`/v1/messages`), which is + * the issue's real scope: Anthropic's SSE spec permits a mid-stream + * `event: error`, and Claude clients treat a stream ending without + * `message_stop` as an error. For every other format (plain OpenAI chat + * completions included) a done-without-recognized-marker close is NOT + * necessarily a drop — many formats have no `[DONE]` equivalent — so + * synthesising an error there would be a false positive. + * + * - **#8649, no content at all.** The stream terminated properly and carried no + * model output. Unlike the marker case this is not format-dependent: a + * completed stream with zero content is a failure everywhere, and it is the + * streaming twin of the non-streaming `isEmptyContentResponse` check. Only + * applies to bodies that actually looked like SSE, and terminal states where + * emptiness is legitimate (length / tool_calls / content_filter / max_tokens / + * tool_use) are excluded by the watcher. + */ +function resolveSilentCloseReason(input: { + bytesWereForwarded: boolean; + clientTerminalSeen: boolean; + clientResponseFormat?: string | null; + contentWatcher: StreamContentWatcher; +}): string | null { + if (!input.bytesWereForwarded) return null; + + if (!input.clientTerminalSeen && input.clientResponseFormat === FORMATS.CLAUDE) { + return "Upstream stream ended without a terminal marker"; + } + + const watcher = input.contentWatcher; + if (watcher.sawSseFrame() && !watcher.sawContent() && !watcher.sawLegitEmptyTerminal()) { + return "Provider returned empty content"; + } + + return null; +} + export function createDisconnectAwareStream(transformStream, streamController) { const reader = transformStream.readable.getReader(); const writer = transformStream.writable.getWriter(); const terminalDecoder = new TextDecoder(); + const contentDecoder = new TextDecoder(); + const contentWatcher = createStreamContentWatcher(); let terminalTail = ""; let clientTerminalSeen = false; let bytesWereForwarded = false; @@ -475,6 +518,9 @@ export function createDisconnectAwareStream(transformStream, streamController) { const noteClientChunk = (chunk: unknown) => { if (!(chunk instanceof Uint8Array)) return; bytesWereForwarded = true; + // Runs past clientTerminalSeen: the frame that carries the terminal marker + // can carry the only content too, and #8649 needs the whole stream scanned. + contentWatcher.note(contentDecoder.decode(chunk, { stream: true })); if (clientTerminalSeen) return; terminalTail += terminalDecoder.decode(chunk, { stream: true }); @@ -501,29 +547,21 @@ export function createDisconnectAwareStream(transformStream, streamController) { try { const { done, value } = await reader.read(); if (done) { - // #7699 — upstream ended without a client-visible terminal marker. - // Scoped to Claude (/v1/messages) specifically, which is the - // issue's real scope: Anthropic's SSE spec permits a mid-stream - // event: error and Claude clients (Claude Code, Anthropic SDK) - // treat a stream that ends without message_stop as an error. For - // every other format (plain OpenAI chat completions included — - // see #7699's "Suggested Fix") a done-without-recognized-marker - // close is NOT necessarily a silent drop (many providers/formats - // legitimately have no [DONE]/response.completed equivalent), so - // injecting a synthetic error there would be a false positive. - if ( - bytesWereForwarded && - !clientTerminalSeen && - streamController.clientResponseFormat === FORMATS.CLAUDE - ) { + contentWatcher.finish(); + const silentCloseReason = resolveSilentCloseReason({ + bytesWereForwarded, + clientTerminalSeen, + clientResponseFormat: streamController.clientResponseFormat, + contentWatcher, + }); + + if (silentCloseReason) { streamController.handleError( - Object.assign(new Error("Upstream stream ended without a terminal marker"), { - statusCode: 502, - }) + Object.assign(new Error(silentCloseReason), { statusCode: 502 }) ); try { for (const chunk of buildStreamErrorChunks( - "Upstream stream ended without a terminal marker", + silentCloseReason, 502, streamController.clientResponseFormat )) { diff --git a/open-sse/utils/streamReadiness.ts b/open-sse/utils/streamReadiness.ts index 7b74f54be3..4b76eafd65 100644 --- a/open-sse/utils/streamReadiness.ts +++ b/open-sse/utils/streamReadiness.ts @@ -140,6 +140,95 @@ export function hasUsefulStreamContent(text: string): boolean { return false; } +// Terminal states where a completion legitimately carries no content, kept in +// step with errorClassifier.ts's LEGIT_EMPTY_OPENAI_FINISH / LEGIT_EMPTY_CLAUDE_STOP +// so the streaming and non-streaming empty-content checks agree. +const LEGIT_EMPTY_TERMINAL_REASONS = new Set([ + "length", + "tool_calls", + "content_filter", + "max_tokens", + "tool_use", +]); + +const TERMINAL_REASON_PATTERN = /"(?:finish_reason|stop_reason)"\s*:\s*"([^"]+)"/g; + +const SSE_FIELD_LINE = /(?:^|\r?\n)\s*(?:data|event):/; + +export type StreamContentWatcher = { + /** Feed a decoded slice of the client-facing stream. Safe to call with partial frames. */ + note: (text: string) => void; + /** Flush any buffered trailing frame; call once the stream is done. */ + finish: () => void; + /** True once any frame carried real model output (text, reasoning, or a tool call). */ + sawContent: () => boolean; + /** True once a terminal state was seen where emitting no content is valid. */ + sawLegitEmptyTerminal: () => boolean; + /** + * True once the stream looked like SSE at all. Not every body reaching the + * client wrapper is event-stream — a plain JSON completion is forwarded + * through the same path — and a non-SSE body has no `data:` frames to judge, + * so callers must not read emptiness into it. + */ + sawSseFrame: () => boolean; +}; + +/** + * Watch a client-facing SSE stream for whether it ever produced actual model + * output, so a stream that terminates cleanly while carrying nothing can be + * reported instead of closing as a silent empty turn (#8649). + * + * Frames are buffered until a blank-line boundary so a delta split across two + * network chunks is still scanned as one payload. The buffer is bounded — a + * single frame larger than the cap is scanned in pieces, which can only ever + * lose content-detection precision in the direction of "saw content", never + * toward a false empty. + */ +export function createStreamContentWatcher(): StreamContentWatcher { + const MAX_BUFFERED = 64 * 1024; + let pending = ""; + let content = false; + let legitEmpty = false; + let sse = false; + + const inspect = (frame: string): void => { + if (!frame) return; + if (!sse && SSE_FIELD_LINE.test(frame)) sse = true; + if (!content && hasUsefulStreamContent(frame)) content = true; + if (legitEmpty) return; + for (const match of frame.matchAll(TERMINAL_REASON_PATTERN)) { + if (LEGIT_EMPTY_TERMINAL_REASONS.has(match[1])) { + legitEmpty = true; + return; + } + } + }; + + return { + note(text: string): void { + if (!text) return; + pending += text; + for (;;) { + const boundary = pending.search(/\r?\n\r?\n/); + if (boundary === -1) break; + inspect(pending.slice(0, boundary)); + pending = pending.slice(boundary).replace(/^\r?\n\r?\n/, ""); + } + if (pending.length > MAX_BUFFERED) { + inspect(pending); + pending = ""; + } + }, + finish(): void { + inspect(pending); + pending = ""; + }, + sawContent: () => content, + sawLegitEmptyTerminal: () => legitEmpty, + sawSseFrame: () => sse, + }; +} + type StreamReadinessSignalState = { currentEvent: string; dataLines: string[]; diff --git a/tests/unit/empty-stream-no-content-8649.test.ts b/tests/unit/empty-stream-no-content-8649.test.ts new file mode 100644 index 0000000000..239997972d --- /dev/null +++ b/tests/unit/empty-stream-no-content-8649.test.ts @@ -0,0 +1,190 @@ +/** + * #8649 — a stream that terminates correctly but carries no content must not + * close silently. + * + * The non-streaming path already refuses this: `isEmptyContentResponse` rewrites + * a 200-with-no-content into a 502 "Provider returned empty content", which the + * combo layer then treats as a model-level transient and fails over (#5085). + * + * The streaming path has no equivalent. Two guards exist and neither applies: + * + * - `ensureStreamReadiness` is a LIVENESS probe. Its own failure message is + * "Stream ended before producing a non-ping SSE event" — any structured + * non-ping frame satisfies it, including a bare `delta:{"role":"assistant"}`. + * - `createDisconnectAwareStream`'s #7699 branch fires on a MISSING terminal + * marker and is scoped to the Claude client format, because for other + * formats a marker-less close is not necessarily a drop. + * + * The reported stream trips neither: it is OpenAI-format, it terminates with + * `finish_reason: "stop"` and `[DONE]`, and it contains nothing. The client + * (issue #8649: an `auto/*` combo landing on an uncredentialed backend) sees a + * clean empty assistant turn and retries to its cap with no error to stop on. + * + * "Terminated normally but emitted zero content" is unambiguous in every format, + * unlike the marker case — so this guard is deliberately format-agnostic. The + * legitimate-empty terminal states are carved out to match the non-streaming + * predicate's `LEGIT_EMPTY_OPENAI_FINISH` / `LEGIT_EMPTY_CLAUDE_STOP`. + */ +import test from "node:test"; +import assert from "node:assert/strict"; + +const { createDisconnectAwareStream, createStreamController } = + await import("../../open-sse/utils/streamHandler.ts"); +const { FORMATS } = await import("../../open-sse/translator/formats.ts"); + +function noopAbortWritable(): { getWriter: () => { abort: () => Promise } } { + return { getWriter: () => ({ abort: () => Promise.resolve() }) }; +} + +async function drainStream(stream: ReadableStream): Promise { + const reader = stream.getReader(); + const parts: Uint8Array[] = []; + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + parts.push(value); + } + return new TextDecoder().decode( + parts.reduce((acc, c) => { + const merged = new Uint8Array(acc.length + c.length); + merged.set(acc, 0); + merged.set(c, acc.length); + return merged; + }, new Uint8Array(0)) + ); +} + +/** Run `frames` through the disconnect-aware wrapper and return what the client sees. */ +async function runClientStream(frames: string[], format: string | null): Promise { + const upstream = new ReadableStream({ + start(controller) { + const encoder = new TextEncoder(); + for (const frame of frames) controller.enqueue(encoder.encode(frame)); + controller.close(); + }, + }); + const transform = new TransformStream({ + transform(chunk, controller) { + controller.enqueue(chunk); + }, + }); + const sc = createStreamController({ + provider: "test", + model: "test-model", + clientResponseFormat: format, + }); + return drainStream( + createDisconnectAwareStream( + { readable: upstream.pipeThrough(transform), writable: noopAbortWritable() }, + sc + ) + ); +} + +const chunk = (delta: string, finish: string) => + `data: {"id":"chatcmpl-x","object":"chat.completion.chunk","model":"m","choices":[{"index":0,"delta":${delta},"finish_reason":${finish}}]}\n\n`; +const DONE = "data: [DONE]\n\n"; + +test("#8649 an OpenAI stream that finishes with stop but no content surfaces an error", async () => { + const text = await runClientStream( + [chunk('{"role":"assistant"}', "null"), chunk("{}", '"stop"'), DONE], + null + ); + + assert.match( + text, + /"finish_reason":\s*"error"/, + "a completed-but-contentless stream must surface an error, not a clean empty turn" + ); + assert.match(text, /empty|no content/i); +}); + +test("#8649 a stream that carries content is passed through untouched", async () => { + const text = await runClientStream( + [ + chunk('{"role":"assistant"}', "null"), + chunk('{"content":"hello"}', "null"), + chunk("{}", '"stop"'), + DONE, + ], + null + ); + + assert.match(text, /"content":"hello"/); + assert.doesNotMatch(text, /"finish_reason":\s*"error"/, "a healthy stream must not be rewritten"); +}); + +test("#8649 a tool-call-only stream is a legitimate completion, not an empty one", async () => { + const toolDelta = + '{"tool_calls":[{"index":0,"id":"c1","function":{"name":"f","arguments":"{}"}}]}'; + const text = await runClientStream( + [ + chunk('{"role":"assistant"}', "null"), + chunk(toolDelta, "null"), + chunk("{}", '"tool_calls"'), + DONE, + ], + null + ); + + assert.match(text, /tool_calls/); + assert.doesNotMatch(text, /"finish_reason":\s*"error"/, "a tool-call turn must not be flagged"); +}); + +test("#8649 finish_reason length with no content is a legitimate truncation, not an error", async () => { + // Mirrors LEGIT_EMPTY_OPENAI_FINISH in errorClassifier.ts — a response + // truncated at the token limit is a valid terminal state even with no text. + const text = await runClientStream( + [chunk('{"role":"assistant"}', "null"), chunk("{}", '"length"'), DONE], + null + ); + + assert.doesNotMatch( + text, + /"finish_reason":\s*"error"/, + "a token-limit truncation must not be rewritten as an error" + ); +}); + +test("#8649 finish_reason content_filter with no content is a legitimate terminal state", async () => { + const text = await runClientStream( + [chunk('{"role":"assistant"}', "null"), chunk("{}", '"content_filter"'), DONE], + null + ); + + assert.doesNotMatch(text, /"finish_reason":\s*"error"/, "a filtered turn must not be rewritten"); +}); + +test("#8649 a reasoning-only stream counts as content (#2520)", async () => { + const text = await runClientStream( + [chunk('{"reasoning_content":"thinking out loud"}', "null"), chunk("{}", '"stop"'), DONE], + null + ); + + assert.doesNotMatch( + text, + /"finish_reason":\s*"error"/, + "reasoning-only output is real model output, not an empty turn" + ); +}); + +test("#8649 a non-SSE body is never judged empty — it has no frames to judge", async () => { + // Not every body reaching this wrapper is event-stream; a plain completion is + // forwarded through the same path. It has no `data:` frames, so "no content + // seen" says nothing about it and the guard must stay out of the way. + const text = await runClientStream(["plain forwarded bytes, no completion marker"], null); + + assert.equal(text, "plain forwarded bytes, no completion marker"); + assert.doesNotMatch(text, /"finish_reason":\s*"error"/); + assert.doesNotMatch(text, /event: error/); +}); + +test("#8649 the Claude-format contentless stream is caught too", async () => { + const frames = [ + 'event: message_start\ndata: {"type":"message_start","message":{"id":"m","content":[]}}\n\n', + 'event: message_stop\ndata: {"type":"message_stop"}\n\n', + ]; + const text = await runClientStream(frames, FORMATS.CLAUDE); + + assert.match(text, /event: error\r?\n/, "a contentless Claude stream must surface an error"); +});