From 93f3e268a8d29dee324cdb41dc3a48c2d3f33046 Mon Sep 17 00:00:00 2001 From: diegosouzapw Date: Wed, 10 Jun 2026 18:01:08 -0300 Subject: [PATCH] fix: pass through valid max_tokens-truncated responses instead of fake 502 (#3572) --- CHANGELOG.md | 4 ++ open-sse/services/errorClassifier.ts | 20 +++++- .../empty-content-stopreason-3572.test.ts | 71 +++++++++++++++++++ 3 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 tests/unit/empty-content-stopreason-3572.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 843be78ce9f7..ea2fd2baeea0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### 🔧 Bug Fixes + +- **fix(routing):** a valid `max_tokens`-truncated upstream response is no longer misclassified as empty content and rewritten into a fake 502 — `isEmptyContentResponse()` flagged any Claude `content:[]` / OpenAI empty-choice payload regardless of `stop_reason`/`finish_reason`, so a Claude Code `max_tokens: 1` connectivity ping (HTTP 200, `stop_reason:"max_tokens"`, empty content) became a synthetic `502 "Provider returned empty content"` and triggered a needless family fallback. The guard now treats a terminal truncation/tool signal (Claude `stop_reason` `max_tokens`/`tool_use`, OpenAI `finish_reason` `length`/`tool_calls`) as a legitimate completion; genuinely empty responses (no terminal reason, or `stop`/`end_turn` with empty content) are still caught. ([#3572](https://github.com/diegosouzapw/OmniRoute/issues/3572)) + --- ## [3.8.20] — 2026-06-10 diff --git a/open-sse/services/errorClassifier.ts b/open-sse/services/errorClassifier.ts index 45d7180f2ac5..acc302858c7e 100644 --- a/open-sse/services/errorClassifier.ts +++ b/open-sse/services/errorClassifier.ts @@ -6,6 +6,13 @@ import { } from "./accountFallback.ts"; import { getProviderCategory } from "../config/providerRegistry.ts"; +// Terminal stop signals where an empty content payload is still a legitimate, +// successful completion (truncated at the token limit, or a tool-call turn) — +// NOT a silent "fake success" failure. Used to avoid rewriting a valid HTTP 200 +// (e.g. a Claude Code `max_tokens: 1` connectivity ping) into a synthetic 502. +const LEGIT_EMPTY_CLAUDE_STOP = new Set(["max_tokens", "tool_use"]); +const LEGIT_EMPTY_OPENAI_FINISH = new Set(["length", "tool_calls"]); + export function isEmptyContentResponse(responseBody: unknown): boolean { if (!responseBody || typeof responseBody !== "object") return false; @@ -28,11 +35,22 @@ export function isEmptyContentResponse(responseBody: unknown): boolean { const hasReasoning = reasoningContent !== null && reasoningContent !== undefined && reasoningContent !== ""; + // A response truncated at the token limit (finish_reason "length") is a valid, + // successful completion even with empty text — do not flag it as a fake success. + const finishReason = + typeof firstChoice.finish_reason === "string" ? firstChoice.finish_reason : ""; + if (LEGIT_EMPTY_OPENAI_FINISH.has(finishReason)) return false; + return !hasContent && !hasReasoning && !hasToolCalls; } if (Array.isArray(body.content)) { - return body.content.length === 0; + if (body.content.length > 0) return false; + // Empty content array: a response truncated at max_tokens (or one that stopped + // to emit a tool_use block) is a legitimate terminal state, not a silent + // failure. Only flag empty content when no such terminal stop_reason is present. + const stopReason = typeof body.stop_reason === "string" ? body.stop_reason : ""; + return !LEGIT_EMPTY_CLAUDE_STOP.has(stopReason); } if (typeof body.text === "string") { diff --git a/tests/unit/empty-content-stopreason-3572.test.ts b/tests/unit/empty-content-stopreason-3572.test.ts new file mode 100644 index 000000000000..ea5fb4f8b1c0 --- /dev/null +++ b/tests/unit/empty-content-stopreason-3572.test.ts @@ -0,0 +1,71 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { isEmptyContentResponse } from "../../open-sse/services/errorClassifier.ts"; + +// #3572 — A valid max_tokens-truncated upstream response (HTTP 200, a legitimate +// terminal stop_reason/finish_reason, empty content) must NOT be misclassified as +// an empty/silent-failure response (which gets rewritten into a synthetic 502). +// The empty-content guard must only fire when content is empty AND there is no +// legitimate terminal stop_reason — i.e. a genuine fake-success / silent failure. + +test("#3572 Claude: empty content + stop_reason=max_tokens is NOT empty-failure", () => { + assert.equal( + isEmptyContentResponse({ + type: "message", + role: "assistant", + content: [], + stop_reason: "max_tokens", + usage: { output_tokens: 1 }, + }), + false + ); +}); + +test("#3572 Claude: empty content + stop_reason=tool_use is NOT empty-failure", () => { + assert.equal(isEmptyContentResponse({ content: [], stop_reason: "tool_use" }), false); +}); + +test("#3572 Claude: empty content with NO stop_reason IS still empty-failure", () => { + assert.equal(isEmptyContentResponse({ content: [] }), true); + assert.equal(isEmptyContentResponse({ content: [], stop_reason: null }), true); +}); + +test("#3572 Claude: empty content + stop_reason=end_turn stays flagged (fake-success guard preserved)", () => { + assert.equal(isEmptyContentResponse({ content: [], stop_reason: "end_turn" }), true); +}); + +test("#3572 OpenAI: empty content + finish_reason=length is NOT empty-failure", () => { + assert.equal( + isEmptyContentResponse({ + choices: [{ index: 0, message: { content: "" }, finish_reason: "length" }], + }), + false + ); +}); + +test("#3572 OpenAI: empty delta + finish_reason=length (stream chunk) is NOT empty-failure", () => { + assert.equal( + isEmptyContentResponse({ + choices: [{ index: 0, delta: { content: "" }, finish_reason: "length" }], + }), + false + ); +}); + +test("#3572 OpenAI: empty content + finish_reason=stop stays flagged (fake-success guard preserved)", () => { + assert.equal( + isEmptyContentResponse({ + choices: [{ index: 0, message: { content: "" }, finish_reason: "stop" }], + }), + true + ); +}); + +test("#3572 regression: non-empty content is never flagged", () => { + assert.equal(isEmptyContentResponse({ content: [{ type: "text", text: "hi" }] }), false); + assert.equal( + isEmptyContentResponse({ choices: [{ message: { content: "hi" }, finish_reason: "length" }] }), + false + ); +});