mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-31 04:12:10 +03:00
`validateResponseQuality()` only inspected a response's top-level `error` field when `choices` was also missing/empty (the narrower #3424 case), so a masked HTTP 200 that echoed a non-empty stub `choices` alongside a structured error object — or a known exhaustion phrase like "insufficient credits" / "quota exceeded" in the error envelope — slipped through as valid, and a `priority` combo kept hammering the exhausted target instead of failing over. The check now inspects the error envelope (top-level `error` object, or a bounded exhaustion-phrase match against error.message/code/type and top-level message/detail) unconditionally, before any shape-specific branch — never against `choices[].message.content`, so legitimate completions that merely mention "quota" in prose are not misclassified. Regression guard: tests/unit/masked-200-exhaustion-fallback-6427.test.ts
This commit is contained in:
committed by
GitHub
parent
908e3bef38
commit
7d67fc705c
@@ -10,6 +10,7 @@ _Living section — bullets land here as PRs merge into `release/v3.8.47` (paral
|
||||
|
||||
### 🐛 Bug Fixes
|
||||
|
||||
- **fix(resilience):** a `priority` combo never fell back when a target masked credit/quota exhaustion behind an HTTP 200 ([#6427](https://github.com/diegosouzapw/OmniRoute/issues/6427)) — `validateResponseQuality()` (`open-sse/services/combo/validateQuality.ts`) only inspected the response body's top-level `error` field when `choices` was ALSO missing/empty (the narrower #3424 empty-completion case); a masked 200 that echoed a non-empty stub `choices` alongside a structured error object, or a known exhaustion phrase (e.g. "insufficient credits", "quota exceeded") in the error envelope, slipped through as "valid" and the combo kept returning the dead target's response forever instead of failing over. The quality check now inspects the error envelope — a top-level OpenAI-shape `error` object, or a bounded, case-insensitive exhaustion-phrase match against `error.message`/`error.code`/`error.type`/top-level `message`/`detail` — unconditionally, before any shape-specific branch, and regardless of whether `choices`/`output` also look structurally present. The check never inspects `choices[].message.content`, so a legitimate completion that merely mentions "quota" or "credits" in assistant prose is not misclassified. Regression guard: `tests/unit/masked-200-exhaustion-fallback-6427.test.ts`.
|
||||
- **fix(compression):** adaptive-compression ladder ranked 6 real catalog engines (`ccr`, `ionizer`, `relevance`, `llmlingua`, `llm`, `read-lifecycle`) as if they didn't exist ([#6533](https://github.com/diegosouzapw/OmniRoute/issues/6533)) — `ladder.ts`'s `AGGRESSIVENESS` and `REDUCTION_FACTOR` maps only covered the 7 engines wired into `DEFAULT_LADDER` (session-dedup/rtk/headroom/lite/caveman/aggressive/ultra); every other engine registered in `open-sse/services/compression/engines/index.ts` — including `ccr`/`llmlingua`, which the ladder doc comment already says are intentionally addable via `ladderOverride` — fell through to `aggressivenessOf()`'s `?? 0` default (same rank as `"off"`) and `expectedReductionFactor()`'s generic `?? 0.9` fallback, so `floor`-mode escalation could not rank or escalate past them once added to a custom ladder. Both maps now carry entries for all 6 missing real engines, rescaled ×10 (`off:0` … `ultra:70`) and placed by each engine's documented `stackPriority` (`ionizer` between `rtk`/`headroom`, `relevance` before `caveman`, `llmlingua`/`llm` between `aggressive` and `ultra`, etc.); `mcpAccessibility` — named in the report — is not a registered `CompressionEngine` (it's a separate MCP tool-response truncation mechanism) and was correctly left out. Regression guard: `tests/unit/ladder-engine-maps-6533.test.ts` (asserts every id from `listCompressionEngines()` ranks above `"off"` with a non-default reduction factor). (thanks @chirag127)
|
||||
- **fix(api):** tool-call arguments could render as `[object Object]` sequences instead of the real JSON through the `/anthropic` (Anthropic-shape `/messages`) routing path ([#6459](https://github.com/diegosouzapw/OmniRoute/issues/6459)) — `appendToolCallArgumentDelta()` (`open-sse/utils/toolCallArguments.ts`), the shared accumulator the streaming `openai-to-claude` response translator, `openai-responses` translator, and `responsesTransformer` all call to build up a tool call's `arguments`/`input_json_delta` buffer, treated any non-string `incoming` fragment as an empty string. Some upstreams deliver the full `tool_calls[].function.arguments` value as an already-parsed JSON object/array instead of the OpenAI-contracted JSON-encoded string; the old code silently discarded that fragment, leaving `tool_use.input` empty, and left downstream buffers open to a plain string coercion of the object (`[object Object]`) once client-side concatenation kicked in. `appendToolCallArgumentDelta()` now `JSON.stringify()`s a non-string, non-null object/array fragment into a valid JSON fragment instead of dropping it, so the assembled `partial_json` always parses back into the original structured value. Regression guard: `tests/unit/anthropic-toolcall-args-6459.test.ts`. (thanks @chirag127)
|
||||
- **fix(providers):** `fusion` combo returned the opaque `"All fusion panel models failed"` 503 even when only a minority of panel members were actually cooling down / rate-limited, and a user-supplied `fusionTuning.minPanel=1` was silently overridden ([#6454](https://github.com/diegosouzapw/OmniRoute/issues/6454)) — `handleFusionChat()` hard-clamped the quorum floor via `Math.min(Math.max(2, cfg.minPanel), panel.length)`, so an operator-configured `minPanel=1` never took effect: `collectPanel()`'s straggler-grace timer only starts once `ok >= minPanel`, and with the floor forced to 2 a single fast success plus N slow-failing stragglers never reached quorum, so the panel sat waiting instead of degrading to the survivor. Per-member failure reasons (`straggler_dropped`/`timeout`/`threw`/`status_XXX`/`empty_content`/`unparseable`) were also logged server-side but never surfaced in the 503 body, leaving operators unable to tell a rate-limit fan-fail from a broader outage. Fixed by honoring `Math.max(1, cfg.minPanel)` and threading a `failures: Array<{ model, reason }>` collector into the 503 message (`model=reason` per entry) — production fix already merged via #6521; this entry backfills the missing CHANGELOG bullet and adds an 11-member, `fusion-free`-scale regression test matching the original repro shape (a cooling minority must not sink a healthy majority; a genuinely all-failed panel still returns the documented 503). Regression guard: `tests/unit/services/fusion-min-panel-and-failure-detail.test.ts` + `tests/unit/fusion-partial-panel-failure-6454.test.ts`. (thanks @chirag127)
|
||||
|
||||
@@ -22,6 +22,38 @@ export function toRetryAfterDisplayValue(value: ComboRetryAfter): string | Date
|
||||
return new Date(value);
|
||||
}
|
||||
|
||||
// Issue #6427: some providers mask credit/quota exhaustion behind an HTTP 200 —
|
||||
// either an OpenAI-shape top-level `error` object, or a known exhaustion phrase
|
||||
// living in the error envelope itself (never in assistant prose — see
|
||||
// `extractEnvelopeErrorText`). Single-quantifier-per-token-class alternation,
|
||||
// no nested/overlapping quantifiers — cannot backtrack catastrophically.
|
||||
const EXHAUSTION_MARKER_PATTERN =
|
||||
/\b(insufficient\s+credit|insufficient\s+balance|quota\s+exceeded|out\s+of\s+credits?|credit\s+exhausted)\b/i;
|
||||
|
||||
/**
|
||||
* Collect the small set of top-level "error envelope" strings a 200 response may
|
||||
* carry alongside (or instead of) a normal completion: the OpenAI-shape `error`
|
||||
* object's `message`/`code`/`type`, a bare string `error`, or sibling top-level
|
||||
* `message`/`detail` fields some providers use for the same purpose. Deliberately
|
||||
* does NOT look inside `choices[].message.content` — assistant prose that merely
|
||||
* mentions "quota" or "credits" must never be misclassified as an upstream failure.
|
||||
*/
|
||||
function extractEnvelopeErrorText(json: Record<string, unknown>): string | null {
|
||||
const parts: string[] = [];
|
||||
const err = json.error;
|
||||
if (err && typeof err === "object") {
|
||||
const e = err as Record<string, unknown>;
|
||||
if (typeof e.message === "string") parts.push(e.message);
|
||||
if (typeof e.code === "string") parts.push(e.code);
|
||||
if (typeof e.type === "string") parts.push(e.type);
|
||||
} else if (typeof err === "string" && err.length > 0) {
|
||||
parts.push(err);
|
||||
}
|
||||
if (typeof json.message === "string") parts.push(json.message);
|
||||
if (typeof json.detail === "string") parts.push(json.detail);
|
||||
return parts.length > 0 ? parts.join(" ") : null;
|
||||
}
|
||||
|
||||
function responsesApiOutputHasContent(output: unknown): boolean {
|
||||
return (
|
||||
Array.isArray(output) &&
|
||||
@@ -335,6 +367,32 @@ export async function validateResponseQuality(
|
||||
}
|
||||
}
|
||||
|
||||
// Issue #6427: a masked 200 — an OpenAI-shape top-level `error` object, or a
|
||||
// known exhaustion phrase in the error envelope — is a failure regardless of
|
||||
// whether `choices`/`output` also look structurally present (some providers
|
||||
// echo a stub completion alongside the error). Checked unconditionally, before
|
||||
// any shape-specific branch, so it can't be shadowed by an otherwise-valid body.
|
||||
const rawError = json?.error;
|
||||
const errorIsMeaningful =
|
||||
(typeof rawError === "string" && rawError.length > 0) ||
|
||||
(!!rawError && typeof rawError === "object" && Object.keys(rawError).length > 0);
|
||||
if (errorIsMeaningful) {
|
||||
const envelopeText = extractEnvelopeErrorText(json);
|
||||
const errMsg =
|
||||
rawError && typeof rawError === "object" && typeof (rawError as Record<string, unknown>).message === "string"
|
||||
? ((rawError as Record<string, unknown>).message as string)
|
||||
: envelopeText || JSON.stringify(rawError).substring(0, 200);
|
||||
return { valid: false, reason: `upstream error in 200 body: ${errMsg}` };
|
||||
}
|
||||
{
|
||||
const envelopeText = extractEnvelopeErrorText(json);
|
||||
if (envelopeText && EXHAUSTION_MARKER_PATTERN.test(envelopeText)) {
|
||||
const snippet =
|
||||
envelopeText.length > 80 ? `${envelopeText.slice(0, 80)}…` : envelopeText;
|
||||
return { valid: false, reason: `upstream exhaustion marker in 200 body: ${snippet}` };
|
||||
}
|
||||
}
|
||||
|
||||
const choices = json?.choices;
|
||||
if (json?.object === "response") {
|
||||
if (!responsesApiOutputHasContent(json.output))
|
||||
@@ -354,14 +412,9 @@ export async function validateResponseQuality(
|
||||
}
|
||||
|
||||
if (!Array.isArray(choices) || choices.length === 0) {
|
||||
// `json?.error` is already handled unconditionally above (#6427); reaching
|
||||
// here means no error envelope was present.
|
||||
if (json?.output || json?.result || json?.data || json?.response) return { valid: true };
|
||||
if (json?.error) {
|
||||
const err = json.error as Record<string, unknown>;
|
||||
return {
|
||||
valid: false,
|
||||
reason: `upstream error in 200 body: ${err?.message || JSON.stringify(json.error).substring(0, 200)}`,
|
||||
};
|
||||
}
|
||||
return { valid: true };
|
||||
}
|
||||
|
||||
|
||||
211
tests/unit/masked-200-exhaustion-fallback-6427.test.ts
Normal file
211
tests/unit/masked-200-exhaustion-fallback-6427.test.ts
Normal file
@@ -0,0 +1,211 @@
|
||||
/**
|
||||
* Issue #6427 — a `priority` combo must fail over to the next target when the
|
||||
* first target returns HTTP 200 OK whose body masks credit/quota exhaustion:
|
||||
* either a top-level OpenAI-shape `error` object, or a known exhaustion phrase
|
||||
* living in the error envelope (`error.message`/top-level `message`/`detail`).
|
||||
*
|
||||
* Before this fix, `validateResponseQuality` (open-sse/services/combo/validateQuality.ts)
|
||||
* only inspected `json.error` when `choices` was ALSO missing/empty — a masked 200
|
||||
* that echoes a non-empty `choices` stub alongside the error slipped through as
|
||||
* "valid" and the combo kept returning the dead target's response instead of
|
||||
* failing over (#3424 already covers the narrower empty-`choices` case).
|
||||
*
|
||||
* Control case: a normal, valid completion whose assistant text merely MENTIONS
|
||||
* "quota" in prose must NOT be misclassified as an upstream failure — the check
|
||||
* only looks at the error envelope, never at `choices[].message.content`.
|
||||
*/
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
|
||||
const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-masked200-6427-"));
|
||||
const ORIGINAL_DATA_DIR = process.env.DATA_DIR;
|
||||
process.env.DATA_DIR = TEST_DATA_DIR;
|
||||
|
||||
const { handleComboChat } = await import("../../open-sse/services/combo.ts");
|
||||
const core = await import("../../src/lib/db/core.ts");
|
||||
const settingsDb = await import("../../src/lib/db/settings.ts");
|
||||
const { resetAllComboMetrics } = await import("../../open-sse/services/comboMetrics.ts");
|
||||
const { resetAllCircuitBreakers } = await import("../../src/shared/utils/circuitBreaker.ts");
|
||||
const { resetAll: resetAllSemaphores } = await import("../../open-sse/services/rateLimitSemaphore.ts");
|
||||
const { _resetAllDecks } = await import("../../src/shared/utils/shuffleDeck.ts");
|
||||
const { clearSessions } = await import("../../open-sse/services/sessionManager.ts");
|
||||
|
||||
function createLog() {
|
||||
const entries: unknown[] = [];
|
||||
return {
|
||||
info: (tag: unknown, msg: unknown) => entries.push({ level: "info", tag, msg }),
|
||||
warn: (tag: unknown, msg: unknown) => entries.push({ level: "warn", tag, msg }),
|
||||
error: (tag: unknown, msg: unknown) => entries.push({ level: "error", tag, msg }),
|
||||
debug: (tag: unknown, msg: unknown) => entries.push({ level: "debug", tag, msg }),
|
||||
entries,
|
||||
};
|
||||
}
|
||||
|
||||
function jsonResponse(body: unknown) {
|
||||
return new Response(JSON.stringify(body), {
|
||||
status: 200,
|
||||
headers: { "content-type": "application/json" },
|
||||
});
|
||||
}
|
||||
|
||||
async function cleanupTestDataDir() {
|
||||
let lastError: unknown;
|
||||
for (let attempt = 0; attempt < 5; attempt += 1) {
|
||||
try {
|
||||
core.resetDbInstance();
|
||||
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
|
||||
return;
|
||||
} catch (error: unknown) {
|
||||
lastError = error;
|
||||
await new Promise((resolve) => setTimeout(resolve, 25));
|
||||
}
|
||||
}
|
||||
if (lastError) throw lastError;
|
||||
}
|
||||
|
||||
test.beforeEach(async () => {
|
||||
resetAllComboMetrics();
|
||||
resetAllCircuitBreakers();
|
||||
resetAllSemaphores();
|
||||
_resetAllDecks();
|
||||
clearSessions();
|
||||
await cleanupTestDataDir();
|
||||
fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
|
||||
await settingsDb.resetAllPricing();
|
||||
settingsDb.clearAllLKGP();
|
||||
});
|
||||
|
||||
test.after(async () => {
|
||||
resetAllComboMetrics();
|
||||
resetAllCircuitBreakers();
|
||||
resetAllSemaphores();
|
||||
_resetAllDecks();
|
||||
settingsDb.clearAllLKGP();
|
||||
if (ORIGINAL_DATA_DIR === undefined) {
|
||||
delete process.env.DATA_DIR;
|
||||
} else {
|
||||
process.env.DATA_DIR = ORIGINAL_DATA_DIR;
|
||||
}
|
||||
await cleanupTestDataDir();
|
||||
core.resetDbInstance();
|
||||
});
|
||||
|
||||
test("#6427 priority combo falls back when the first target's 200 body carries a structured `error` object", async () => {
|
||||
const calls: string[] = [];
|
||||
const result = await handleComboChat({
|
||||
body: {},
|
||||
combo: {
|
||||
name: "priority-masked-200-error",
|
||||
strategy: "priority",
|
||||
models: ["deadprovider/exhausted-model", "healthyprovider/backup-model"],
|
||||
config: { maxRetries: 0, retryDelayMs: 0, fallbackDelayMs: 0 },
|
||||
},
|
||||
handleSingleModel: async (_body: unknown, modelStr: string) => {
|
||||
calls.push(modelStr);
|
||||
if (modelStr === "deadprovider/exhausted-model") {
|
||||
// Masked 200: HTTP OK, with a NON-EMPTY stub `choices[0].message.content`
|
||||
// (so the pre-existing #3424 empty-content check alone would call this
|
||||
// "valid") AND a structured OpenAI-shape error object reporting
|
||||
// exhaustion. Only the #6427 envelope check catches this.
|
||||
return jsonResponse({
|
||||
choices: [{ message: { role: "assistant", content: "Request could not be completed." } }],
|
||||
error: { message: "Insufficient credits balance", type: "insufficient_quota" },
|
||||
});
|
||||
}
|
||||
return jsonResponse({ choices: [{ message: { role: "assistant", content: "real answer" } }] });
|
||||
},
|
||||
isModelAvailable: async () => true,
|
||||
log: createLog(),
|
||||
settings: null,
|
||||
allCombos: null,
|
||||
});
|
||||
|
||||
assert.equal(result.ok, true, "combo must ultimately succeed via the fallback target");
|
||||
assert.deepEqual(
|
||||
calls,
|
||||
["deadprovider/exhausted-model", "healthyprovider/backup-model"],
|
||||
"combo must fail over past the masked-200 target instead of returning it"
|
||||
);
|
||||
const bodyText = await result.clone().text();
|
||||
assert.match(bodyText, /real answer/, "the returned body must be the fallback target's real answer");
|
||||
});
|
||||
|
||||
test("#6427 priority combo falls back when the first target's 200 body carries a known exhaustion phrase (no structured error)", async () => {
|
||||
const calls: string[] = [];
|
||||
const result = await handleComboChat({
|
||||
body: {},
|
||||
combo: {
|
||||
name: "priority-masked-200-phrase",
|
||||
strategy: "priority",
|
||||
models: ["deadprovider/exhausted-model", "healthyprovider/backup-model"],
|
||||
config: { maxRetries: 0, retryDelayMs: 0, fallbackDelayMs: 0 },
|
||||
},
|
||||
handleSingleModel: async (_body: unknown, modelStr: string) => {
|
||||
calls.push(modelStr);
|
||||
if (modelStr === "deadprovider/exhausted-model") {
|
||||
// Masked 200: NON-EMPTY stub choice content, no `error` object, but a
|
||||
// top-level `message` field (a shape some providers use instead of the
|
||||
// OpenAI `error` envelope) carries a recognizable exhaustion phrase.
|
||||
return jsonResponse({
|
||||
choices: [{ message: { role: "assistant", content: "Request could not be completed." } }],
|
||||
message: "Quota exceeded for this account",
|
||||
});
|
||||
}
|
||||
return jsonResponse({ choices: [{ message: { role: "assistant", content: "real answer" } }] });
|
||||
},
|
||||
isModelAvailable: async () => true,
|
||||
log: createLog(),
|
||||
settings: null,
|
||||
allCombos: null,
|
||||
});
|
||||
|
||||
assert.equal(result.ok, true, "combo must ultimately succeed via the fallback target");
|
||||
assert.deepEqual(
|
||||
calls,
|
||||
["deadprovider/exhausted-model", "healthyprovider/backup-model"],
|
||||
"combo must fail over past the masked-200 target instead of returning it"
|
||||
);
|
||||
});
|
||||
|
||||
test("#6427 control: a normal 200 completion that merely mentions 'quota' in assistant prose is returned, not misclassified", async () => {
|
||||
const calls: string[] = [];
|
||||
const result = await handleComboChat({
|
||||
body: {},
|
||||
combo: {
|
||||
name: "priority-quota-in-prose-control",
|
||||
strategy: "priority",
|
||||
models: ["healthyprovider/primary-model", "healthyprovider/backup-model"],
|
||||
config: { maxRetries: 0, retryDelayMs: 0, fallbackDelayMs: 0 },
|
||||
},
|
||||
handleSingleModel: async (_body: unknown, modelStr: string) => {
|
||||
calls.push(modelStr);
|
||||
return jsonResponse({
|
||||
choices: [
|
||||
{
|
||||
message: {
|
||||
role: "assistant",
|
||||
content:
|
||||
"Sure — here is an explanation of API quota exceeded errors and insufficient credits handling in general.",
|
||||
},
|
||||
},
|
||||
],
|
||||
});
|
||||
},
|
||||
isModelAvailable: async () => true,
|
||||
log: createLog(),
|
||||
settings: null,
|
||||
allCombos: null,
|
||||
});
|
||||
|
||||
assert.equal(result.ok, true, "combo must succeed on the first target");
|
||||
assert.deepEqual(
|
||||
calls,
|
||||
["healthyprovider/primary-model"],
|
||||
"a legitimate completion mentioning 'quota'/'credits' in prose must NOT trigger a false-positive fallback"
|
||||
);
|
||||
const bodyText = await result.clone().text();
|
||||
assert.match(bodyText, /quota exceeded/i, "the real assistant prose must be returned unchanged");
|
||||
});
|
||||
Reference in New Issue
Block a user