From a123bd049e64848e27a4a6e2580ce56cebf49a08 Mon Sep 17 00:00:00 2001 From: Tiangao <53409436+tiangao88@users.noreply.github.com> Date: Sat, 19 Sep 2026 05:03:11 +0200 Subject: [PATCH] fix(images): fall back when combo leg returns empty 2xx response (#12982) * fix(images): fall back when combo leg returns empty 2xx response fetchImageEndpoint() normalized any successful HTTP response to success:true with data.data || [], so an OpenAI-compatible provider returning 200 with an empty/malformed image payload stopped image combos on the first leg and produced an image-less 200. Require at least one usable item (non-empty b64_json or url) before declaring success; empty 2xx becomes a retryable 502 with a sanitized error so executeImageCombo() advances to the next priority leg. Valid responses and direct image-model requests are unchanged. * chore(changelog): add fragment for image-combo empty-2xx fallback (#12982) * test(images): drop misleading #10199 reference from empty-200 fallback test The test file and its production-code comment referenced issue #10199, which belongs to an unrelated, already-merged PR (auto/best-free free-tier filter fix). Rename the test file and update comments to remove the incorrect reference so future readers are not misled. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> * test(images): fix response-shape assertions after #12268 combo envelope change release/v3.8.51 already ships #12268, which changed executeImageCombo() to return the handler's payload unchanged ({created, data: [...]}) instead of unwrapping it into a bare array. Update the two assertions that still expected a bare array so the test reflects the combo response shape that is actually live on the target branch; the fallback behavior under test is unaffected. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --------- Co-authored-by: Tiangao (hermes) Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --- .../12982-image-combo-empty-2xx-fallback.md | 1 + open-sse/handlers/imageGeneration.ts | 32 +- .../image-combo-empty-200-fallback.test.ts | 328 ++++++++++++++++++ 3 files changed, 360 insertions(+), 1 deletion(-) create mode 100644 changelog.d/fixes/12982-image-combo-empty-2xx-fallback.md create mode 100644 tests/unit/combo/image-combo-empty-200-fallback.test.ts diff --git a/changelog.d/fixes/12982-image-combo-empty-2xx-fallback.md b/changelog.d/fixes/12982-image-combo-empty-2xx-fallback.md new file mode 100644 index 0000000000..674a1cfc70 --- /dev/null +++ b/changelog.d/fixes/12982-image-combo-empty-2xx-fallback.md @@ -0,0 +1 @@ +- **fix(images):** image-combo legs now fall back when an upstream provider returns HTTP 2xx with an empty or malformed image payload. `fetchImageEndpoint` previously normalized any successful HTTP response to `success: true` (`data.data || []`), so `executeImageCombo` stopped on the first leg and handed the client an image-less 200. The OpenAI-compatible normalization now requires at least one usable item (non-empty `b64_json` or `url`) in `data[]`; an empty/malformed 2xx becomes a retryable 502 with a sanitized error, so priority image combos advance to the next leg. Valid payloads and direct image-model requests are unchanged. [#12982](https://github.com/diegosouzapw/OmniRoute/pull/12982) diff --git a/open-sse/handlers/imageGeneration.ts b/open-sse/handlers/imageGeneration.ts index bd6f1b68aa..7447fb125e 100644 --- a/open-sse/handlers/imageGeneration.ts +++ b/open-sse/handlers/imageGeneration.ts @@ -2912,11 +2912,41 @@ async function fetchImageEndpoint(url, headers, body, provider, log) { const data = await response.json(); // Normalize response to OpenAI format + const items = Array.isArray(data?.data) ? data.data : []; + + // Some providers return HTTP 2xx with an empty or malformed image + // payload (empty data array, missing/blank b64_json and url). Treating that + // as success makes image-combo strategies stop on the first leg and hand an + // image-less 200 to the client. Require at least one usable image item and + // surface an empty 2xx as a retryable 502 so combos fall back to the next + // priority leg. + const hasUsableImage = items.some( + (item: unknown) => + isJsonObject(item) && + ((typeof item.b64_json === "string" && item.b64_json.length > 0) || + (typeof item.url === "string" && item.url.length > 0)) + ); + if (!hasUsableImage) { + if (log) { + log.warn( + "IMAGE", + `${provider} returned 200 without a usable image payload; treating as retryable 502` + ); + } + return { + success: false, + status: HTTP_STATUS.BAD_GATEWAY, + error: sanitizeErrorMessage( + "Image provider returned a success status without an image payload" + ), + }; + } + return { success: true, data: { created: data.created || Math.floor(Date.now() / 1000), - data: data.data || [], + data: items, }, }; } catch (err: unknown) { diff --git a/tests/unit/combo/image-combo-empty-200-fallback.test.ts b/tests/unit/combo/image-combo-empty-200-fallback.test.ts new file mode 100644 index 0000000000..bed58ae22e --- /dev/null +++ b/tests/unit/combo/image-combo-empty-200-fallback.test.ts @@ -0,0 +1,328 @@ +/** + * Image combo fallback on empty 2xx upstream responses + * + * Repro: an OpenAI-compatible image provider (e.g. openrouter/*) can return + * HTTP 200 with an empty or malformed image payload (no usable b64_json/url in + * data[]). fetchImageEndpoint() used to normalize that to success:true, so + * executeImageCombo() stopped on the first leg and the client received an + * image-less 200. Hermes then rejected the response. + * + * Fix: require at least one usable image item before declaring success; an + * empty 2xx becomes a retryable 502 so the combo advances to the next leg. + * + * Strategy: real isolated SQLite DATA_DIR + real combo resolution + real + * credentials path (seeded apikey connection) + stubbed globalThis.fetch. + * No paid requests, no module mocking (tsx loader cannot mock ESM exports). + * + * Run: node --import tsx/esm --import ./open-sse/utils/setupPolyfill.ts + * --import ./tests/_setup/isolateDataDir.ts --test + * tests/unit/combo/image-combo-empty-200-fallback.test.ts + */ +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-image-combo-empty-200-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.JWT_SECRET = "test-jwt-secret-for-image-combo-empty-200-tests"; +process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "image-combo-empty-200-test-secret"; + +const core = await import("@/lib/db/core.ts"); +const providersDb = await import("@/lib/db/providers.ts"); +const { createCombo } = await import("@/lib/db/combos"); +const { executeImageCombo } = await import("@omniroute/open-sse/services/imageCombo"); + +const PNG_B64 = Buffer.from([0x89, 0x50, 0x4e, 0x47]).toString("base64"); + +const originalFetch = globalThis.fetch; + +type LogEntry = { level: string; tag: unknown; msg: unknown }; + +function createLog() { + const entries: LogEntry[] = []; + const record = + (level: string) => + (tag: unknown, msg: unknown): number => + entries.push({ level, tag, msg }); + return { + info: record("info"), + warn: record("warn"), + error: record("error"), + debug: record("debug"), + entries, + }; +} + +function createMockAuth() { + return { + request: new Request("http://localhost:20128/v1/images/generations", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ model: "empty-200-combo", prompt: "a cat" }), + }), + policy: { apiKeyInfo: { id: "test-key", name: "test-key" } }, + }; +} + +async function resetStorage() { + globalThis.fetch = originalFetch; + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); +} + +/** Seed an active openrouter apikey connection so credential resolution succeeds. */ +async function seedOpenRouterConnection() { + return providersDb.createProviderConnection({ + provider: "openrouter", + authType: "apikey", + name: "openrouter-empty-200-test", + apiKey: "sk-or-test-not-a-real-key", + isActive: true, + testStatus: "active", + rateLimitedUntil: null, + }); +} + +/** + * Seed the pmoc-image-text style two-leg combo: first leg returns an empty + * 200 (stubbed upstream), second leg returns a valid image. + */ +async function seedTwoLegCombo(name: string) { + return createCombo({ + name, + strategy: "priority", + models: ["openrouter/openai/gpt-5-image-mini", "openrouter/openai/gpt-5.4-image-2"], + }); +} + +/** Stub fetch: first call → empty 200, subsequent calls → valid image 200. */ +function stubFetchEmptyThenValid(hitLog: Array<{ url: string; model?: string }>) { + let callIndex = 0; + globalThis.fetch = (async (url: unknown, init?: RequestInit) => { + const index = callIndex++; + const bodyText = typeof init?.body === "string" ? init.body : String(init?.body ?? ""); + let model: string | undefined; + try { + model = (JSON.parse(bodyText) as { model?: string }).model; + } catch { + model = undefined; + } + hitLog.push({ url: String(url), model }); + if (index === 0) { + return new Response(JSON.stringify({ created: Math.floor(Date.now() / 1000), data: [] }), { + status: 200, + headers: { "content-type": "application/json" }, + }); + } + return new Response( + JSON.stringify({ created: Math.floor(Date.now() / 1000), data: [{ b64_json: PNG_B64 }] }), + { status: 200, headers: { "content-type": "application/json" } } + ); + }) as typeof fetch; +} + +/** Stub fetch: every call → empty 200 (all legs unusable). */ +function stubFetchAlwaysEmpty() { + globalThis.fetch = (async () => + new Response(JSON.stringify({ created: Math.floor(Date.now() / 1000), data: [] }), { + status: 200, + headers: { "content-type": "application/json" }, + })) as typeof fetch; +} + +/** Stub fetch: every call → single-leg valid image (direct-model baseline). */ +function stubFetchAlwaysValid(hitLog?: Array<{ url: string; model?: string }>) { + globalThis.fetch = (async (url: unknown, init?: RequestInit) => { + if (hitLog) { + let model: string | undefined; + try { + model = ( + JSON.parse(typeof init?.body === "string" ? init.body : String(init?.body ?? "")) as { + model?: string; + } + ).model; + } catch { + model = undefined; + } + hitLog.push({ url: String(url), model }); + } + return new Response( + JSON.stringify({ created: Math.floor(Date.now() / 1000), data: [{ b64_json: PNG_B64 }] }), + { status: 200, headers: { "content-type": "application/json" } } + ); + }) as typeof fetch; +} + +test("empty 200 from first leg falls back to second leg and second leg image is served", async () => { + await resetStorage(); + await seedOpenRouterConnection(); + await seedTwoLegCombo("empty-200-combo"); + + const hits: Array<{ url: string; model?: string }> = []; + stubFetchEmptyThenValid(hits); + + const log = createLog(); + const response = await executeImageCombo( + "empty-200-combo", + { model: "empty-200-combo", prompt: "a cat", n: 1 }, + createMockAuth(), + Date.now(), + log + ); + + assert.equal(response.status, 200, "combo must ultimately succeed via leg 2"); + const body = (await response.json()) as { data?: Array<{ b64_json?: string }> }; + assert.ok(Array.isArray(body.data), "response body must carry the image items array"); + assert.equal(body.data?.length, 1, "exactly one image (from the second leg)"); + assert.equal(body.data?.[0]?.b64_json, PNG_B64, "served image must come from leg 2"); + + // Both legs were tried: first the empty-200 stub, then the valid stub. + assert.equal(hits.length, 2, "combo must advance to the second leg"); + assert.equal(hits[0].model, "openai/gpt-5-image-mini", "leg 1 model hit first"); + assert.equal(hits[1].model, "openai/gpt-5.4-image-2", "leg 2 model hit second"); + + const warnJoined = log.entries + .filter((e) => e.level === "warn") + .map((e) => String(e.msg)) + .join(" "); + assert.ok( + warnJoined.includes("without a usable image payload"), + "leg-1 empty 200 must be logged as unusable payload" + ); +}); + +test("fallback metadata reflects the additional attempt via X-OmniRoute-Fallback-Attempts", async () => { + await resetStorage(); + await seedOpenRouterConnection(); + await seedTwoLegCombo("empty-200-fallback-meta-combo"); + + const hits: Array<{ url: string; model?: string }> = []; + stubFetchEmptyThenValid(hits); + + const response = await executeImageCombo( + "empty-200-fallback-meta-combo", + { model: "empty-200-fallback-meta-combo", prompt: "a cat", n: 1 }, + createMockAuth(), + Date.now(), + createLog() + ); + + assert.equal(response.status, 200); + const attempts = response.headers.get("X-OmniRoute-Fallback-Attempts"); + assert.ok(attempts !== null, "fallback attempts header must be present"); + assert.equal(attempts, "1", "one leg failed over, so fallback attempts must be 1"); +}); + +test("valid first-leg response does not invoke later legs", async () => { + await resetStorage(); + await seedOpenRouterConnection(); + await seedTwoLegCombo("empty-200-valid-first-combo"); + + const hits: Array<{ url: string; model?: string }> = []; + stubFetchAlwaysValid(hits); + + const response = await executeImageCombo( + "empty-200-valid-first-combo", + { model: "empty-200-valid-first-combo", prompt: "a cat", n: 1 }, + createMockAuth(), + Date.now(), + createLog() + ); + + assert.equal(response.status, 200); + const body = (await response.json()) as { data?: Array<{ b64_json?: string }> }; + assert.equal(body.data?.[0]?.b64_json, PNG_B64); + assert.equal(hits.length, 1, "first leg success must stop the combo (no later legs hit)"); + assert.equal(hits[0].model, "openai/gpt-5-image-mini"); +}); + +test("all legs returning empty 200 yields a retryable 502 with sanitized error", async () => { + await resetStorage(); + await seedOpenRouterConnection(); + await createCombo({ + name: "empty-200-all-legs-combo", + strategy: "priority", + models: ["openrouter/openai/gpt-5-image-mini", "openrouter/openai/gpt-5.4-image-2"], + }); + + stubFetchAlwaysEmpty(); + + const response = await executeImageCombo( + "empty-200-all-legs-combo", + { model: "empty-200-all-legs-combo", prompt: "a cat", n: 1 }, + createMockAuth(), + Date.now(), + createLog() + ); + + assert.equal(response.status, 502, "exhausted combo must surface the retryable 502"); + const bodyStr = JSON.stringify(await response.json()); + assert.ok( + bodyStr.includes("image payload") || bodyStr.includes("Image provider"), + "error must describe the unusable payload" + ); + assert.ok(!bodyStr.includes("sk-or-test"), "error must not leak credentials"); + assert.ok(!bodyStr.includes("at "), "error must not leak stack traces"); +}); + +test("direct image model request with empty 200 is a retryable 502 (behavior preserved for valid payloads)", async () => { + await resetStorage(); + await seedOpenRouterConnection(); + + const { handleImageGeneration } = await import("@omniroute/open-sse/handlers/imageGeneration"); + + // Empty 200 → retryable 502 (previously a bogus success) + stubFetchAlwaysEmpty(); + const emptyResult = (await handleImageGeneration({ + body: { model: "openrouter/openai/gpt-5-image-mini", prompt: "a cat", n: 1 }, + credentials: { apiKey: "sk-or-test-not-a-real-key" }, + log: createLog(), + })) as { success: boolean; status?: number; error?: string }; + assert.equal(emptyResult.success, false); + assert.equal(emptyResult.status, 502); + assert.ok( + typeof emptyResult.error === "string" && !emptyResult.error.includes("sk-or-test"), + "sanitized error must not include credentials" + ); + + // Valid 200 → success preserved + stubFetchAlwaysValid(); + const validResult = (await handleImageGeneration({ + body: { model: "openrouter/openai/gpt-5-image-mini", prompt: "a cat", n: 1 }, + credentials: { apiKey: "sk-or-test-not-a-real-key" }, + log: createLog(), + })) as { success: boolean; data?: { data?: Array<{ b64_json?: string }> } }; + assert.equal(validResult.success, true, "valid payload must still succeed"); + assert.equal(validResult.data?.data?.[0]?.b64_json, PNG_B64); + + // url-style payload → success preserved + globalThis.fetch = (async () => + new Response(JSON.stringify({ created: 1, data: [{ url: "https://example.test/img.png" }] }), { + status: 200, + headers: { "content-type": "application/json" }, + })) as typeof fetch; + const urlResult = (await handleImageGeneration({ + body: { model: "openrouter/openai/gpt-5-image-mini", prompt: "a cat", n: 1 }, + credentials: { apiKey: "sk-or-test-not-a-real-key" }, + log: createLog(), + })) as { success: boolean; data?: { data?: Array<{ url?: string }> } }; + assert.equal(urlResult.success, true, "url-bearing payload must still succeed"); + assert.equal(urlResult.data?.data?.[0]?.url, "https://example.test/img.png"); + + // Malformed: 200 with non-array data → retryable 502 + globalThis.fetch = (async () => + new Response(JSON.stringify({ created: 1, data: "not-an-array" }), { + status: 200, + headers: { "content-type": "application/json" }, + })) as typeof fetch; + const malformed = (await handleImageGeneration({ + body: { model: "openrouter/openai/gpt-5-image-mini", prompt: "a cat", n: 1 }, + credentials: { apiKey: "sk-or-test-not-a-real-key" }, + log: createLog(), + })) as { success: boolean; status?: number }; + assert.equal(malformed.success, false); + assert.equal(malformed.status, 502); +});