From e077906401dea27dad036b00a1d02c3c53e965bd Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Tue, 14 Jul 2026 21:14:01 -0300 Subject: [PATCH] fix: surface real claude-web error body for non-SSE 400s (#7134) (#7196) tlsFetchStreaming() streams the upstream response to a temp file via tls-client-node's streamOutputPath mode. For a non-SSE, non-2xx response the native binding resolves with an empty in-memory `body` field even though the real error bytes were already written to (and peeked from) the temp file, so genuine Claude 400/403/429/500 error details were silently discarded and replaced with "no response body". Fall back to a bounded read of the temp file when the resolved response's body is empty, and export tlsFetchStreaming for dependency-injected testing without --experimental-test-module-mocks. --- .../7134-claude-web-400-no-response-body.md | 1 + open-sse/services/claudeTlsClient.ts | 27 ++++- ...e-7134-claude-web-empty-error-body.test.ts | 102 ++++++++++++++++++ 3 files changed, 128 insertions(+), 2 deletions(-) create mode 100644 changelog.d/fixes/7134-claude-web-400-no-response-body.md create mode 100644 tests/unit/probe-7134-claude-web-empty-error-body.test.ts diff --git a/changelog.d/fixes/7134-claude-web-400-no-response-body.md b/changelog.d/fixes/7134-claude-web-400-no-response-body.md new file mode 100644 index 0000000000..f9fd94cf5f --- /dev/null +++ b/changelog.d/fixes/7134-claude-web-400-no-response-body.md @@ -0,0 +1 @@ +- **fix(sse):** claude-web now surfaces the real upstream error body for non-SSE 400/403/429/500 responses instead of reporting "no response body" — the streaming client was discarding the already-captured temp-file bytes and reading the native binding's empty in-memory body field instead (#7134). diff --git a/open-sse/services/claudeTlsClient.ts b/open-sse/services/claudeTlsClient.ts index 499c32e130..0e785af9f5 100644 --- a/open-sse/services/claudeTlsClient.ts +++ b/open-sse/services/claudeTlsClient.ts @@ -344,7 +344,19 @@ function toHeaders(raw: Record): Headers { // to a file path, terminating when the upstream sends `streamOutputEOFSymbol`. // We tail the file from a worker and surface the bytes as a ReadableStream. -async function tlsFetchStreaming( +// Cap for the bounded fallback read of a non-SSE error body straight from the +// streaming temp file (mirrors the 2048-byte cap executors/claude-web.ts +// already applies when reading error bodies) — avoids buffering an unbounded +// error page into memory. See #7134. +const MAX_ERROR_BODY_BYTES = 16 * 1024; + +/** + * Exported for tests (issue #7134): allows injecting a fake `client` so the + * non-SSE error-body fallback path can be exercised without + * `--experimental-test-module-mocks`, matching the DI pattern already used + * by `__setTlsFetchOverrideForTesting` for the outer `tlsFetchClaude`. + */ +export async function tlsFetchStreaming( client: { request: (url: string, opts: Record) => Promise }, url: string, requestOptions: Record, @@ -417,11 +429,22 @@ async function tlsFetchStreaming( const r = await requestPromise.catch( (e) => ({ status: 502, headers: {}, body: String(e) }) as TlsResponseLike ); + // tls-client-node's `streamOutputPath` mode writes the response body to + // the temp file chunk-by-chunk and does NOT also populate the resolved + // response's in-memory `body` field (confirmed against + // node_modules/tls-client-node/dist/response.js) — so for every non-SSE, + // non-2xx claude-web response (400/403/429/500 with a real JSON/HTML + // error), `r.body` is empty even though the real bytes are sitting in + // `path` (we just peeked them above). Prefer `r.body` when it IS + // populated (some native-client modes do fill it in); otherwise fall + // back to a bounded read of the temp file so the real upstream error + // detail reaches the caller instead of being silently discarded. #7134 + const text = r.body || (await readFirstBytes(path, MAX_ERROR_BODY_BYTES).catch(() => "")); await cleanupTempPath(path); return { status: r.status, headers: toHeaders(r.headers), - text: r.body, + text, body: null, }; } diff --git a/tests/unit/probe-7134-claude-web-empty-error-body.test.ts b/tests/unit/probe-7134-claude-web-empty-error-body.test.ts new file mode 100644 index 0000000000..b62659616a --- /dev/null +++ b/tests/unit/probe-7134-claude-web-empty-error-body.test.ts @@ -0,0 +1,102 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { writeFile } from "node:fs/promises"; + +// Issue #7134 — claude-web reported "Claude Web API error (400) with no +// response body" even when Claude's upstream DID send a real JSON error body. +// +// Root cause: tlsFetchStreaming() streams the upstream response to a temp +// file via tls-client-node's `streamOutputPath` mode. For a non-SSE, +// non-2xx response, the native binding resolves with an EMPTY in-memory +// `body` field (it only populates `body` for its non-streaming mode) even +// though the real error bytes were already written to the temp file and +// even peeked (`looksLikeSse`) to decide the response wasn't SSE. The old +// code read the empty `r.body` instead of the file it just peeked, throwing +// away the real upstream error detail. +// +// This test injects a fake `client` (matching the `{ request }` shape +// tlsFetchStreaming already accepts for DI) that reproduces the exact +// tls-client-node contract under `streamOutputPath`: write bytes to the file, +// resolve with an empty `body`. No `--experimental-test-module-mocks` flag +// needed — this exercises the real, unmodified `tlsFetchStreaming` via +// dependency injection instead of module-mocking `tls-client-node`. + +const { tlsFetchStreaming } = await import("../../open-sse/services/claudeTlsClient.ts"); + +const REAL_CLAUDE_ERROR_BODY = JSON.stringify({ + type: "error", + error: { + type: "invalid_request_error", + message: "This conversation UUID does not exist or you do not have access to it.", + }, +}); + +function makeFakeClient(status: number, bodyOnFile: string) { + return { + request: async (_url: string, opts: Record) => { + const streamOutputPath = opts.streamOutputPath as string; + await writeFile(streamOutputPath, bodyOnFile); + return { + status, + headers: {}, + // tls-client-node does not populate `body` for streamed requests — + // this is the exact defect condition. + body: "", + cookies: {}, + text: async () => "", + json: async () => ({}), + bytes: async () => new Uint8Array(), + }; + }, + }; +} + +test("issue #7134: tlsFetchStreaming surfaces the real error body for a non-SSE 400 under stream:true", async () => { + const client = makeFakeClient(400, REAL_CLAUDE_ERROR_BODY); + + const result = await tlsFetchStreaming( + client, + "https://claude.ai/api/organizations/x/chat_conversations/y/completion", + { method: "POST" }, + "[DONE]", + null, + 5_000 + ); + + assert.equal(result.status, 400); + assert.equal(result.body, null); + assert.ok( + result.text && result.text.includes("does not exist or you do not have access to it"), + `expected the real Claude error body to be surfaced, got: ${JSON.stringify(result.text)}` + ); +}); + +test("issue #7134: tlsFetchStreaming still uses r.body when the native client DOES populate it", async () => { + const client = { + request: async (_url: string, opts: Record) => { + const streamOutputPath = opts.streamOutputPath as string; + await writeFile(streamOutputPath, "{}"); + return { + status: 403, + headers: {}, + body: "populated body from native client", + cookies: {}, + text: async () => "", + json: async () => ({}), + bytes: async () => new Uint8Array(), + }; + }, + }; + + const result = await tlsFetchStreaming( + client, + "https://claude.ai/api/organizations/x/chat_conversations/y/completion", + { method: "POST" }, + "[DONE]", + null, + 5_000 + ); + + assert.equal(result.status, 403); + assert.equal(result.text, "populated body from native client"); +});