From 3b1083cf7ff3336c24e6ba2f019222647ca72168 Mon Sep 17 00:00:00 2001 From: adevwithpurpose Date: Sat, 15 Aug 2026 19:16:57 -0300 Subject: [PATCH] fix(resilience): mark embed connection terminal on hard upstream failure so dead accounts are not re-hit (#10347) --- open-sse/handlers/embeddings.ts | 23 ++++ .../providers/[provider]/embeddings/route.ts | 9 +- src/lib/embeddings/service.ts | 7 +- tests/unit/10347-embed-402-cooldown.test.ts | 103 ++++++++++++++++++ 4 files changed, 140 insertions(+), 2 deletions(-) create mode 100644 tests/unit/10347-embed-402-cooldown.test.ts diff --git a/open-sse/handlers/embeddings.ts b/open-sse/handlers/embeddings.ts index 7846ec3d58..0945e3138a 100644 --- a/open-sse/handlers/embeddings.ts +++ b/open-sse/handlers/embeddings.ts @@ -35,6 +35,7 @@ import { prepareStructuredEmbeddingRequest, } from "./embeddingStructuredInput.ts"; import { MAX_EMBEDDING_INLINE_ITEM_BYTES } from "@/shared/validation/schemas/apiV1"; +import { markAccountUnavailable } from "../../src/sse/services/auth.ts"; interface ClientRawRequest { endpoint: string; @@ -389,6 +390,28 @@ export async function handleEmbedding({ connectionId, }).catch(() => {}); + // #10347 — persist a connection-level failure marker on a hard upstream failure so + // the dead account is not re-selected and re-hit on the next embed request (chat + // parity). markAccountUnavailable classifies the status via checkFallbackError: a + // payment-required 402 becomes the TERMINAL state credits_exhausted (the terminal + // marker excludes the account from selection until an operator resets it), benign + // 4xx are a no-op, and terminal statuses are never overwritten. honors per-connection + // disableCooling. The write must never break the error response path, so it is + // best-effort. + if (connectionId) { + try { + await markAccountUnavailable( + connectionId, + response.status, + errorText, + provider, + model + ); + } catch { + // swallow — the upstream error response takes priority + } + } + return { success: false, status: response.status, diff --git a/src/app/api/v1/providers/[provider]/embeddings/route.ts b/src/app/api/v1/providers/[provider]/embeddings/route.ts index 01dbe5bc84..bb8f242290 100644 --- a/src/app/api/v1/providers/[provider]/embeddings/route.ts +++ b/src/app/api/v1/providers/[provider]/embeddings/route.ts @@ -84,7 +84,14 @@ export async function POST(request, { params }) { ); } - const result = await handleEmbedding({ body, credentials, log }); + const result = await handleEmbedding({ + body, + credentials, + log, + // #10347 — thread the selected connection id so a hard upstream failure cools + // the account instead of re-hitting it on every request. + connectionId: (credentials as { connectionId?: string } | null)?.connectionId ?? null, + }); if (result.success) { await clearRecoveredProviderState(credentials); diff --git a/src/lib/embeddings/service.ts b/src/lib/embeddings/service.ts index 5845cb773f..a615958b18 100644 --- a/src/lib/embeddings/service.ts +++ b/src/lib/embeddings/service.ts @@ -302,7 +302,12 @@ export async function createEmbeddingResponse( clientRawRequest: options.clientRawRequest || null, apiKeyId: options.apiKeyId || null, apiKeyName: options.apiKeyName || null, - connectionId: options.connectionId || null, + // #10347 — thread the selected connection id so handleEmbedding can cool the + // account on a hard upstream failure (previously always null on /v1/embeddings). + connectionId: + ((credentials as { connectionId?: string } | null)?.connectionId) || + options.connectionId || + null, }); const result = connectionIdForProxy diff --git a/tests/unit/10347-embed-402-cooldown.test.ts b/tests/unit/10347-embed-402-cooldown.test.ts new file mode 100644 index 0000000000..ddb98259ba --- /dev/null +++ b/tests/unit/10347-embed-402-cooldown.test.ts @@ -0,0 +1,103 @@ +/** + * TDD regression (#10347): the embed path reads a connection's cooldown at + * selection time but NEVER writes one on a terminal upstream failure. A Mistral + * (or any) connection returning HTTP 402 "payment required — Check your + * subscription" on embeds is re-selected and re-hit upstream on every request — + * the repeated EMBED/ERROR/ProxyEgress storm on 3.8.49. Chat wires the cooldown + * write (`markAccountUnavailable`) on hard failures; embed never does. + * + * Repro: create a real mistral apikey connection, mock `globalThis.fetch` to + * return HTTP 402 with a payment-required JSON body, call `handleEmbedding` + * with that connectionId, then assert the connection's `rate_limited_until` + * becomes a future timestamp. Today it stays `undefined` (RED); with the fix + * `markAccountUnavailable` persists a 1h QUOTA_EXHAUSTED cooldown (GREEN). + */ +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-embed-402-")); +process.env.DATA_DIR = TEST_DATA_DIR; + +const core = await import("../../src/lib/db/core.ts"); +const providersDb = await import("../../src/lib/db/providers.ts"); +const { handleEmbedding } = await import("../../open-sse/handlers/embeddings.ts"); + +test.after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +function readConnectionRow(connId: string) { + const db = core.getDbInstance() as unknown as { + prepare: (sql: string) => { + get: (id: string) => { + test_status: unknown; + rate_limited_until: unknown; + last_error_type: unknown; + } | undefined; + }; + }; + return db + .prepare( + "SELECT test_status, rate_limited_until, last_error_type FROM provider_connections WHERE id = ?" + ) + .get(connId); +} + +test("embed 402 marks the connection terminal credits_exhausted (stops re-selection)", async () => { + const conn = await providersDb.createProviderConnection({ + provider: "mistral", + authType: "apikey", + name: "embed 402 cooldown", + }); + const connId = (conn as { id: string }).id; + + const originalFetch = globalThis.fetch; + globalThis.fetch = async () => + new Response( + JSON.stringify({ + code: "subscription_inactive", + message: "Check your subscription", + }), + { + status: 402, + headers: { "content-type": "application/json" }, + } + ); + + try { + const result = await handleEmbedding({ + body: { model: "mistral/mistral-embed", input: "ping" }, + credentials: { apiKey: "mistral-key" }, + connectionId: connId, + log: null, + }); + + // The upstream was hit and surfaced a 402 — the bug scope. + assert.equal(result.success, false); + assert.equal(result.status, 402); + + const row = readConnectionRow(connId); + // markAccountUnavailable classifies a payment-required 402 as the TERMINAL state + // credits_exhausted (last_error_type quota_exhausted) with no transient numeric + // cooldown — the terminal marker is what excludes the account from the embed + // selection path on the next request, stopping the repeat re-hit storm. + assert.equal( + row?.test_status, + "credits_exhausted", + `expected the 402 to mark the connection terminal (test_status=credits_exhausted) on ${connId}, got ${String( + row?.test_status + )}` + ); + assert.equal( + row?.last_error_type, + "quota_exhausted", + `expected last_error_type=quota_exhausted on ${connId}, got ${String(row?.last_error_type)}` + ); + } finally { + globalThis.fetch = originalFetch; + } +}); \ No newline at end of file