mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-05 14:52:09 +03:00
#7268: classifyProviderError() only inspected the response body for model-unavailable wording on 400/403/404, so a 401 body like "Model X is not supported" (free-tier/aggregator providers) fell through to a generic UNAUTHORIZED classification. Because chatCore.ts only calls lockModel(..., "model_not_found", ...) on the MODEL_NOT_FOUND branch, the broken model was never locked out and auto-combo kept re-selecting it every request. Added a shared containsModelUnavailableMessage() regex (bounded, ReDoS-safe) in errorClassifier.ts, consulted by the 401 branch before falling back to ACCOUNT_DEACTIVATED/UNAUTHORIZED, and reused by modelFamilyFallback.ts's isModelUnavailableError() for the literal "<model> is not supported" phrasing. #7387: applySessionStickiness() (combo-level session stickiness) only gated a sticky pin's release on testStatus (credits_exhausted/banned/expired) and rateLimitedUntil (#6692's fix). It never consulted isAccountQuotaExhausted() (src/domain/quotaCache.ts) — the authoritative per-window (5h/weekly) quota signal that src/sse/services/auth.ts and sessionAffinityPin.ts (the provider-level pin) already gate on. A connection whose quota window was depleted, but that hadn't yet received a hard failure severe enough to flip testStatus/rateLimitedUntil, was re-promoted to position 0 on every request regardless of routing strategy. Added isStickyConnectionQuotaExhausted(), a dynamic-import seam (mirroring resolveConnectionHealth/resolveSaturation, no new static edge from open-sse/ into src/domain/) with an injectable checker for tests, gating the release condition alongside the existing checks. Regression tests: tests/unit/repro-7268-401-model-not-supported-lockout.test.ts, tests/unit/repro-7387-sticky-quota-exhausted.test.ts (both RED before, GREEN after). Existing sticky/error-classifier suites re-run and stay green. Closes #7268 Closes #7387
This commit is contained in:
committed by
GitHub
parent
eb92e626d2
commit
054df422be
@@ -0,0 +1 @@
|
||||
- fix(sse): classify 401 "model X is not supported" as model-not-found so it locks the model out instead of looping forever (#7268)
|
||||
1
changelog.d/fixes/7387-sticky-quota-exhausted.md
Normal file
1
changelog.d/fixes/7387-sticky-quota-exhausted.md
Normal file
@@ -0,0 +1 @@
|
||||
- fix(sse): combo session stickiness now releases a connection whose per-window quota is exhausted, matching the provider-level session-affinity pin (#7387)
|
||||
@@ -35,6 +35,15 @@
|
||||
* the same dynamic-import-with-injectable-override seam (fail-open on lookup
|
||||
* errors, mirroring resolveSaturation) and gates the pin alongside headroom.
|
||||
* For tests the fetcher is injected via __setStickinessConnectionFetcherForTests.
|
||||
* • Quota-exhaustion gate (#7387): testStatus/rateLimitedUntil alone still
|
||||
* miss a connection whose 5h/weekly quota window is depleted but that
|
||||
* hasn't (yet) received a hard failure severe enough to flip either field —
|
||||
* exactly what a quota-preflight/dashboard-detected depletion looks like
|
||||
* before any upstream 429 lands for this run. isAccountQuotaExhausted()
|
||||
* (src/domain/quotaCache.ts) is the authoritative per-window signal the rest
|
||||
* of the credential-selection pipeline already gates on (auth.ts,
|
||||
* sessionAffinityPin.ts); it now also releases the combo-level sticky pin.
|
||||
* For tests the checker is injected via __setStickinessQuotaCheckerForTests.
|
||||
*
|
||||
* No barrel import — consistent with the other combo/* helpers.
|
||||
*
|
||||
@@ -164,6 +173,51 @@ export function isStickyConnectionTerminallyUnhealthy(
|
||||
return Number.isFinite(rl) && rl > now;
|
||||
}
|
||||
|
||||
// ─── Per-window quota-exhaustion gate (#7387) ────────────────────────────────
|
||||
|
||||
/**
|
||||
* Injectable quota-exhaustion checker seam (for unit tests that don't want to
|
||||
* hydrate the real in-memory quota cache).
|
||||
*/
|
||||
export type QuotaExhaustionChecker = (connectionId: string) => boolean;
|
||||
|
||||
let _quotaExhaustionOverride: QuotaExhaustionChecker | null = null;
|
||||
|
||||
/** Test-only: inject the quota-exhaustion checker; pass null to restore default. */
|
||||
export function __setStickinessQuotaCheckerForTests(
|
||||
checker: QuotaExhaustionChecker | null
|
||||
): void {
|
||||
_quotaExhaustionOverride = checker;
|
||||
}
|
||||
|
||||
/**
|
||||
* Is the sticky-bound connection's per-window (5h/weekly) quota exhausted?
|
||||
*
|
||||
* `isStickyConnectionTerminallyUnhealthy` above only looks at testStatus/
|
||||
* rateLimitedUntil (#6692) — it misses a connection whose quota window is
|
||||
* fully depleted (per src/domain/quotaCache.ts::isAccountQuotaExhausted, the
|
||||
* same authoritative per-window signal src/sse/services/auth.ts and
|
||||
* sessionAffinityPin.ts already gate on) but that hasn't yet received a hard
|
||||
* failure severe enough to flip testStatus or set rateLimitedUntil. Without
|
||||
* this check the combo-level sticky pin re-promotes the depleted account on
|
||||
* every request, defeating whatever strategy picked a healthy one. (#7387)
|
||||
*
|
||||
* Dynamic import (mirroring resolveConnectionHealth/resolveSaturation above)
|
||||
* so this open-sse/ leaf keeps no static edge into src/domain/. Fail-open
|
||||
* (false) on any lookup error — an unresolved check must never drop a
|
||||
* healthy pin.
|
||||
*/
|
||||
async function isStickyConnectionQuotaExhausted(connectionId: string): Promise<boolean> {
|
||||
if (_quotaExhaustionOverride) return _quotaExhaustionOverride(connectionId);
|
||||
|
||||
try {
|
||||
const mod = await import("../../../src/domain/quotaCache");
|
||||
return Boolean(mod.isAccountQuotaExhausted(connectionId));
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the HeadroomSaturation for a connection by fetching both the 5h and
|
||||
* weekly utilisation signals. Uses the same dynamic-import pattern as
|
||||
@@ -374,15 +428,17 @@ export async function applySessionStickiness(
|
||||
// accounts report healthy 5h/weekly utilization, so headroom alone never
|
||||
// catches them).
|
||||
const stickyTarget = orderedTargets[stickyIdx];
|
||||
const [sat, connHealth] = await Promise.all([
|
||||
const [sat, connHealth, quotaExhausted] = await Promise.all([
|
||||
resolveSaturation(connectionId, stickyTarget.provider),
|
||||
resolveConnectionHealth(connectionId, stickyTarget.provider),
|
||||
isStickyConnectionQuotaExhausted(connectionId),
|
||||
]);
|
||||
const headroom = computeHeadroom(sat);
|
||||
|
||||
if (
|
||||
headroom <= STICKINESS_HEADROOM_THRESHOLD ||
|
||||
isStickyConnectionTerminallyUnhealthy(connHealth, Date.now())
|
||||
isStickyConnectionTerminallyUnhealthy(connHealth, Date.now()) ||
|
||||
quotaExhausted
|
||||
) {
|
||||
// Connection saturated or durably unhealthy — rebind on next success
|
||||
clearStickyBinding(messageHash);
|
||||
|
||||
@@ -99,6 +99,19 @@ export function isContextOverflow(errorText: string): boolean {
|
||||
return CONTEXT_OVERFLOW_REGEX.test(String(errorText || ""));
|
||||
}
|
||||
|
||||
// Matches phrasing like `Model minimax-m3-free is not supported` or
|
||||
// `model "gpt-9" is not supported` — free-tier/aggregator providers name the
|
||||
// specific model in the sentence instead of using a fixed fragment like
|
||||
// "model not supported". Shared by modelFamilyFallback.ts's
|
||||
// isModelUnavailableError() (400/403/404) and this module's 401 branch below,
|
||||
// so the same phrasing locks the model out on either status. Bounded
|
||||
// quantifier ({0,80}) keeps it ReDoS-safe. (#7268)
|
||||
const MODEL_NAMED_UNSUPPORTED_REGEX = /\bmodel\b[^\n]{0,80}\bis not supported\b/i;
|
||||
|
||||
export function containsModelUnavailableMessage(errorMessage: string): boolean {
|
||||
return MODEL_NAMED_UNSUPPORTED_REGEX.test(String(errorMessage || "").toLowerCase());
|
||||
}
|
||||
|
||||
function responseBodyToString(responseBody: unknown): string {
|
||||
if (typeof responseBody === "string") return responseBody;
|
||||
if (responseBody !== null && typeof responseBody === "object") {
|
||||
@@ -158,6 +171,16 @@ export function classifyProviderError(
|
||||
if (oauthInvalid) {
|
||||
return PROVIDER_ERROR_TYPES.OAUTH_INVALID_TOKEN;
|
||||
}
|
||||
// Some free-tier/aggregator providers return 401 (instead of 404) for a
|
||||
// model the account isn't entitled to, with a body like "Model X is not
|
||||
// supported". Without this check the error falls through to a generic
|
||||
// UNAUTHORIZED classification, which never triggers lockModel() in
|
||||
// chatCore.ts — auto-combo keeps re-selecting the same broken model on
|
||||
// every request. Detect the phrasing here, same as the 404 branch above
|
||||
// always does regardless of body content. (#7268)
|
||||
if (containsModelUnavailableMessage(bodyStr)) {
|
||||
return PROVIDER_ERROR_TYPES.MODEL_NOT_FOUND;
|
||||
}
|
||||
return accountDeactivated
|
||||
? PROVIDER_ERROR_TYPES.ACCOUNT_DEACTIVATED
|
||||
: PROVIDER_ERROR_TYPES.UNAUTHORIZED;
|
||||
|
||||
@@ -13,7 +13,7 @@
|
||||
|
||||
import { getModelContextLimit } from "../../src/lib/modelCapabilities";
|
||||
import { parseModel } from "./model.ts";
|
||||
import { CONTEXT_OVERFLOW_REGEX } from "./errorClassifier.ts";
|
||||
import { CONTEXT_OVERFLOW_REGEX, containsModelUnavailableMessage } from "./errorClassifier.ts";
|
||||
import { getRegistryEntry } from "../config/providerRegistry.ts";
|
||||
|
||||
// ── Model Family Definitions ─────────────────────────────────────────────────
|
||||
@@ -129,7 +129,8 @@ export function isModelUnavailableError(status: number, errorMessage: string): b
|
||||
if (status !== 400 && status !== 403) return false;
|
||||
|
||||
const msg = errorMessage.toLowerCase();
|
||||
return MODEL_UNAVAILABLE_FRAGMENTS.some((fragment) => msg.includes(fragment));
|
||||
if (MODEL_UNAVAILABLE_FRAGMENTS.some((fragment) => msg.includes(fragment))) return true;
|
||||
return containsModelUnavailableMessage(errorMessage);
|
||||
}
|
||||
|
||||
export function isContextOverflowError(status: number, errorMessage: string): boolean {
|
||||
|
||||
@@ -0,0 +1,48 @@
|
||||
/**
|
||||
* TDD repro/regression test for issue #7268 — "Model X is not supported"
|
||||
* 401 responses never lock the model out.
|
||||
*
|
||||
* Root cause: classifyProviderError() only inspects the response body for
|
||||
* status codes 400/403/404 to detect a model-unavailable signal. For status
|
||||
* 401 it only checks isOAuthInvalidToken()/isAccountDeactivated() and falls
|
||||
* through to a generic UNAUTHORIZED classification — even when the body
|
||||
* literally says "Model X is not supported". Because chatCore.ts only calls
|
||||
* lockModel(..., "model_not_found", ...) on the MODEL_NOT_FOUND branch, the
|
||||
* broken model is never locked out and auto-combo keeps re-selecting it.
|
||||
*
|
||||
* Expected (correct) behavior: a 401 whose body matches a model-unavailable
|
||||
* fragment (e.g. "<model> is not supported") classifies as MODEL_NOT_FOUND,
|
||||
* the same way a 404 always does.
|
||||
*/
|
||||
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
const { classifyProviderError, PROVIDER_ERROR_TYPES } = await import(
|
||||
"../../open-sse/services/errorClassifier.ts"
|
||||
);
|
||||
const { isModelUnavailableError } = await import(
|
||||
"../../open-sse/services/modelFamilyFallback.ts"
|
||||
);
|
||||
|
||||
test("#7268: classifyProviderError(401, 'Model X is not supported') classifies as MODEL_NOT_FOUND", () => {
|
||||
const classified = classifyProviderError(401, { error: "Model minimax-m3-free is not supported" });
|
||||
assert.equal(classified, PROVIDER_ERROR_TYPES.MODEL_NOT_FOUND);
|
||||
});
|
||||
|
||||
test("#7268: classifyProviderError(401, 'Model X is not supported') for a different model name", () => {
|
||||
const classified = classifyProviderError(401, { error: "Model qwen3.6-plus-free is not supported" });
|
||||
assert.equal(classified, PROVIDER_ERROR_TYPES.MODEL_NOT_FOUND);
|
||||
});
|
||||
|
||||
test("#7268: a genuine 401 auth error (no model-unavailable wording) stays UNAUTHORIZED", () => {
|
||||
const classified = classifyProviderError(401, { error: "Invalid API key provided" });
|
||||
assert.equal(classified, PROVIDER_ERROR_TYPES.UNAUTHORIZED);
|
||||
});
|
||||
|
||||
test("#7268: isModelUnavailableError() recognizes the literal '<model> is not supported' phrase", () => {
|
||||
assert.equal(
|
||||
isModelUnavailableError(400, "Model minimax-m3-free is not supported"),
|
||||
true
|
||||
);
|
||||
});
|
||||
108
tests/unit/repro-7387-sticky-quota-exhausted.test.ts
Normal file
108
tests/unit/repro-7387-sticky-quota-exhausted.test.ts
Normal file
@@ -0,0 +1,108 @@
|
||||
/**
|
||||
* TDD repro/regression test for issue #7387 — combo-level session stickiness
|
||||
* (open-sse/services/combo/sessionStickiness.ts) never checks per-window
|
||||
* quota exhaustion (src/domain/quotaCache.ts::isAccountQuotaExhausted) before
|
||||
* re-promoting a bound connection back to position 0 of the target list.
|
||||
*
|
||||
* The provider-level session-affinity pin (src/sse/services/sessionAffinityPin.ts)
|
||||
* already gates on isAccountQuotaExhausted() correctly — sessionStickiness.ts
|
||||
* is the one place that forgot it, only checking testStatus
|
||||
* (credits_exhausted/banned/expired) and rateLimitedUntil.
|
||||
*
|
||||
* Expected (correct) behavior: once a sticky-bound connection's quota is
|
||||
* exhausted (per quotaCache, independent of testStatus/rateLimitedUntil), the
|
||||
* pin must release and the healthy target takes position 0.
|
||||
*/
|
||||
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import type { HeadroomSaturation } from "../../open-sse/services/combo/headroomRanking.ts";
|
||||
import type { StickyConnectionHealth } from "../../open-sse/services/combo/sessionStickiness.ts";
|
||||
|
||||
const stickinessMod = await import("../../open-sse/services/combo/sessionStickiness.ts");
|
||||
const {
|
||||
deriveMessageHash,
|
||||
applySessionStickiness,
|
||||
recordStickyBinding,
|
||||
clearAllStickyBindings,
|
||||
__setStickinessHeadroomFetcherForTests,
|
||||
__setStickinessConnectionFetcherForTests,
|
||||
} = stickinessMod;
|
||||
|
||||
const quotaCacheMod = await import("../../src/domain/quotaCache.ts");
|
||||
const { setQuotaCache, isAccountQuotaExhausted, __clearForTests } = quotaCacheMod;
|
||||
|
||||
function makeTarget(connectionId: string) {
|
||||
return {
|
||||
kind: "model",
|
||||
stepId: `step-${connectionId}`,
|
||||
executionKey: `key-${connectionId}`,
|
||||
modelStr: `codex/gpt-5-codex/${connectionId}`,
|
||||
provider: "codex",
|
||||
providerId: null,
|
||||
connectionId,
|
||||
weight: 1,
|
||||
label: null,
|
||||
};
|
||||
}
|
||||
|
||||
function injectSat(sat: HeadroomSaturation | undefined) {
|
||||
__setStickinessHeadroomFetcherForTests(async (_id: string) => sat);
|
||||
}
|
||||
|
||||
function injectConnectionHealth(byId: Record<string, StickyConnectionHealth | undefined>) {
|
||||
__setStickinessConnectionFetcherForTests(async (connectionId: string) => byId[connectionId]);
|
||||
}
|
||||
|
||||
test.beforeEach(() => {
|
||||
clearAllStickyBindings();
|
||||
__clearForTests();
|
||||
});
|
||||
|
||||
test.after(() => {
|
||||
__setStickinessHeadroomFetcherForTests(null);
|
||||
__setStickinessConnectionFetcherForTests(null);
|
||||
__clearForTests();
|
||||
});
|
||||
|
||||
test("#7387: sticky pin releases a QUOTA-EXHAUSTED account whose testStatus/rateLimitedUntil are still healthy", async () => {
|
||||
injectSat({ util5h: 0.05, util7d: 0.05 }); // headroom well above threshold
|
||||
injectConnectionHealth({
|
||||
"conn-codex-exhausted": { testStatus: "active", rateLimitedUntil: null },
|
||||
});
|
||||
|
||||
setQuotaCache("conn-codex-exhausted", "codex", {
|
||||
session: { remainingPercentage: 0, resetAt: null },
|
||||
weekly: { remainingPercentage: 0, resetAt: null },
|
||||
});
|
||||
assert.equal(isAccountQuotaExhausted("conn-codex-exhausted"), true);
|
||||
|
||||
const targets = [makeTarget("conn-healthy"), makeTarget("conn-codex-exhausted")];
|
||||
const messages = [{ role: "user", content: "Multi-turn Codex conversation, turn 1" }];
|
||||
const hash = deriveMessageHash(messages)!;
|
||||
|
||||
recordStickyBinding(hash, "conn-codex-exhausted"); // turn 1: served successfully
|
||||
|
||||
const result = await applySessionStickiness(targets, messages); // turn 2+: quota now exhausted
|
||||
|
||||
assert.equal(result.stuck, false, "sticky pin must release once quota is exhausted (#7387)");
|
||||
assert.equal(result.targets[0].connectionId, "conn-healthy");
|
||||
});
|
||||
|
||||
test("#7387: sticky pin stays bound when the connection is healthy and NOT quota-exhausted", async () => {
|
||||
injectSat({ util5h: 0.05, util7d: 0.05 });
|
||||
injectConnectionHealth({
|
||||
"conn-codex-ok": { testStatus: "active", rateLimitedUntil: null },
|
||||
});
|
||||
|
||||
const targets = [makeTarget("conn-other"), makeTarget("conn-codex-ok")];
|
||||
const messages = [{ role: "user", content: "Multi-turn Codex conversation, turn 1 (healthy)" }];
|
||||
const hash = deriveMessageHash(messages)!;
|
||||
|
||||
recordStickyBinding(hash, "conn-codex-ok");
|
||||
|
||||
const result = await applySessionStickiness(targets, messages);
|
||||
|
||||
assert.equal(result.stuck, true);
|
||||
assert.equal(result.targets[0].connectionId, "conn-codex-ok");
|
||||
});
|
||||
Reference in New Issue
Block a user