From f91911e087a47b4916eb18e0f42cd976dabc6bf6 Mon Sep 17 00:00:00 2001 From: Dizzle <112548150+maxmad64bis@users.noreply.github.com> Date: Fri, 2 Oct 2026 17:30:33 +0200 Subject: [PATCH] fix(api): log the auto-combo variants GET /api/combos/auto skips instead of failing silently Silent per-variant catch blocks left skipped variants untraced. The route now logs one line naming the variants that stayed out of the list, and a variant that fails in one loop but builds in a later one is still listed. Covered by injected-failure tests that run in CI. --- .../15362-combos-auto-log-skipped-variants.md | 1 + src/app/api/combos/auto/route.ts | 80 ++++---- .../unit/combos-auto-skipped-variants.test.ts | 193 ++++++++++++++++++ 3 files changed, 238 insertions(+), 36 deletions(-) create mode 100644 changelog.d/fixes/15362-combos-auto-log-skipped-variants.md create mode 100644 tests/unit/combos-auto-skipped-variants.test.ts diff --git a/changelog.d/fixes/15362-combos-auto-log-skipped-variants.md b/changelog.d/fixes/15362-combos-auto-log-skipped-variants.md new file mode 100644 index 00000000000..2b65cd51035 --- /dev/null +++ b/changelog.d/fixes/15362-combos-auto-log-skipped-variants.md @@ -0,0 +1 @@ +- **fix(api):** log the auto-combo variants GET /api/combos/auto skips instead of failing silently ([#15362](https://github.com/diegosouzapw/OmniRoute/pull/15362)) — thanks @maxmad64bis diff --git a/src/app/api/combos/auto/route.ts b/src/app/api/combos/auto/route.ts index ee25f6f0e87..1d1cf4ba551 100644 --- a/src/app/api/combos/auto/route.ts +++ b/src/app/api/combos/auto/route.ts @@ -33,14 +33,24 @@ export async function GET(request: Request) { // which made this route rebuild the whole pool once per listed variant. const prepared = await prepareVirtualAutoComboInputs(); - const combos = []; + const combos: Array> = []; const seenIds = new Set(); - for (const { variant, name } of ALL_VARIANTS) { + const skipped: Array<{ id: string; reason: string }> = []; + const pushVariant = async (id: string, build: () => Promise<(typeof combos)[number]>) => { + if (seenIds.has(id)) return; try { - const virtual = await createVirtualAutoComboFromPrepared(prepared, variant); - const id = variant ? `auto/${variant}` : "auto"; + combos.push(await build()); seenIds.add(id); - combos.push({ + } catch (error) { + const reason = error instanceof Error ? error.message : String(error); + skipped.push({ id, reason: reason.replace(/[\r\n\t]+/g, " ").trim() }); + } + }; + for (const { variant, name } of ALL_VARIANTS) { + const id = variant ? `auto/${variant}` : "auto"; + await pushVariant(id, async () => { + const virtual = await createVirtualAutoComboFromPrepared(prepared, variant); + return { id, name, variant: variant ?? null, @@ -57,10 +67,8 @@ export async function GET(request: Request) { context_length: virtual.advertisedContextLength || 128000, max_output_tokens: virtual.advertisedMaxOutputTokens || 8192, config: virtual.config ?? {}, - }); - } catch { - // Individual variant failure — skip, don't break the whole list - } + }; + }); } // Phase B: enumerate template variants (auto/best-coding, auto/pro-*, @@ -70,16 +78,16 @@ export async function GET(request: Request) { // ids (auto/reasoning, auto/vision), matching catalog.ts behavior. for (const modelStr of Object.keys(AUTO_TEMPLATE_VARIANTS)) { if (seenIds.has(modelStr)) continue; - try { - const variant = AUTO_TEMPLATE_VARIANTS[modelStr]; - const spec = modelStr === "auto/best-free" ? { tier: "free" as const } : undefined; + const variant = AUTO_TEMPLATE_VARIANTS[modelStr]; + const spec = modelStr === "auto/best-free" ? { tier: "free" as const } : undefined; + await pushVariant(modelStr, async () => { const virtual = await createVirtualAutoComboFromPrepared(prepared, variant, spec); const displayName = variant ? `Auto ${variant.charAt(0).toUpperCase() + variant.slice(1)}` : "Auto Chat"; - combos.push({ + return { id: modelStr, name: displayName, variant: null, @@ -94,11 +102,8 @@ export async function GET(request: Request) { context_length: virtual.advertisedContextLength || 128000, max_output_tokens: virtual.advertisedMaxOutputTokens || 8192, config: virtual.config ?? {}, - }); - seenIds.add(modelStr); - } catch { - // Individual variant failure — skip, don't break the whole list - } + }; + }); } // Phase C: enumerate tiered `auto/[:]` variants @@ -107,11 +112,11 @@ export async function GET(request: Request) { // exposed by this endpoint. for (const modelStr of AUTO_SUFFIX_VARIANTS) { if (seenIds.has(modelStr)) continue; - try { - const suffix = modelStr.slice("auto/".length); - const parsed = parseAutoSuffix(suffix); - if (!parsed.valid) continue; + const suffix = modelStr.slice("auto/".length); + const parsed = parseAutoSuffix(suffix); + if (!parsed.valid) continue; + await pushVariant(modelStr, async () => { const virtual = await createVirtualAutoComboFromPrepared(prepared, undefined, { category: parsed.category, tier: parsed.tier, @@ -126,7 +131,7 @@ export async function GET(request: Request) { : ""; const displayName = tierName ? `${catName} ${tierName}` : catName; - combos.push({ + return { id: modelStr, name: `Auto ${displayName}`, variant: null, @@ -141,11 +146,8 @@ export async function GET(request: Request) { context_length: virtual.advertisedContextLength || 128000, max_output_tokens: virtual.advertisedMaxOutputTokens || 8192, config: virtual.config ?? {}, - }); - seenIds.add(modelStr); - } catch { - // Individual variant failure — skip, don't break the whole list - } + }; + }); } // Phase D: enumerate family variants (auto/glm, auto/llama, @@ -153,15 +155,15 @@ export async function GET(request: Request) { // but were not exposed by this endpoint. for (const modelStr of AUTO_FAMILY_IDS) { if (seenIds.has(modelStr)) continue; - try { - const suffix = modelStr.slice("auto/".length); + const suffix = modelStr.slice("auto/".length); + await pushVariant(modelStr, async () => { const virtual = await createVirtualAutoComboFromPrepared(prepared, undefined, { family: suffix, }); const displayName = `Auto ${suffix.charAt(0).toUpperCase() + suffix.slice(1)}`; - combos.push({ + return { id: modelStr, name: displayName, variant: null, @@ -176,11 +178,17 @@ export async function GET(request: Request) { context_length: virtual.advertisedContextLength || 128000, max_output_tokens: virtual.advertisedMaxOutputTokens || 8192, config: virtual.config ?? {}, - }); - seenIds.add(modelStr); - } catch { - // Individual variant failure — skip, don't break the whole list - } + }; + }); + } + + // An id that failed in one loop but built in a later one is not skipped. + const missing = skipped.filter((s) => !seenIds.has(s.id)); + if (missing.length > 0) { + const ids = [...new Set(missing.map((s) => s.id))].join(", "); + console.warn( + `auto combo variants skipped: ${ids} (first error: ${missing[0].reason.slice(0, 300)})` + ); } return NextResponse.json({ combos }); diff --git a/tests/unit/combos-auto-skipped-variants.test.ts b/tests/unit/combos-auto-skipped-variants.test.ts new file mode 100644 index 00000000000..11b4e05bd49 --- /dev/null +++ b/tests/unit/combos-auto-skipped-variants.test.ts @@ -0,0 +1,193 @@ +/** + * GET /api/combos/auto logs the variants it skips instead of failing silently. + * + * Each of the four variant families (named, template, suffix, family) builds + * every entry from the same candidate pool. When one build fails the route + * leaves that variant out of the list but must say so: a single warning line + * per request names the skipped variants and the first error. With no failure + * the route stays quiet and the payload matches the unfixed baseline. + * + * Failure injection poisons one scoring weight pack through the shared + * modePacks module (plain mutable export, no module mock needed): every + * variant resolves its weights from a pack synchronously, so a throwing pack + * makes exactly the builds that use it fail while the rest succeed. The + * offline pack only feeds the named variant, the reliability pack only the + * tiered suffix variant, and the cost pack a template-only id. + */ +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-skipped-variants-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.API_KEY_SECRET = process.env.API_KEY_SECRET ?? "skipped-variants-test-secret"; + +const core = await import("../../src/lib/db/core.ts"); +const settingsDb = await import("../../src/lib/db/settings.ts"); +const modePacks = await import("../../open-sse/services/autoCombo/modePacks.ts"); +const combosAutoRoute = await import("../../src/app/api/combos/auto/route.ts"); + +const PACKS = modePacks.MODE_PACKS as Record; +const SAVED_PACKS: Record = {}; +for (const key of Object.keys(PACKS)) SAVED_PACKS[key] = PACKS[key]; + +function poisonPack(pack: string, message: string): void { + Object.defineProperty(PACKS, pack, { + configurable: true, + enumerable: true, + get() { + throw new Error(message); + }, + }); +} + +function restorePacks(): void { + for (const key of Object.keys(SAVED_PACKS)) { + Object.defineProperty(PACKS, key, { + configurable: true, + enumerable: true, + value: SAVED_PACKS[key], + writable: true, + }); + } +} + +function watchWarnings(): { lines: string[]; stop: () => void } { + const lines: string[] = []; + const original = console.warn; + console.warn = (...args: unknown[]) => { + lines.push(args.map(String).join(" ")); + }; + return { + lines, + stop: () => { + console.warn = original; + }, + }; +} + +function skippedLines(lines: string[]): string[] { + return lines.filter((line) => line.includes("auto combo variants skipped")); +} + +async function callRoute(): Promise<{ status: number; combos: Array<{ id: string }> }> { + const res = await combosAutoRoute.GET(new Request("http://localhost/api/combos/auto")); + return { status: res.status, combos: (await res.json()).combos }; +} + +test.after(() => { + restorePacks(); + core.resetDbInstance(); + try { + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } catch { + // best-effort cleanup + } +}); + +test("stays quiet when every variant builds", async () => { + await settingsDb.updateSettings({ requireLogin: false }); + restorePacks(); + + const watcher = watchWarnings(); + try { + const { status, combos } = await callRoute(); + assert.equal(status, 200); + assert.ok(combos.length > 1); + assert.equal(skippedLines(watcher.lines).length, 0); + } finally { + watcher.stop(); + } +}); + +async function expectSkipped(pack: string, ids: string[], notIds: string[] = []): Promise { + await settingsDb.updateSettings({ requireLogin: false }); + restorePacks(); + const { combos: baseline } = await callRoute(); + for (const id of ids) assert.ok(baseline.map((combo) => combo.id).includes(id)); + + poisonPack(pack, `injected failure for ${pack}`); + const watcher = watchWarnings(); + try { + const { status, combos } = await callRoute(); + const present = combos.map((combo) => combo.id); + assert.equal(status, 200); + for (const id of ids) assert.ok(!present.includes(id)); + for (const id of notIds) assert.ok(present.includes(id)); + const lines = skippedLines(watcher.lines); + assert.equal(lines.length, 1); + for (const id of ids) assert.equal(lines[0].split(id).length - 1, 1, `${id} named once`); + assert.ok(lines[0].includes(`injected failure for ${pack}`)); + } finally { + watcher.stop(); + restorePacks(); + } +} + +test("logs one line naming the skipped named variant", async () => { + // auto/offline is built by both the named and the template loop: still one mention. + await expectSkipped("offline-friendly", ["auto/offline"], ["auto/fast"]); +}); + +test("logs one line naming the skipped template variant", async () => { + await expectSkipped("cost-saver", ["auto/best-free"], ["auto/fast"]); +}); + +test("logs one line naming the skipped tiered suffix variant", async () => { + await expectSkipped("reliability-first", ["auto/coding:reliable"], ["auto/coding:fast"]); +}); + +test("a variant that fails once and builds in a later loop is kept and not reported", async () => { + await settingsDb.updateSettings({ requireLogin: false }); + restorePacks(); + let calls = 0; + Object.defineProperty(PACKS, "offline-friendly", { + configurable: true, + enumerable: true, + get() { + calls += 1; + if (calls === 1) throw new Error("transient failure"); + return SAVED_PACKS["offline-friendly"]; + }, + }); + const watcher = watchWarnings(); + try { + const { combos } = await callRoute(); + assert.ok(combos.map((combo) => combo.id).includes("auto/offline")); + assert.equal(skippedLines(watcher.lines).length, 0); + } finally { + watcher.stop(); + restorePacks(); + } +}); + +test("keeps the payload identical to the unfixed baseline", async () => { + await settingsDb.updateSettings({ requireLogin: false }); + restorePacks(); + const { combos } = await callRoute(); + const ids = combos.map((combo) => combo.id); + assert.equal(new Set(ids).size, ids.length); + assert.ok(ids.includes("auto/coding")); + assert.ok(ids[0] === "auto"); +}); + +test("logs the first error on a single bounded line", async () => { + await settingsDb.updateSettings({ requireLogin: false }); + restorePacks(); + poisonPack("offline-friendly", `first\nsecond\tline ${"x".repeat(1000)}`); + const watcher = watchWarnings(); + try { + const { status } = await callRoute(); + assert.equal(status, 200); + const lines = skippedLines(watcher.lines); + assert.equal(lines.length, 1); + assert.ok(!/[\r\n\t]/.test(lines[0])); + assert.ok(lines[0].includes("first second line")); + assert.ok(lines[0].length < 450); + } finally { + watcher.stop(); + restorePacks(); + } +});