mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-21 22:52:19 +03:00
fix(sse): combo diagnostics provider/connection truncation and quota recovery hint (#10967, #10966) (#11012)
⭐5 — Fix(#10967,#10966): combo diag exhausted_connection truncava o UUID por hardcode de provider="unknown"; recovery hint de quota caía em "retry" genérico. TDD RED→GREEN, 149 testes-irmãos verdes. UNSTABLE é o base-red inherited #9985.
This commit is contained in:
committed by
GitHub
parent
effc542a6a
commit
a492d6d780
2
changelog.d/fixes/10967-10966-combo-diag-recovery.md
Normal file
2
changelog.d/fixes/10967-10966-combo-diag-recovery.md
Normal file
@@ -0,0 +1,2 @@
|
||||
- fix(sse): combo diagnostics no longer truncate `exhausted_connection` entries to a hardcoded `provider: "unknown"` with the provider prefix eaten by an 8-char slice — the real provider id is preserved and only the connection id is truncated (#10967)
|
||||
- fix(sse): combo terminal failures caused entirely by quota/account-balance exhaustion (including a durable HTTP 403 `insufficient_quota` / `AUTHZ_INSUFFICIENT_BALANCE`) now stamp a stable `quota_exhausted` diagnostics reason with a `switch-combo` recovery hint instead of the misleading default `retry` action (#10966)
|
||||
@@ -34,6 +34,7 @@ import {
|
||||
recordComboFailure,
|
||||
} from "./combo/failureTracker.ts";
|
||||
import { buildNoUpstreamResponseDiagnostics, buildRecoveryHint } from "./combo/pinRecovery.ts";
|
||||
import { formatExhaustedConnectionKey } from "./combo/comboDiagFormat.ts";
|
||||
import { buildTargetTimeoutRunner } from "./combo/targetTimeoutRunner.ts";
|
||||
import { recordComboRequest, recordComboShadowRequest, getComboMetrics } from "./comboMetrics.ts";
|
||||
import { qualityScoreFor } from "./routing/index.ts";
|
||||
@@ -1081,10 +1082,7 @@ async function handleComboChatInner({
|
||||
attempted: recordedAttempts,
|
||||
excluded: [
|
||||
...[...exhaustedProviders].map((p) => ({ provider: p, reason: "exhausted" })),
|
||||
...[...exhaustedConnections].map((c) => ({
|
||||
provider: "unknown",
|
||||
reason: `exhausted_connection:${String(c).slice(0, 8)}`,
|
||||
})),
|
||||
...[...exhaustedConnections].map((c) => formatExhaustedConnectionKey(String(c))),
|
||||
],
|
||||
attemptOrder: comboAttemptOrder,
|
||||
terminalReason,
|
||||
@@ -2727,11 +2725,22 @@ async function handleComboChatInner({
|
||||
);
|
||||
}
|
||||
const retryAfterSeconds = undefined;
|
||||
// #10966: when every observed failure was independently classified as quota/
|
||||
// balance exhaustion (isQuotaExhaustionResponse, tracked via observeFailure's
|
||||
// allObservedFailuresQuota accumulator), stamp a stable `quota_exhausted`
|
||||
// terminalReason instead of forwarding the raw upstream error string. The raw
|
||||
// string falls through buildRecoveryHint's default branch ("retry" / "failed
|
||||
// transiently"), which is actively misleading for a durable wallet/quota
|
||||
// exhaustion — retrying the same combo will never refill it.
|
||||
const terminalReason =
|
||||
observedFailure && allObservedFailuresQuota
|
||||
? "quota_exhausted"
|
||||
: (lastError ?? "all_models_failed");
|
||||
return withQuotaExhaustionClassification(
|
||||
errorResponseWithComboDiagnostics(
|
||||
status,
|
||||
msg,
|
||||
buildComboDiag(lastError ?? "all_models_failed", retryAfterSeconds)
|
||||
buildComboDiag(terminalReason, retryAfterSeconds)
|
||||
),
|
||||
observedFailure ? allObservedFailuresQuota : null
|
||||
);
|
||||
|
||||
29
open-sse/services/combo/comboDiagFormat.ts
Normal file
29
open-sse/services/combo/comboDiagFormat.ts
Normal file
@@ -0,0 +1,29 @@
|
||||
/**
|
||||
* #10967: format an `exhaustedConnections` key stored by targetExhaustion.ts
|
||||
* (`markAuthLevelExhaustion` / `markAgentrouterConnectionQuotaExhaustion` /
|
||||
* `markConnectionLevelExhaustion`, all keyed as `` `${provider}:${connectionId}` ``)
|
||||
* into a diagnostics `excluded` entry.
|
||||
*
|
||||
* Before this fix, `buildComboDiag` (combo.ts) hardcoded `provider: "unknown"` and
|
||||
* `slice(0, 8)`'d the WHOLE key — for a typical `jina-ai:<uuid>` key that produced
|
||||
* `exhausted_connection:jina-ai:` (the 7-char provider id + colon consumed the
|
||||
* entire 8-char budget, the UUID silently dropped, and the real provider id
|
||||
* discarded in favor of the literal string "unknown").
|
||||
*
|
||||
* Splitting on the FIRST `:` recovers the real provider id and truncates only the
|
||||
* connection-id half (never the full UUID, matching the public combo projection's
|
||||
* connection-id redaction policy — #2300).
|
||||
*/
|
||||
export function formatExhaustedConnectionKey(key: string): {
|
||||
provider: string;
|
||||
reason: string;
|
||||
} {
|
||||
const raw = String(key);
|
||||
const sepIdx = raw.indexOf(":");
|
||||
const provider = sepIdx >= 0 ? raw.slice(0, sepIdx) : "";
|
||||
const connId = sepIdx >= 0 ? raw.slice(sepIdx + 1) : raw;
|
||||
return {
|
||||
provider: provider || "unknown",
|
||||
reason: `exhausted_connection:${connId.slice(0, 8)}`,
|
||||
};
|
||||
}
|
||||
@@ -32,6 +32,12 @@ export function buildRecoveryHint(
|
||||
next_step:
|
||||
"No active accounts are connected for this combo. Open /dashboard/providers, reconnect at least one, then retry.",
|
||||
};
|
||||
case "quota_exhausted":
|
||||
return {
|
||||
action: "switch-combo",
|
||||
next_step:
|
||||
"Every target in this combo failed with a quota or account-balance exhaustion error. Top up the account/wallet or switch to a combo/provider with available quota — this will not recover on retry.",
|
||||
};
|
||||
case "all_models_failed":
|
||||
return {
|
||||
action: "try-auto",
|
||||
|
||||
@@ -6,6 +6,10 @@ const TERMINAL_QUOTA_CODES = new Set([
|
||||
"credits_exhausted",
|
||||
"insufficient_quota",
|
||||
"quota_exhausted",
|
||||
// #10966: durable wallet/balance exhaustion signalled on a 403 (not 402/429) by
|
||||
// some upstreams — e.g. AUTHZ_INSUFFICIENT_BALANCE, "Insufficient account balance.
|
||||
// Top up your account at …".
|
||||
"authz_insufficient_balance",
|
||||
]);
|
||||
|
||||
const trustedClassifications = new WeakMap<Response, boolean>();
|
||||
@@ -62,7 +66,13 @@ export async function isQuotaExhaustionResponse(
|
||||
const trusted = trustedClassifications.get(response);
|
||||
if (trusted !== undefined) return trusted;
|
||||
|
||||
if (response.status !== 402 && response.status !== 429) return false;
|
||||
// #10966: 403 is included alongside 402/429 — some upstreams (e.g. durable
|
||||
// wallet/balance exhaustion) return a 403 for a terminal quota condition instead
|
||||
// of the more common 402/429. The structured-code/text checks below still gate
|
||||
// this to genuine quota signals (CREDITS_EXHAUSTED_SIGNALS / TERMINAL_QUOTA_CODES /
|
||||
// checkFallbackError's own classification), so a generic auth-only 403 (invalid
|
||||
// key, no matching quota signal) still falls through to `false`.
|
||||
if (response.status !== 402 && response.status !== 429 && response.status !== 403) return false;
|
||||
|
||||
const { text, structuredError } = await parseError(response);
|
||||
if (provider === "gemini" && response.status === 429) {
|
||||
|
||||
62
tests/unit/combo-diag-exhausted-connection-10967.test.ts
Normal file
62
tests/unit/combo-diag-exhausted-connection-10967.test.ts
Normal file
@@ -0,0 +1,62 @@
|
||||
// tests/unit/combo-diag-exhausted-connection-10967.test.ts
|
||||
// #10967: buildComboDiag (open-sse/services/combo.ts) formats exhaustedConnections
|
||||
// entries via formatExhaustedConnectionKey (open-sse/services/combo/comboDiagFormat.ts).
|
||||
// Repro: exhaustedConnections is a Set<string> keyed as `${provider}:${connectionId}`
|
||||
// (open-sse/services/combo/targetExhaustion.ts — markAuthLevelExhaustion /
|
||||
// markAgentrouterConnectionQuotaExhaustion / markConnectionLevelExhaustion, all three
|
||||
// call `sets.exhaustedConnections.add(`${provider}:${connId}`)`).
|
||||
//
|
||||
// Before the fix, the diagnostics formatter hardcoded `provider: "unknown"` and did
|
||||
// `String(c).slice(0, 8)` on the WHOLE key, so a real-world key like
|
||||
// `jina-ai:3f9a1c2e-...` (7-char provider id + colon = 8 chars) produced
|
||||
// `{ provider: "unknown", reason: "exhausted_connection:jina-ai:" }` — the UUID
|
||||
// entirely dropped and the real provider id discarded.
|
||||
//
|
||||
// This test imports the REAL exported function used by buildComboDiag's
|
||||
// `excluded` mapping — not a re-implementation of the truncation logic.
|
||||
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { formatExhaustedConnectionKey } from "../../open-sse/services/combo/comboDiagFormat.ts";
|
||||
|
||||
test("formatExhaustedConnectionKey: real jina-ai UUID key keeps the provider id and truncates only the connection id", () => {
|
||||
const key = "jina-ai:3f9a1c2e-8b7d-4e11-9c2a-1a2b3c4d5e6f";
|
||||
const { provider, reason } = formatExhaustedConnectionKey(key);
|
||||
|
||||
// The exact #10967 regression: provider must NOT be "unknown" when the key
|
||||
// carries a real provider prefix, and the reason must not read as an empty
|
||||
// truncated "provider:" artifact.
|
||||
assert.equal(provider, "jina-ai");
|
||||
assert.notEqual(provider, "unknown");
|
||||
assert.doesNotMatch(reason, /exhausted_connection:jina-ai:$/);
|
||||
assert.equal(reason, "exhausted_connection:3f9a1c2e");
|
||||
});
|
||||
|
||||
test("formatExhaustedConnectionKey: connection id is truncated to 8 chars, never leaking the full UUID", () => {
|
||||
const { reason } = formatExhaustedConnectionKey("openai:aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee");
|
||||
assert.equal(reason, "exhausted_connection:aaaaaaaa");
|
||||
assert.ok(!reason.includes("bbbb-cccc-dddd"), "must not leak the full connection UUID");
|
||||
});
|
||||
|
||||
test("formatExhaustedConnectionKey: short provider ids (e.g. glm) still resolve correctly", () => {
|
||||
const { provider, reason } = formatExhaustedConnectionKey("glm:0123456789abcdef");
|
||||
assert.equal(provider, "glm");
|
||||
assert.equal(reason, "exhausted_connection:01234567");
|
||||
});
|
||||
|
||||
test("formatExhaustedConnectionKey: falls back to unknown when the key carries no provider prefix", () => {
|
||||
const { provider, reason } = formatExhaustedConnectionKey("no-colon-here");
|
||||
assert.equal(provider, "unknown");
|
||||
assert.equal(reason, "exhausted_connection:no-colon");
|
||||
});
|
||||
|
||||
test("formatExhaustedConnectionKey: builds the same excluded-entry shape buildComboDiag emits", () => {
|
||||
// Mirrors the Set-of-keys -> excluded[] mapping that lives inline in
|
||||
// buildComboDiag (open-sse/services/combo.ts), proving the exported helper
|
||||
// produces exactly the shape the diagnostics payload expects.
|
||||
const exhaustedConnections = new Set(["jina-ai:3f9a1c2e-8b7d-4e11-9c2a-1a2b3c4d5e6f"]);
|
||||
const excluded = [...exhaustedConnections].map((c) => formatExhaustedConnectionKey(String(c)));
|
||||
assert.deepEqual(excluded, [
|
||||
{ provider: "jina-ai", reason: "exhausted_connection:3f9a1c2e" },
|
||||
]);
|
||||
});
|
||||
81
tests/unit/combo-recovery-quota-10966.test.ts
Normal file
81
tests/unit/combo-recovery-quota-10966.test.ts
Normal file
@@ -0,0 +1,81 @@
|
||||
// tests/unit/combo-recovery-quota-10966.test.ts
|
||||
// #10966: a combo that exhausts because every observed failure was a durable
|
||||
// quota/balance error (HTTP 403 `insufficient_quota` / `AUTHZ_INSUFFICIENT_BALANCE`,
|
||||
// or the more common 402/429 quota signals) must NOT ship `recovery.action = "retry"`.
|
||||
//
|
||||
// Root cause chain (both pieces exercised here against the REAL exported functions,
|
||||
// not re-implemented logic):
|
||||
//
|
||||
// 1. open-sse/services/combo/quotaExhaustion.ts::isQuotaExhaustionResponse only
|
||||
// classified status 402/429 as quota-exhaustion; a 403 insufficient_quota /
|
||||
// AUTHZ_INSUFFICIENT_BALANCE response (the issue's exact repro) fell straight
|
||||
// through to `false`, so combo.ts's `allObservedFailuresQuota` accumulator
|
||||
// never went (and stayed) true for this class of failure.
|
||||
// 2. open-sse/services/combo/pinRecovery.ts::buildRecoveryHint had no case for a
|
||||
// stable "quota_exhausted" terminalReason, so even a caller that DID resolve
|
||||
// the reason correctly fell through to the `default` "retry" / "failed
|
||||
// transiently" branch — actively wrong advice for a durable wallet exhaustion.
|
||||
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { buildRecoveryHint } from "../../open-sse/services/combo/pinRecovery.ts";
|
||||
import { isQuotaExhaustionResponse } from "../../open-sse/services/combo/quotaExhaustion.ts";
|
||||
|
||||
test("buildRecoveryHint: quota_exhausted terminalReason maps to switch-combo, not retry", () => {
|
||||
const hint = buildRecoveryHint("quota_exhausted");
|
||||
assert.equal(hint.action, "switch-combo");
|
||||
assert.notEqual(hint.action, "retry");
|
||||
assert.match(hint.next_step, /quota|balance/i);
|
||||
assert.doesNotMatch(hint.next_step, /failed transiently/i);
|
||||
});
|
||||
|
||||
test("buildRecoveryHint: the raw upstream message (pre-fix behavior) still falls through to retry — proves the default branch is the bug surface, not a strawman", () => {
|
||||
// This is the literal bug from the issue: combo.ts used to pass `lastError` (the
|
||||
// raw upstream string) straight into buildComboDiag/buildRecoveryHint instead of
|
||||
// a stable code — this assertion documents that the raw-string path is still
|
||||
// (correctly) unmapped, which is exactly why combo.ts must stamp "quota_exhausted"
|
||||
// instead of forwarding the raw message.
|
||||
const hint = buildRecoveryHint("Insufficient account balance. Top up your account at …");
|
||||
assert.equal(hint.action, "retry");
|
||||
});
|
||||
|
||||
test("isQuotaExhaustionResponse: HTTP 403 insufficient_quota code is classified as quota exhaustion (#10966 repro)", async () => {
|
||||
const response = new Response(
|
||||
JSON.stringify({ error: { code: "insufficient_quota", message: "Insufficient account balance." } }),
|
||||
{ status: 403 }
|
||||
);
|
||||
const exhausted = await isQuotaExhaustionResponse(response, "some-provider", "some-model", null);
|
||||
assert.equal(exhausted, true);
|
||||
});
|
||||
|
||||
test("isQuotaExhaustionResponse: HTTP 403 AUTHZ_INSUFFICIENT_BALANCE type is classified as quota exhaustion (#10966 repro)", async () => {
|
||||
const response = new Response(
|
||||
JSON.stringify({
|
||||
error: {
|
||||
type: "AUTHZ_INSUFFICIENT_BALANCE",
|
||||
message: "Insufficient account balance. Top up your account at https://example.invalid/billing",
|
||||
},
|
||||
}),
|
||||
{ status: 403 }
|
||||
);
|
||||
const exhausted = await isQuotaExhaustionResponse(response, "some-provider", "some-model", null);
|
||||
assert.equal(exhausted, true);
|
||||
});
|
||||
|
||||
test("isQuotaExhaustionResponse: a generic 403 with no quota signal stays false (no over-widening)", async () => {
|
||||
const response = new Response(
|
||||
JSON.stringify({ error: { code: "invalid_api_key", message: "Invalid API key provided." } }),
|
||||
{ status: 403 }
|
||||
);
|
||||
const exhausted = await isQuotaExhaustionResponse(response, "some-provider", "some-model", null);
|
||||
assert.equal(exhausted, false);
|
||||
});
|
||||
|
||||
test("isQuotaExhaustionResponse: 402 payment-required still classified as quota exhaustion (pre-existing behavior preserved)", async () => {
|
||||
const response = new Response(
|
||||
JSON.stringify({ error: { code: "insufficient_quota", message: "Payment required." } }),
|
||||
{ status: 402 }
|
||||
);
|
||||
const exhausted = await isQuotaExhaustionResponse(response, "some-provider", "some-model", null);
|
||||
assert.equal(exhausted, true);
|
||||
});
|
||||
Reference in New Issue
Block a user