mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-31 04:12:10 +03:00
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.
This commit is contained in:
committed by
GitHub
parent
de0db5a777
commit
e077906401
@@ -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).
|
||||
@@ -344,7 +344,19 @@ function toHeaders(raw: Record<string, string[]>): 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<string, unknown>) => Promise<TlsResponseLike> },
|
||||
url: string,
|
||||
requestOptions: Record<string, unknown>,
|
||||
@@ -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,
|
||||
};
|
||||
}
|
||||
|
||||
102
tests/unit/probe-7134-claude-web-empty-error-body.test.ts
Normal file
102
tests/unit/probe-7134-claude-web-empty-error-body.test.ts
Normal file
@@ -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<string, unknown>) => {
|
||||
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<string, unknown>) => {
|
||||
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");
|
||||
});
|
||||
Reference in New Issue
Block a user