diff --git a/src/server/relay.ts b/src/server/relay.ts index 0e29bd85c85..bdd5c26e39e 100644 --- a/src/server/relay.ts +++ b/src/server/relay.ts @@ -227,9 +227,19 @@ export function refusalFailedTailFrame( ); } +/** + * A bare upstream error event: the `type: "error"` SSE event, or an untyped envelope that + * carries only an `error` object (`{"error":{...}}`, #8216). Some OpenAI-compatible gateways + * end a stream with the untyped form; without this it was dropped and EOF reported + * `adapter_eof`. A typed event of any other kind is never bare, even with an `error` field. + */ +function isBareUpstreamErrorEvent(root: JsonRecord): boolean { + return root.type === "error" || (root.type === undefined && asJsonRecord(root.error) !== null); +} + function boundedBareUpstreamErrorMessage(payload: unknown): string | undefined { - if (!payload || typeof payload !== "object" || Array.isArray(payload) - || (payload as { type?: unknown }).type !== "error") return undefined; + const root = asJsonRecord(payload); + if (!root || !isBareUpstreamErrorEvent(root)) return undefined; const message = upstreamErrorMessageFromPayload(payload); return message ? redactSecretString(message).slice(0, MAX_TAIL_ERROR_MESSAGE_CHARS) : undefined; } @@ -256,7 +266,7 @@ function boundedBareUpstreamError(payload: unknown): { errorCode: string | undefined; } | undefined { const root = asJsonRecord(payload); - if (!root || root.type !== "error") return undefined; + if (!root || !isBareUpstreamErrorEvent(root)) return undefined; const message = boundedBareUpstreamErrorMessage(payload); const response = asJsonRecord(root.response); // Precedence is {@link upstreamErrorMessageFromPayload}'s, so the envelope @@ -272,7 +282,7 @@ function boundedBareUpstreamError(payload: unknown): { ]; const code = candidates.map(candidate => stringField(candidate, "code")) .find(candidate => candidate !== undefined)?.slice(0, 128); - // Root `type` is the SSE event discriminator (`"error"`), not an error class. Only nested + // Root `type` is the SSE event discriminator (`"error"` or absent), not an error class. Only nested // error records can authoritatively name classes such as rate_limit_error or server_error. const errorType = candidates.slice(0, -1).map(candidate => stringField(candidate, "type")) .find(candidate => candidate !== undefined)?.slice(0, 128); @@ -280,13 +290,15 @@ function boundedBareUpstreamError(payload: unknown): { ? (isTerminalRefusalCode(code) ? code : undefined) : message === undefined ? undefined : safetyRefusalCodeFromMessage(message); if (message !== undefined) return { message, refusalCode, errorType, errorCode: code }; - if (refusalCode === undefined) return undefined; - return { - message: terminalRefusalFallbackMessage(refusalCode), - refusalCode, - errorType, - errorCode: code, - }; + if (refusalCode !== undefined) { + return { message: terminalRefusalFallbackMessage(refusalCode), refusalCode, errorType, errorCode: code }; + } + // A structured code or class with no message is still the upstream's verdict, so it must end in + // `response.failed` rather than `adapter_eof`. Its own bounded identifiers are the only + // diagnostic it sent; the tail redacts and masks them like any other message. + const identity = code ?? errorType; + if (identity === undefined) return undefined; + return { message: `Upstream error without a message (${identity})`, refusalCode, errorType, errorCode: code }; } export type SseTerminalOutputBoundary = { @@ -670,8 +682,9 @@ function parseSsePayload(payload: string): unknown | undefined { } function isPolicyRewriteType(parsed: unknown): boolean { - const type = asJsonRecord(parsed)?.type; - return type === "response.failed" || type === "response.incomplete" || type === "error"; + const root = asJsonRecord(parsed); + if (!root) return false; + return root.type === "response.failed" || root.type === "response.incomplete" || isBareUpstreamErrorEvent(root); } /** @@ -802,7 +815,9 @@ function createFirstOutputReporter(onFirstOutput?: () => void): { } export function terminalStatusFromParsed(parsed: unknown): ResponsesTerminalStatus | null { - const type = asJsonRecord(parsed)?.type; + const root = asJsonRecord(parsed); + // An untyped bare error envelope (#8216) is classified like the `error` event it stands for. + const type = root && isBareUpstreamErrorEvent(root) ? "error" : root?.type; switch (type) { case "response.completed": return "completed"; @@ -1719,8 +1734,10 @@ export function consumeForInspection( logCtx, onCompletedResponse, onParsedPayload: payload => { - const message = boundedBareUpstreamErrorMessage(payload); - if (message !== undefined) bareUpstreamError = message; + // Same extraction as the client boundary, so a refusal or code sent without a message is + // still a failure here and cannot be recorded as an incomplete (non-failed) turn. + const bare = boundedBareUpstreamError(payload); + if (bare !== undefined) bareUpstreamError = bare.message; options?.onParsedPayload?.(payload); }, onFirstOutput, diff --git a/src/server/ws-bridge.ts b/src/server/ws-bridge.ts index a306ee4aadd..1df234bd3a2 100644 --- a/src/server/ws-bridge.ts +++ b/src/server/ws-bridge.ts @@ -210,6 +210,23 @@ function payloadType(payload: string): string | null { } } +/** + * An untyped `{"error":{...}}` envelope (#8216): not a Responses event, but not a malformed frame + * either. The SSE relay boundary keeps the error and reports it as `response.failed` at EOF unless a + * real terminal arrives first, so the pump skips the frame and lets that terminal reach the client. + * Mirrors the untyped half of `isBareUpstreamErrorEvent` in `relay.ts`. + */ +function isUntypedErrorEnvelope(payload: string): boolean { + try { + const json = JSON.parse(payload) as unknown; + if (!json || typeof json !== "object" || Array.isArray(json)) return false; + const root = json as { type?: unknown; error?: unknown }; + return root.type === undefined && !!root.error && typeof root.error === "object" && !Array.isArray(root.error); + } catch { + return false; + } +} + function terminalStatusFromType(type: string): ResponsesTerminalStatus | null { switch (type) { case "response.completed": @@ -290,6 +307,7 @@ export async function pumpResponsesSseToWebSocket( /* payload observation must not affect WebSocket delivery */ } const type = payloadType(payload); + if (!type && isUntypedErrorEnvelope(payload)) return false; if (!type) { reportTerminal("incomplete"); sendProtocolError(ws, 502, "Invalid JSON payload in upstream SSE frame"); diff --git a/structure/transports/responses-combo-boundaries.md b/structure/transports/responses-combo-boundaries.md index d76940d21d9..c0da42c5a4e 100644 --- a/structure/transports/responses-combo-boundaries.md +++ b/structure/transports/responses-combo-boundaries.md @@ -144,8 +144,10 @@ prelude. the ordinary bounded combo-failure classifier and may advance to the next declared target; - a zero-output `response.incomplete` with no reason, `adapter_eof`, `missing_terminal_event`, or `upstream_stall_timeout` follows the same failure path; explicit semantic or unknown reasons stay terminal; -- a top-level `error` arrives before output, in which case unknown, rate-limit, and server failures - may advance while errors explicitly classified as non-retryable 4xx remain committed; +- a top-level `error` (`type: "error"`) arrives before output, in which case unknown, rate-limit, and server failures + may advance while errors explicitly classified as non-retryable 4xx remain committed. An untyped + `{"error":{...}}` envelope is not a control event, so it commits the target; the relay then reports it + as `response.failed` rather than `adapter_eof`; - another completed/incomplete terminal or the aggregate preflight byte or retained-chunk cap is reached, in which case the current target is committed conservatively. diff --git a/structure/transports/responses-wire-shapes.md b/structure/transports/responses-wire-shapes.md index 5f11fd37224..2bd79832c85 100644 --- a/structure/transports/responses-wire-shapes.md +++ b/structure/transports/responses-wire-shapes.md @@ -397,14 +397,14 @@ Native passthrough SSE has TWO shapes, selected per request in The shared `createSseInspector` factory in `relay.ts` retains the complete inspection side effects, including the #44 late-terminal semantics. -Both client readers retain a bounded, redacted message and the first structured refusal code from a bare upstream `error`. EOF synthesis also retains narrowly recognized rate-limit codes (`rate_limit_exceeded`, `rate_limit_error`) overload codes (`server_is_overloaded`, `overloaded_error`), and the client-refusal classes `invalid_request_error`, `authentication_error`, `permission_error`, `not_found_error` (400/401/403/404, the code repeating the class); recognized types apply only without an explicit code. -At EOF without a real terminal they synthesize `response.failed` rather than `adapter_eof`; a code without a message still -produces a terminal. Codex retries codes outside its fatal set, so code and message follow the same candidate precedence; +Both client readers retain a bounded, redacted message and the first structured refusal code from a bare upstream `error`: a `type: "error"` event, or an envelope with no root `type` that carries an `error` object (#8216). A typed event of any other kind is never bare, even when it has an `error` field. The SSE-to-WebSocket pump in `src/server/ws-bridge.ts` skips the untyped envelope rather than failing it as an invalid frame, so the boundary's terminal still reaches a WebSocket client; any other frame without a string `type` stays a protocol error. EOF synthesis also retains narrowly recognized rate-limit codes (`rate_limit_exceeded`, `rate_limit_error`) overload codes (`server_is_overloaded`, `overloaded_error`), and the client-refusal classes `invalid_request_error`, `authentication_error`, `permission_error`, `not_found_error` (400/401/403/404, the code repeating the class); recognized types apply only without an explicit code. +At EOF without a real terminal they synthesize `response.failed` rather than `adapter_eof`; a code or error class without a +message still produces a terminal (a refusal code gets refusal copy, any other identifier is named in the message). Codex retries codes outside its fatal set, so code and message follow the same candidate precedence; recognized refusal copy is used only when the event has no code. A read failure after refusal reports that refusal (#5176). The shared outbound rewrite masks diagnostics on real failed and incomplete terminals before SSE or buffered JSON delivery, while preserving status and output; failed turns are not retained as continuation state. Buffered JSON masks selected credentials in synthetic bare-error fields before log inspection or client formatting; request logs keep transport status. The delivering reader owns refusal evidence before EOF; asynchronous tee inspection cannot reliably supply it. The client ingress in `src/server/index/websocket-handler.ts` preserves the same in-process replay-refusal marker as the HTTP deferred logger before finalization: a synthetic 429 retains `upstream_reset_replay_refused` and `transport-ambiguous` on the row and serving attempt, with `terminalSource: synthetic` on the row. `src/server/responses/core-combo-failure.ts` carries that marker through child failure rebuilds, and the combo loop stamps the failed child attempt before adopting its log. Synthetic refusals stop before combo cooldown/account rotation and supply no quota reset or cooldown metadata. Genuine upstream 429s keep rate-limit attribution and health handling. This is independent of the upstream transport choice; `headers-only` does not prove upstream execution or quota exhaustion. `tests/responses/ws-replay-refusal-attribution.test.ts` checks direct routes; `tests/responses/combo-replay-refusal-attribution.test.ts` checks both combo ingresses, gateway and pool children, health/cooldown state and genuine 429 controls, with one send and no recovery. -Inspection still applies the bare-error rule at EOF for account health. Real terminals and caller cancellation take precedence. Unknown codes retain `upstream_error` / `upstream_server_error` in the client tail; on the streaming path the log status takes the frame's Codex WebSocket wrapper 5xx when stated (buffered JSON callers keep their own presentation status). Refusal precedence is unchanged and classification adds no replay or retry. Bare-error log status is provisional until a genuine terminal supersedes it through `src/server/request-log-terminal-status.ts`. +Inspection still applies the same bare-error extraction at EOF for account health, so a refusal or code sent without a message is reported `failed` there too. Real terminals and caller cancellation take precedence. Unknown codes retain `upstream_error` / `upstream_server_error` in the client tail; on the streaming path the log status takes the frame's Codex WebSocket wrapper 5xx when stated (buffered JSON callers keep their own presentation status). Refusal precedence is unchanged and classification adds no replay or retry. Bare-error log status is provisional until a genuine terminal supersedes it through `src/server/request-log-terminal-status.ts`. Native recovery preflight keeps the rejected body reader and bounded prefix for normal mid-stream failure, without decrypt retry. Native Responses may rebuild once when encrypted function/custom-tool output or @@ -517,7 +517,7 @@ byte-identical payloads unchanged. The client-facing boundary treats the first Responses terminal as authoritative in both relay shapes. High-confidence policy errors carried as `response.incomplete`, `response.failed`, or a -top-level `error` are normalized to one `response.failed / cyber_policy` event without changing the +bare `error` (typed or untyped) are normalized to one `response.failed / cyber_policy` event without changing the refusal outcome; later bytes cannot create a second terminal. A clean HTTP 200 EOF with no terminal instead emits one `response.incomplete` with `adapter_eof`, followed by one `[DONE]`. Delimiter-less EOF candidates follow the owning repair policy: the native boundary accepts a structurally valid diff --git a/tests/responses/sse-failed-tail.test.ts b/tests/responses/sse-failed-tail.test.ts index bc3975cf661..b439b3905b5 100644 --- a/tests/responses/sse-failed-tail.test.ts +++ b/tests/responses/sse-failed-tail.test.ts @@ -9,6 +9,8 @@ import { readCodexWsStage } from "../../src/server/responses/codex-ws-wire"; import { TERMINAL_REFUSAL_FALLBACK_MESSAGE } from "../../src/lib/errors"; import { TranslatorBudgetExceededError } from "../../src/lib/translator-budget"; import { createOutboundCredentialMask } from "../../src/server/responses/terminal-error-redaction"; +import { pumpResponsesSseToWebSocket, type WsData } from "../../src/server/ws-bridge"; +import type { ServerWebSocket } from "bun"; const encoder = new TextEncoder(); const decoder = new TextDecoder(); @@ -447,6 +449,117 @@ describe("relaySseWithFailedTail", () => { expect(doneEvents(out)).toHaveLength(1); expect(out).not.toContain("event: response.failed"); }); + + /** Client relay output plus the terminal the tee inspection branch or eager hooks reported. */ + async function relayAndInspect(mode: "tee" | "eager", raw: string): Promise<{ out: string; reported: string[] }> { + const reported: string[] = []; + const src = sourceStream([raw]); + if (mode === "eager") { + const hooks = { ...parityHooks, onSynthetic: (kind: string) => { reported.push(kind); } }; + return { out: await drain(relaySseEagerBounded(src, new AbortController(), hooks)), reported }; + } + const [client, inspection] = src.tee(); + const log: RequestLogContext = { model: "fixture-model", provider: "openai" }; + const inspected = new Promise(resolve => consumeForInspection(inspection, status => { reported.push(status); }, undefined, resolve, log)); + const out = await drain(relaySseWithFailedTail(client, new AbortController())); + await inspected; + return { out, reported }; + } + + test.each(["tee", "eager"] as const)("%s untyped error envelope ends in response.failed, not adapter_eof (#8216)", async (mode) => { + // Captured gateway shape: heartbeat, an `error` object with no root `type`, DONE, EOF. + const raw = ": PING\n\n" + + 'data: {"error":{"message":"openai_error","type":"bad_response_status_code","param":"","code":"bad_response_status_code"}}\n\n' + + "data: [DONE]\n\n"; + const { out, reported } = await relayAndInspect(mode, raw); + + expect(failedMessage(out)).toBe("openai_error"); + expect(out).toContain('"code":"upstream_server_error"'); + expect(terminalEvents(out)).toEqual(["response.failed"]); + expect(doneEvents(out)).toHaveLength(1); + expect(out).not.toContain("adapter_eof"); + expect(reported).toEqual(["failed"]); + }); + + test.each(["tee", "eager"] as const)("%s typed event carrying an error field is not a bare error (#8216)", async (mode) => { + const raw = 'data: {"type":"response.in_progress","response":{"status":"in_progress"},' + + '"error":{"message":"not a failure","code":"server_error"}}\n\n'; + const { out, reported } = await relayAndInspect(mode, raw); + + expect(terminalEvents(out)).toEqual(["response.incomplete"]); + expect(out).toContain('"reason":"adapter_eof"'); + expect(out).not.toContain("event: response.failed"); + expect(doneEvents(out)).toHaveLength(1); + expect(reported).toEqual(["incomplete"]); + }); + + test.each(["tee", "eager"] as const)("%s untyped refusal code without a message reports failed on both readers (#8216)", async (mode) => { + // The client tail already carried the refusal; tee inspection used to read only the message and + // reported `incomplete`, which account health counts as a non-failure. + const raw = 'data: {"error":{"code":"invalid_prompt"}}\n\n'; + const { out, reported } = await relayAndInspect(mode, raw); + + expect(terminalEvents(out)).toEqual(["response.failed"]); + expect(out).toContain('"code":"invalid_prompt"'); + expect(failedMessage(out)).toBe(TERMINAL_REFUSAL_FALLBACK_MESSAGE); + expect(reported).toEqual(["failed"]); + }); + + test.each([ + ["tee", "untyped"], ["eager", "untyped"], ["tee", "typed"], ["eager", "typed"], + ] as const)("%s %s error with a code but no message still ends in response.failed", async (mode, shape) => { + const raw = shape === "untyped" + ? 'data: {"error":{"code":"bad_response_status_code"}}\n\n' + : 'data: {"type":"error","code":"bad_response_status_code"}\n\n'; + const { out, reported } = await relayAndInspect(mode, raw); + + expect(terminalEvents(out)).toEqual(["response.failed"]); + expect(out).not.toContain("adapter_eof"); + expect(out).toContain('"code":"upstream_server_error"'); + // The upstream's own code is the only diagnostic it sent; keep it in the message. + expect(failedMessage(out)).toContain("bad_response_status_code"); + expect(doneEvents(out)).toHaveLength(1); + expect(reported).toEqual(["failed"]); + }); + + test.each(["tee", "eager"] as const)("%s untyped cyber_policy error is rewritten as the policy terminal (#8216)", async (mode) => { + const raw = 'data: {"error":{"type":"invalid_request_error","code":"cyber_policy","message":"blocked by policy"}}\n\n' + + 'data: {"type":"response.output_text.delta","delta":"late"}\n\n'; + const { out, reported } = await relayAndInspect(mode, raw); + + expect(terminalEvents(out)).toEqual(["response.failed"]); + expect(out).not.toContain('data: {"error":'); + expect(out).not.toContain("late"); + expect(failedMessage(out)).toBe("blocked by policy"); + expect(out).toContain('"code":"cyber_policy"'); + expect(out).toContain('"retryable":false'); + expect(doneEvents(out)).toHaveLength(1); + // The rewritten frame is a real terminal: tee inspection reports it, and eager synthesizes none. + expect(reported).toEqual(mode === "tee" ? ["failed"] : []); + }); + + test.each(["tee", "eager"] as const)("%s untyped error envelope reaches a WebSocket client as response.failed (#8216)", async (mode) => { + const raw = 'data: {"type":"response.created","response":{"id":"r1","status":"in_progress"}}\n\n' + + 'data: {"error":{"message":"openai_error","code":"bad_response_status_code"}}\n\n' + + "data: [DONE]\n\n"; + const src = sourceStream([raw]); + const relayed = mode === "tee" + ? relaySseWithFailedTail(src, new AbortController()) + : relaySseEagerBounded(src, new AbortController(), parityHooks); + const sent: string[] = []; + const terminals: string[] = []; + const ws = { + readyState: 1, + data: {} as WsData, + send(message: string) { sent.push(message); return 1; }, + } as unknown as ServerWebSocket; + await pumpResponsesSseToWebSocket(ws, relayed, { onTerminal: status => { terminals.push(status); } }); + + const types = sent.map(frame => (JSON.parse(frame) as { type?: unknown }).type); + expect(types).toEqual(["response.created", "response.failed"]); + expect((JSON.parse(sent[1]!) as { response: { error: { message: string } } }).response.error.message).toBe("openai_error"); + expect(terminals).toEqual(["failed"]); + }); }); diff --git a/tests/routing/combo-stream-preflight.test.ts b/tests/routing/combo-stream-preflight.test.ts index 97d9f905441..3671d22f3d2 100644 --- a/tests/routing/combo-stream-preflight.test.ts +++ b/tests/routing/combo-stream-preflight.test.ts @@ -379,6 +379,18 @@ describe("combo stream preflight", () => { } }); + test("commits an untyped error envelope instead of advancing the combo (#8216)", async () => { + // Not a control event, so it is never resent to another target; the relay reports it as failed. + const source = sse( + { type: "response.created", response: { id: "r1", status: "in_progress" } }, + { error: { message: "openai_error", code: "bad_response_status_code" } }, + ); + const expected = await source.clone().text(); + const result = await preflightComboStreamResponse(source, { model: "m1", provider: "a" }); + expect(result.kind).toBe("accepted"); + expect(await result.response.text()).toBe(expected); + }); + test("passes a zero-output credential error to the ordinary combo classifier", async () => { const result = await preflightComboStreamResponse(sse({ type: "error",