diff --git a/changelog.d/fixes/8396-cooldown-429-cap.md b/changelog.d/fixes/8396-cooldown-429-cap.md new file mode 100644 index 0000000000..684e2e1f6f --- /dev/null +++ b/changelog.d/fixes/8396-cooldown-429-cap.md @@ -0,0 +1 @@ +- fix(resilience): cap the connection cooldown after a 429 burst so combo fallback is not blacked out past the real rate-limit window (#8396) diff --git a/open-sse/services/accountFallback.ts b/open-sse/services/accountFallback.ts index 28f871719f..c10ade929f 100644 --- a/open-sse/services/accountFallback.ts +++ b/open-sse/services/accountFallback.ts @@ -56,6 +56,7 @@ import { import { parseDayGranularityResetMs, shouldPreserveQuotaSignals } from "./quotaResetParsing.ts"; import { evictLockoutOverflow } from "./accountFallback/lockoutEviction.ts"; export { MODEL_LOCKOUT_EVICTION_CAP } from "./accountFallback/lockoutEviction.ts"; +import { capScaledCooldownMs } from "./accountFallback/cooldownCap.ts"; export type ProviderProfile = { baseCooldownMs: number; @@ -1451,9 +1452,14 @@ export function checkFallbackError( typeof profile?.baseCooldownMs === "number" && profile.baseCooldownMs >= 0 ? profile.baseCooldownMs : COOLDOWN_MS.transientInitial; + // #8396: cap against profile.maxCooldownMs, mirroring the model-lockout path. return { baseCooldownMs, - cooldownMs: getScaledCooldown(baseCooldownMs, level + 1, maxBackoffSteps), + cooldownMs: capScaledCooldownMs( + getScaledCooldown(baseCooldownMs, level + 1, maxBackoffSteps), + profile?.maxCooldownMs, + BACKOFF_CONFIG.max + ), newBackoffLevel: Math.min(level + 1, maxBackoffSteps), }; } diff --git a/open-sse/services/accountFallback/cooldownCap.ts b/open-sse/services/accountFallback/cooldownCap.ts new file mode 100644 index 0000000000..66a50da03b --- /dev/null +++ b/open-sse/services/accountFallback/cooldownCap.ts @@ -0,0 +1,26 @@ +/** + * accountFallback/cooldownCap.ts — absolute ceiling for exponentially-scaled cooldowns. + * + * Extracted from services/accountFallback.ts (file-size gate): a pure, one-purpose clamp + * so the connection-level 429/retryable-error cooldown path (getScaledBaseCooldown, inside + * checkFallbackError) applies the same absolute ceiling the model-lockout path + * (recordModelLockoutFailure) already enforces. Fixes #8396 — after a sustained 429 burst + * pushed backoffLevel high, `baseCooldownMs * 2^level` had no upper bound on this path and + * could black a connection out for hours, long past any real rate-limit reset window. + */ + +/** + * Clamp an exponentially-scaled cooldown to `maxCooldownMs` (operator-configured, per + * ProviderProfile) falling back to `fallbackMaxMs` (e.g. BACKOFF_CONFIG.max) when the + * profile did not configure one. Never widens the cooldown, only bounds it — the + * exponential backoff itself is untouched. + */ +export function capScaledCooldownMs( + cooldownMs: number, + maxCooldownMs: number | undefined | null, + fallbackMaxMs: number +): number { + const cap = + typeof maxCooldownMs === "number" && maxCooldownMs > 0 ? maxCooldownMs : fallbackMaxMs; + return Math.min(cooldownMs, cap); +} diff --git a/tests/unit/8396-cooldown-429-cap.test.ts b/tests/unit/8396-cooldown-429-cap.test.ts new file mode 100644 index 0000000000..bbdc9a0157 --- /dev/null +++ b/tests/unit/8396-cooldown-429-cap.test.ts @@ -0,0 +1,80 @@ +// Regression guard for #8396: after a burst of retryable failures (e.g. 429s), the +// connection-level cooldown computed by checkFallbackError() must never exceed +// profile.maxCooldownMs. Before the fix, getScaledBaseCooldown() scaled +// baseCooldownMs * 2^backoffLevel with NO absolute ceiling on the connection-level +// path (unlike the model-lockout path, which already clamps to maxCooldownMs). A +// legacy-migrated OAuth profile with baseCooldownMs=60000 and maxBackoffSteps=8 +// produced cooldownMs = 60000 * 2^8 = 15,360,000ms (~4.27h), blowing straight past +// an operator-configured maxCooldownMs of 10 minutes. +import test from "node:test"; +import assert from "node:assert/strict"; +import { checkFallbackError, type ProviderProfile } from "../../open-sse/services/accountFallback.ts"; + +const legacyMigratedOAuthProfile: ProviderProfile = { + baseCooldownMs: 60000, + useUpstreamRetryHints: false, + maxCooldownMs: 600000, // operator-configured 10-minute ceiling + maxBackoffSteps: 8, + failureThreshold: 3, + resetTimeoutMs: 30 * 60 * 1000, + transientCooldown: 5000, + rateLimitCooldown: 60000, + maxBackoffLevel: 8, + circuitBreakerThreshold: 3, + circuitBreakerReset: 60000, + providerFailureThreshold: 3, + providerFailureWindowMs: 60000, + providerCooldownMs: 60000, +}; + +test("#8396: connection-level 429 cooldown after a high-failureIndex burst is capped at profile.maxCooldownMs", () => { + const result = checkFallbackError( + 429, + "", + 8, // backoffLevel — a large failureIndex from a sustained 429 burst + "some-model", + "test-oauth-provider", + null, + legacyMigratedOAuthProfile + ); + + assert.equal(result.shouldFallback, true); + assert.ok( + result.cooldownMs <= legacyMigratedOAuthProfile.maxCooldownMs, + `expected cooldownMs to be capped at ${legacyMigratedOAuthProfile.maxCooldownMs}ms ` + + `(profile.maxCooldownMs), but got ${result.cooldownMs}ms — no absolute ceiling was ` + + "applied on the connection-level 429 cooldown path" + ); +}); + +test("#8396: an upstream Retry-After hint is honored (bypasses the exponential scale entirely)", () => { + const apikeyProfile: ProviderProfile = { + ...legacyMigratedOAuthProfile, + useUpstreamRetryHints: true, + }; + const headers = new Headers({ "retry-after": "30" }); + + const result = checkFallbackError(429, "", 8, "some-model", "test-apikey-provider", headers, apikeyProfile); + + assert.equal(result.usedUpstreamRetryHint, true); + assert.ok( + result.cooldownMs <= 31000 && result.cooldownMs >= 29000, + `expected the ~30s upstream Retry-After hint to be honored, got ${result.cooldownMs}ms` + ); +}); + +test("#8396: a single 429 (low backoffLevel) still cools down normally, well under the cap", () => { + const result = checkFallbackError( + 429, + "", + 0, // first failure + "some-model", + "test-oauth-provider", + null, + legacyMigratedOAuthProfile + ); + + assert.equal(result.shouldFallback, true); + assert.equal(result.cooldownMs, legacyMigratedOAuthProfile.baseCooldownMs); + assert.ok(result.cooldownMs < legacyMigratedOAuthProfile.maxCooldownMs); +});