From f8487648c8acee2da07fd0ad44146760514ed5dd Mon Sep 17 00:00:00 2001 From: Wilson Date: Mon, 27 Jul 2026 21:12:42 -0300 Subject: [PATCH] fix(resilience): keep resource 404s from cooling models (#8756) --- changelog.d/fixes/8756-resource-404-health.md | 1 + open-sse/services/combo.ts | 20 ++-- open-sse/services/combo/comboPredicates.ts | 13 +++ open-sse/services/errorClassifier.ts | 29 +++++- open-sse/services/modelFamilyFallback.ts | 8 +- src/sse/services/auth.ts | 14 +-- src/sse/services/requestResourceHealth.ts | 28 ++++++ stryker.conf.json | 2 + tests/unit/combo-resource-404-health.test.ts | 91 +++++++++++++++++++ tests/unit/error-classifier.test.ts | 37 ++++++-- tests/unit/sse-auth-resource-404.test.ts | 47 ++++++++++ .../t30-kiro-400-model-unavailable.test.ts | 8 ++ 12 files changed, 270 insertions(+), 28 deletions(-) create mode 100644 changelog.d/fixes/8756-resource-404-health.md create mode 100644 src/sse/services/requestResourceHealth.ts create mode 100644 tests/unit/combo-resource-404-health.test.ts create mode 100644 tests/unit/sse-auth-resource-404.test.ts diff --git a/changelog.d/fixes/8756-resource-404-health.md b/changelog.d/fixes/8756-resource-404-health.md new file mode 100644 index 0000000000..0fb18aab21 --- /dev/null +++ b/changelog.d/fixes/8756-resource-404-health.md @@ -0,0 +1 @@ +- **fix(resilience):** keep missing request resources such as Files API ids from triggering model-family fallback, model lockout, or provider/account cooldown (thanks @wilsonicdev) ([#8756](https://github.com/diegosouzapw/OmniRoute/pull/8756)) diff --git a/open-sse/services/combo.ts b/open-sse/services/combo.ts index 5ae2518d34..ce1e1dc7e2 100644 --- a/open-sse/services/combo.ts +++ b/open-sse/services/combo.ts @@ -146,6 +146,7 @@ import { clampComboDepth, shouldSkipForPredictedTtft, shouldRecordProviderBreakerFailure, + isComboRequestScopedFailure as isScopedFailure, isRequestScopedUpstreamFailure, isInputBoundRequestFailure, shouldSkipConnDisable, @@ -217,8 +218,7 @@ import { import { expandTargetsByFingerprints } from "./combo/fingerprintExpansion.ts"; import { resolveComboTargetPipeline } from "./combo/targetResolution.ts"; -export { RESET_WINDOW_NAMES }; -export { QUOTA_SOFT_DEPRIORITIZE_FACTOR, setCandidateQuotaSoftPenalty }; +export { RESET_WINDOW_NAMES, QUOTA_SOFT_DEPRIORITIZE_FACTOR, setCandidateQuotaSoftPenalty }; export { scoreAutoTargets, expandAutoComboCandidatePool }; export type { SingleModelTarget, ResolvedComboTarget }; export { validateResponseQuality }; @@ -1557,7 +1557,7 @@ export async function handleComboChat({ : undefined, } : undefined; - const requestScopedFailure = isRequestScopedUpstreamFailure(structuredError); + const scopedFailure = isScopedFailure(result.status, errorText, structuredError); // #8375: input-bound request-scoped failures (context_length_exceeded) are // deterministic for the same input — retrying on other accounts of the same @@ -1716,7 +1716,7 @@ export async function handleComboChat({ status: result.status, sameProviderNext, skipProviderBreaker: fallbackResult.skipProviderBreaker, - requestScopedFailure, + requestScopedFailure: scopedFailure, error: errorText, isProxyUnreachable: structuredError?.code === "proxy_unreachable", }) @@ -1754,7 +1754,7 @@ export async function handleComboChat({ // once the model is cooling down, retrying it would waste an upstream // call and extend the cooldown via exponential backoff. let lockoutRecorded = false; - if (provider && rawModel && retry === 0 && !requestScopedFailure) { + if (provider && rawModel && retry === 0 && !scopedFailure) { const mlSettings = resolveModelLockoutSettings(settings); if (mlSettings.enabled && mlSettings.errorCodes.includes(result.status)) { recordModelLockoutFailure( @@ -1812,7 +1812,7 @@ export async function handleComboChat({ if (i > 0) fallbackCount++; // Wire combo failures into the resilience dashboard (model-level lockout) // alongside the provider-level cooldown below — they govern different scopes. - if (provider && rawModel && !requestScopedFailure) { + if (provider && rawModel && !scopedFailure) { const mlSettings = resolveModelLockoutSettings(settings); if (mlSettings.enabled && mlSettings.errorCodes.includes(result.status)) { recordModelLockoutFailure( @@ -1846,7 +1846,7 @@ export async function handleComboChat({ resilienceSettings.providerCooldown.enabled && provider && provider !== "unknown" && - !requestScopedFailure && + !scopedFailure && !( (result.status === 500 || result.status === 429) && hasPerModelQuota(provider, rawModel) @@ -2783,7 +2783,7 @@ async function handleRoundRobinCombo({ : undefined, } : undefined; - const requestScopedFailure = isRequestScopedUpstreamFailure(structuredError); + const scopedFailure = isScopedFailure(result.status, errorText, structuredError); const fallbackResult = checkFallbackError( result.status, errorText, @@ -2835,7 +2835,7 @@ async function handleRoundRobinCombo({ if ( !isStreamReadinessFailure && !isTokenLimitBreach && - !requestScopedFailure && + !scopedFailure && TRANSIENT_FOR_SEMAPHORE.includes(result.status) && cooldownMs > 0 ) { @@ -2878,7 +2878,7 @@ async function handleRoundRobinCombo({ resilienceSettings.providerCooldown.enabled && provider && provider !== "unknown" && - !requestScopedFailure && + !scopedFailure && !( (result.status === 500 || result.status === 429) && hasPerModelQuota(provider, parseModel(modelStr).model || modelStr) diff --git a/open-sse/services/combo/comboPredicates.ts b/open-sse/services/combo/comboPredicates.ts index 07478591a8..dd150e210d 100644 --- a/open-sse/services/combo/comboPredicates.ts +++ b/open-sse/services/combo/comboPredicates.ts @@ -11,6 +11,7 @@ import { parseModel } from "../model.ts"; import { isSelfInflictedUpstreamTimeout } from "../../handlers/chatCore/cooldownClassification.ts"; import { isLocalStreamLifecycleError } from "@/shared/utils/circuitBreaker"; import { CONTEXT_OVERFLOW_PATTERNS, MODEL_ACCESS_DENIED_PATTERNS } from "../accountFallback.ts"; +import { isResourceNotFoundResponse } from "../errorClassifier.ts"; import type { ResolvedComboTarget } from "./types.ts"; // Status codes that should mark round-robin target semaphores as cooling down. @@ -197,6 +198,18 @@ export function isRequestScopedUpstreamFailure(error?: { return REQUEST_SCOPED_UPSTREAM_ERROR_CODES.has(code) || type === "context_length_exceeded"; } +/** Request-scoped classification that also has access to the HTTP body. */ +export function isComboRequestScopedFailure( + status: number, + errorText: string, + error?: { code?: string | null; type?: string | null } +): boolean { + return ( + isRequestScopedUpstreamFailure(error) || + (status === 404 && isResourceNotFoundResponse(errorText)) + ); +} + const INPUT_BOUND_ERROR_CODES = new Set(["context_length_exceeded", "context_window_exceeded"]); /** diff --git a/open-sse/services/errorClassifier.ts b/open-sse/services/errorClassifier.ts index e7f9653b07..7f1ff1dece 100644 --- a/open-sse/services/errorClassifier.ts +++ b/open-sse/services/errorClassifier.ts @@ -125,6 +125,30 @@ function responseBodyToString(responseBody: unknown): string { return ""; } +// A provider can return 404 for request-scoped resources (Files API ids, +// response items, uploads, etc.). These failures describe the request payload, +// not provider/model health. Keep every expression bounded to avoid ReDoS on +// upstream-controlled error bodies. +const RESOURCE_NOT_FOUND_PATTERNS = [ + /\bfiles?\b[^\n]{0,160}\b(?:not found|does not exist)\b/i, + /\b(?:not found|does not exist)\b[^\n]{0,160}\bfiles?\b/i, + /\b(?:input[_ -]?file|file[_ -]?id|item|response|vector[_ -]?store|upload)\b[^\n]{0,160}\b(?:not found|does not exist)\b/i, + /\b(?:not found|does not exist)\b[^\n]{0,160}\b(?:input[_ -]?file|file[_ -]?id|item|response|vector[_ -]?store|upload)\b/i, + /\bfile-[a-z0-9_-]+\b[^\n]{0,160}\b(?:not found|does not exist)\b/i, +]; + +/** + * Whether an upstream error identifies a missing request-scoped resource. + * + * Resource signals intentionally take precedence over an outer + * `code: "model_not_found"` because compatibility layers may synthesize that + * code from the HTTP status before preserving the upstream file error. + */ +export function isResourceNotFoundResponse(responseBody: unknown): boolean { + const body = responseBodyToString(responseBody); + return RESOURCE_NOT_FOUND_PATTERNS.some((pattern) => pattern.test(body)); +} + function shouldPreserveQuotaSignalsFor429(provider?: string | null): boolean { if (!provider) return true; return getProviderCategory(provider) === "oauth"; @@ -164,8 +188,11 @@ export function classifyProviderError( // falls through to `return null`, so no cooldown/lockout is applied and the // retry/backoff loop keeps hammering the dead endpoint until the upstream // rate-limits it (404 + 429 storm). Classify as MODEL_NOT_FOUND so the model - // gets locked via the cooldown layer and retries stop. (#6827) + // gets locked via the cooldown layer and retries stop. Request-scoped + // resource errors are excluded because retrying another account/model cannot + // make an unknown file/item id valid. (#6827) if (statusCode === 404) { + if (isResourceNotFoundResponse(responseBody)) return null; return PROVIDER_ERROR_TYPES.MODEL_NOT_FOUND; } diff --git a/open-sse/services/modelFamilyFallback.ts b/open-sse/services/modelFamilyFallback.ts index 9f3ff882f3..9411e0a2dc 100644 --- a/open-sse/services/modelFamilyFallback.ts +++ b/open-sse/services/modelFamilyFallback.ts @@ -13,7 +13,11 @@ import { getModelContextLimit } from "../../src/lib/modelCapabilities"; import { parseModel } from "./model.ts"; -import { CONTEXT_OVERFLOW_REGEX, containsModelUnavailableMessage } from "./errorClassifier.ts"; +import { + CONTEXT_OVERFLOW_REGEX, + containsModelUnavailableMessage, + isResourceNotFoundResponse, +} from "./errorClassifier.ts"; import { getRegistryEntry } from "../config/providerRegistry.ts"; // ── Model Family Definitions ───────────────────────────────────────────────── @@ -126,7 +130,7 @@ const MODEL_UNAVAILABLE_FRAGMENTS = [ * itself is not available, not a transient server error. */ export function isModelUnavailableError(status: number, errorMessage: string): boolean { - if (status === 404) return true; + if (status === 404) return !isResourceNotFoundResponse(errorMessage); if (status !== 400 && status !== 403) return false; const msg = errorMessage.toLowerCase(); diff --git a/src/sse/services/auth.ts b/src/sse/services/auth.ts index 204476ba31..e0c03fdbea 100644 --- a/src/sse/services/auth.ts +++ b/src/sse/services/auth.ts @@ -76,6 +76,7 @@ import { import { isNoAuthProviderBlockedBySettings } from "./noAuthProviderSettings"; import { resolveAccountProxiesFromRegistry } from "./noAuthProxyResolution"; import { getNoAuthHydrationProviderIds } from "./noAuthProviderSiblings"; +import { getResource404Bypass } from "./requestResourceHealth"; import * as log from "../utils/logger"; import { fisherYatesShuffle, getNextFromDeckSync } from "@/shared/utils/shuffleDeck"; @@ -1897,15 +1898,7 @@ export async function getProviderCredentialsWithQuotaPreflight( } } -/** - * Mark account as unavailable — reads backoffLevel from DB, calculates cooldown with exponential backoff, saves new level - * @param {string} connectionId - * @param {number} status - HTTP status code - * @param {string} errorText - Error message - * @param {string|null} provider - * @param {string|null} model - Model name for per-model lockout - * @returns {{ shouldFallback: boolean, cooldownMs: number }} - */ +/** Persist exponential-backoff state for an unavailable provider connection. */ export async function markAccountUnavailable( connectionId: string, status: number, @@ -1931,6 +1924,9 @@ export async function markAccountUnavailable( try { await currentMutex; + const resourceBypass = getResource404Bypass(status, errorText, connectionId, log); + if (resourceBypass) return resourceBypass; + // Read current connection to get backoffLevel const connectionsRaw = await getProviderConnections({ provider }); const connections = (Array.isArray(connectionsRaw) ? connectionsRaw : []) diff --git a/src/sse/services/requestResourceHealth.ts b/src/sse/services/requestResourceHealth.ts new file mode 100644 index 0000000000..ff2302b266 --- /dev/null +++ b/src/sse/services/requestResourceHealth.ts @@ -0,0 +1,28 @@ +import { isResourceNotFoundResponse } from "@omniroute/open-sse/services/errorClassifier.ts"; + +type HealthLogger = { + info(tag: string, message: string): void; +}; + +export type Resource404CooldownBypass = { + shouldFallback: false; + cooldownMs: 0; +}; + +/** + * Return a no-cooldown decision when a 404 belongs to this request's resource, + * rather than to the selected model, endpoint, account, or provider. + */ +export function getResource404Bypass( + status: number, + errorText: string, + connectionId: string, + logger: HealthLogger +): Resource404CooldownBypass | null { + if (status !== 404 || !isResourceNotFoundResponse(errorText)) return null; + logger.info( + "AUTH", + `${connectionId.slice(0, 8)} request-resource 404; skipping model/account cooldown` + ); + return { shouldFallback: false, cooldownMs: 0 }; +} diff --git a/stryker.conf.json b/stryker.conf.json index 2e1eb79e04..0f3e3285be 100644 --- a/stryker.conf.json +++ b/stryker.conf.json @@ -163,6 +163,7 @@ "tests/unit/combo-quality-validator-reasoning.test.ts", "tests/unit/combo-quota-share-cooldown-wait.test.ts", "tests/unit/combo-quota-soft-penalty.test.ts", + "tests/unit/combo-resource-404-health.test.ts", "tests/unit/combo-round-robin-streaming-lock-3811.test.ts", "tests/unit/combo-roundrobin-compat-fallback-6238.test.ts", "tests/unit/combo-routing-engine.test.ts", @@ -287,6 +288,7 @@ "tests/unit/settings/authz-bypass.test.ts", "tests/unit/skip-provider-breaker-consumer-2743.test.ts", "tests/unit/sse-auth-antigravity-credits.test.ts", + "tests/unit/sse-auth-resource-404.test.ts", "tests/unit/sse-auth.test.ts", "tests/unit/strict-random-deck.test.ts", "tests/unit/strip-reasoning-header.test.ts", diff --git a/tests/unit/combo-resource-404-health.test.ts b/tests/unit/combo-resource-404-health.test.ts new file mode 100644 index 0000000000..a7d318a8d3 --- /dev/null +++ b/tests/unit/combo-resource-404-health.test.ts @@ -0,0 +1,91 @@ +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(), "omr-combo-resource-404-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.API_KEY_SECRET = "test-combo-resource-404"; + +const core = await import("../../src/lib/db/core.ts"); +const { handleComboChat } = await import("../../open-sse/services/combo.ts"); +const { clearAllModelLockouts, getModelLockoutInfo } = + await import("../../open-sse/services/accountFallback.ts"); +const { clearCooldownState, isProviderInCooldown } = + await import("../../open-sse/services/providerCooldownTracker.ts"); + +const settings = { + modelLockout: { + enabled: true, + errorCodes: [404], + baseCooldownMs: 120_000, + maxCooldownMs: 1_800_000, + maxBackoffSteps: 10, + useExponentialBackoff: true, + }, + providerCooldown: { + enabled: true, + minRetryCooldownMs: 1_000, + maxRetryCooldownMs: 60_000, + }, +}; + +const log = { + info: () => {}, + warn: () => {}, + error: () => {}, + debug: () => {}, +}; + +test.beforeEach(() => { + clearAllModelLockouts(); + clearCooldownState(); +}); + +test.after(() => { + clearAllModelLockouts(); + clearCooldownState(); + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +test("combo resource 404 never records model lockout or provider cooldown", async () => { + const provider = "openai"; + const models = ["gpt-5", "gpt-4o"]; + + const result = await handleComboChat({ + body: { messages: [{ role: "user", content: "read the attached file" }] }, + combo: { + name: "resource-404-health", + strategy: "priority", + models: models.map((model) => `${provider}/${model}`), + config: { maxRetries: 0, retryDelayMs: 0, fallbackDelayMs: 0 }, + }, + handleSingleModel: async () => + new Response( + JSON.stringify({ + error: { + message: "[404]: Files [file-be30851bd1614656872e725e] were not found", + type: "invalid_request_error", + code: "model_not_found", + }, + }), + { status: 404, headers: { "content-type": "application/json" } } + ), + isModelAvailable: async () => true, + log, + settings, + allCombos: null, + }); + + assert.equal(result.status, 404); + for (const model of models) { + assert.equal( + getModelLockoutInfo(provider, "", model), + null, + `${model} must remain available after a request-resource 404` + ); + } + assert.equal(isProviderInCooldown(provider, undefined, settings), false); +}); diff --git a/tests/unit/error-classifier.test.ts b/tests/unit/error-classifier.test.ts index 4a427c8180..4a13235779 100644 --- a/tests/unit/error-classifier.test.ts +++ b/tests/unit/error-classifier.test.ts @@ -1,7 +1,7 @@ import test from "node:test"; import assert from "node:assert/strict"; -const { classifyProviderError, PROVIDER_ERROR_TYPES } = +const { classifyProviderError, isResourceNotFoundResponse, PROVIDER_ERROR_TYPES } = await import("../../open-sse/services/errorClassifier.ts"); test("classifyProviderError: 401 + account_deactivated => ACCOUNT_DEACTIVATED", () => { @@ -146,10 +146,35 @@ test("classifyProviderError: 404 => MODEL_NOT_FOUND", () => { }); test("classifyProviderError: 404 with provider => MODEL_NOT_FOUND", () => { - const result = classifyProviderError( - 404, - { error: { message: "Not Found" } }, - "v0-vercel" - ); + const result = classifyProviderError(404, { error: { message: "Not Found" } }, "v0-vercel"); assert.equal(result, PROVIDER_ERROR_TYPES.MODEL_NOT_FOUND); }); + +test("classifyProviderError: Files API 404 is request-scoped, not MODEL_NOT_FOUND", () => { + const body = { + error: { + message: "[404]: Files [file-be30851bd1614656872e725e] were not found", + type: "invalid_request_error", + // A compatibility layer may derive this from the HTTP status before the + // upstream file message is inspected. The resource signal must win. + code: "model_not_found", + }, + }; + + assert.equal(isResourceNotFoundResponse(body), true); + assert.equal(classifyProviderError(404, body, "codex"), null); +}); + +test("classifyProviderError: other request-resource 404 shapes do not poison model health", () => { + const bodies = [ + { error: { message: "input_file file_id does not exist" } }, + { error: { message: "Response resp_123 was not found" } }, + { error: { message: "vector_store vs_123 not found" } }, + "Upload upload_123 does not exist", + ]; + + for (const body of bodies) { + assert.equal(isResourceNotFoundResponse(body), true); + assert.equal(classifyProviderError(404, body, "openai"), null); + } +}); diff --git a/tests/unit/sse-auth-resource-404.test.ts b/tests/unit/sse-auth-resource-404.test.ts new file mode 100644 index 0000000000..9980627c91 --- /dev/null +++ b/tests/unit/sse-auth-resource-404.test.ts @@ -0,0 +1,47 @@ +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-auth-resource-404-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.API_KEY_SECRET = "auth-resource-404-test-secret"; + +const core = await import("../../src/lib/db/core.ts"); +const providersDb = await import("../../src/lib/db/providers.ts"); +const auth = await import("../../src/sse/services/auth.ts"); + +test.after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +test("markAccountUnavailable preserves connection health for a missing Files API resource", async () => { + const connection = await providersDb.createProviderConnection({ + provider: "codex", + authType: "apikey", + name: "request-resource-404", + apiKey: "sk-request-resource-404", + isActive: true, + testStatus: "active", + providerSpecificData: { + baseUrl: "https://chatgpt.com/backend-api/codex", + }, + }); + + const result = await auth.markAccountUnavailable( + connection.id, + 404, + "[404]: Files [file-be30851bd1614656872e725e] were not found", + "codex", + "gpt-5.5-medium" + ); + const updated = await providersDb.getProviderConnectionById(connection.id); + + assert.deepEqual(result, { shouldFallback: false, cooldownMs: 0 }); + assert.equal(updated.testStatus, "active"); + assert.equal(updated.rateLimitedUntil, undefined); + assert.equal(updated.backoffLevel, 0); + assert.equal(updated.lastError, undefined); +}); diff --git a/tests/unit/t30-kiro-400-model-unavailable.test.ts b/tests/unit/t30-kiro-400-model-unavailable.test.ts index c58a2e327d..e59cde1035 100644 --- a/tests/unit/t30-kiro-400-model-unavailable.test.ts +++ b/tests/unit/t30-kiro-400-model-unavailable.test.ts @@ -22,6 +22,14 @@ test("T30: 404 still maps to model-unavailable", () => { assert.equal(unavailable, true); }); +test("T30: a missing Files API resource does not trigger model-family fallback", () => { + const unavailable = isModelUnavailableError( + 404, + "[404]: Files [file-be30851bd1614656872e725e] were not found" + ); + assert.equal(unavailable, false); +}); + test("T30: model family helper returns a sibling candidate when available", () => { const next = getNextFamilyFallback("gemini-3.1-pro-high", new Set(["gemini-3.1-pro-high"])); assert.equal(typeof next, "string");