From 13e15e9cfe0ee085274f761de8304fcf255d948c Mon Sep 17 00:00:00 2001 From: backryun Date: Tue, 28 Jul 2026 05:24:09 +0900 Subject: [PATCH] refactor(sse): guard the KIE task-id and callback-url reads at their source (#8661) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `kieExecutor.createTask()` returns `JsonObject` (`Record`), so `createData.data` is `unknown` and the `createData?.data?.taskId` read that image, video and music generation each duplicated could not compile. The same three-line expression appeared verbatim in all three handlers. `open-sse/utils/kieTask.ts` already holds two helpers with exactly this shape — `normalizeKieTaskState()` and `parseKieResultJson()` both take `unknown`, guard with `isJsonObject()` and return a declared type. `getKieTaskId()` follows them, so the three handlers now share one guarded read instead of three unguarded ones. `getKieCallbackUrl()` took `KieCallbackBody`, a weak type (all properties optional). Passing a request body whose declared keys are `prompt` / `timeout_ms` / `poll_interval_ms` tripped TS2559 "no properties in common" at both music call sites. It receives arbitrary upstream request bodies, so it now takes `unknown` and guards the same way its neighbours do; `KieCallbackBody` had no other reference and is gone. Behaviour is unchanged. `isJsonObject()` rejects arrays and null exactly where optional chaining already yielded `undefined`, and the callers' `String(taskId)` coercion moved inside the helper, so a numeric id still reaches `pollTask()` as a string and a falsy id still takes the 502 branch. Fixes 5 of the 208 `tsc -p open-sse/tsconfig.json` diagnostics with no new ones: 3 x TS2339 `taskId` on `unknown`, 2 x TS2559 on `KieCallbackBody`. Refs #8484 --- open-sse/handlers/imageGeneration.ts | 3 +- open-sse/handlers/musicGeneration.ts | 9 +++- open-sse/handlers/videoGeneration.ts | 4 +- open-sse/utils/kieTask.ts | 18 ++++--- tests/unit/kie-task-helpers.test.ts | 73 ++++++++++++++++++++++++++++ 5 files changed, 94 insertions(+), 13 deletions(-) create mode 100644 tests/unit/kie-task-helpers.test.ts diff --git a/open-sse/handlers/imageGeneration.ts b/open-sse/handlers/imageGeneration.ts index 631f32ac45..c5d338c0bc 100644 --- a/open-sse/handlers/imageGeneration.ts +++ b/open-sse/handlers/imageGeneration.ts @@ -32,6 +32,7 @@ import { sleep } from "../utils/sleep.ts"; import { getKieErrorMessage, getKieErrorStatus, + getKieTaskId, isJsonObject, parseKieResultJson, } from "../utils/kieTask.ts"; @@ -758,7 +759,7 @@ async function handleKieImageGeneration({ payload, endpoint, }); - const taskId = createData?.data?.taskId || createData?.taskId; + const taskId = getKieTaskId(createData); if (!taskId) { const errorMessage = diff --git a/open-sse/handlers/musicGeneration.ts b/open-sse/handlers/musicGeneration.ts index 96ddbc2d3e..766abdd542 100644 --- a/open-sse/handlers/musicGeneration.ts +++ b/open-sse/handlers/musicGeneration.ts @@ -25,7 +25,12 @@ import { resolveComfyUiBaseUrl, } from "../utils/comfyuiClient.ts"; import { saveCallLog } from "@/lib/usageDb"; -import { getKieCallbackUrl, isJsonObject, parseKieResultJson } from "../utils/kieTask.ts"; +import { + getKieCallbackUrl, + getKieTaskId, + isJsonObject, + parseKieResultJson, +} from "../utils/kieTask.ts"; import { sanitizeErrorMessage } from "../utils/error.ts"; function normalizeKieSunoModel(model: string): string { @@ -341,7 +346,7 @@ async function handleKieMusicGeneration({ try { const endpoint = new URL(url).pathname; const createData = await kieExecutor.createTask({ baseUrl, token, payload, endpoint }); - const taskId = createData?.data?.taskId || createData?.taskId; + const taskId = getKieTaskId(createData); if (!taskId) { const errorMessage = createData?.msg || diff --git a/open-sse/handlers/videoGeneration.ts b/open-sse/handlers/videoGeneration.ts index 2df27942a1..29b0902269 100644 --- a/open-sse/handlers/videoGeneration.ts +++ b/open-sse/handlers/videoGeneration.ts @@ -19,7 +19,7 @@ import { handleXaiVideoGeneration } from "./videoGeneration/xaiGrokImagineHandle import { handleSegmindVideoGeneration } from "./videoGeneration/providers/segmind.ts"; import { handleAdobeFireflyVideoGeneration } from "./videoGeneration/adobeFireflyHandler.ts"; import { getExecutor } from "../executors/index.ts"; -import { isJsonObject, parseKieResultJson } from "../utils/kieTask.ts"; +import { getKieTaskId, isJsonObject, parseKieResultJson } from "../utils/kieTask.ts"; import { buildRunwayApiUrl, buildRunwayHeaders, @@ -540,7 +540,7 @@ async function handleKieVideoGeneration({ try { const createData = await kieExecutor.createTask({ baseUrl, token, payload }); - const taskId = createData?.data?.taskId || createData?.taskId; + const taskId = getKieTaskId(createData); if (!taskId) { const errorMessage = createData?.msg || diff --git a/open-sse/utils/kieTask.ts b/open-sse/utils/kieTask.ts index c3b7953d97..ef15eec1ce 100644 --- a/open-sse/utils/kieTask.ts +++ b/open-sse/utils/kieTask.ts @@ -2,12 +2,6 @@ export type JsonObject = Record; export type KieTaskState = "success" | "failed" | "pending"; -export type KieCallbackBody = { - callBackUrl?: unknown; - callback_url?: unknown; - callbackUrl?: unknown; -}; - const FALLBACK_KIE_CALLBACK_URL = "https://omniroute.local/api/kie/callback"; export function isJsonObject(value: unknown): value is JsonObject { @@ -42,8 +36,9 @@ function getConfiguredKieCallbackUrl(): string { ); } -export function getKieCallbackUrl(body: KieCallbackBody = {}): string { - const callbackUrl = body.callBackUrl ?? body.callback_url ?? body.callbackUrl; +export function getKieCallbackUrl(body: unknown = {}): string { + const record = isJsonObject(body) ? body : {}; + const callbackUrl = record.callBackUrl ?? record.callback_url ?? record.callbackUrl; return typeof callbackUrl === "string" && callbackUrl.trim().length > 0 ? callbackUrl : getConfiguredKieCallbackUrl(); @@ -65,6 +60,13 @@ export function parseKieResultJson(recordData: unknown): JsonObject { return isJsonObject(resultJson) ? resultJson : {}; } +export function getKieTaskId(createData: unknown): string | null { + const record = isJsonObject(createData) ? createData : {}; + const data = isJsonObject(record.data) ? record.data : {}; + const taskId = data.taskId || record.taskId; + return taskId ? String(taskId) : null; +} + export function normalizeKieTaskState(recordData: unknown): KieTaskState { const record = isJsonObject(recordData) ? recordData : {}; const data = isJsonObject(record.data) ? record.data : {}; diff --git a/tests/unit/kie-task-helpers.test.ts b/tests/unit/kie-task-helpers.test.ts new file mode 100644 index 0000000000..83a7c717f7 --- /dev/null +++ b/tests/unit/kie-task-helpers.test.ts @@ -0,0 +1,73 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { getKieCallbackUrl, getKieTaskId } from "../../open-sse/utils/kieTask.ts"; + +// getKieTaskId replaces the expression `createData?.data?.taskId || createData?.taskId` +// that image/video/music generation each duplicated. Every arm below is one the three +// handlers could hit, since `createTask()` returns whatever the KIE upstream sent. + +test("getKieTaskId reads the nested data.taskId first", () => { + assert.equal(getKieTaskId({ data: { taskId: "abc123" } }), "abc123"); +}); + +test("getKieTaskId falls back to a top-level taskId", () => { + assert.equal(getKieTaskId({ taskId: "top-level" }), "top-level"); +}); + +test("getKieTaskId prefers the nested id when both are present", () => { + assert.equal(getKieTaskId({ taskId: "top", data: { taskId: "nested" } }), "nested"); +}); + +test("getKieTaskId coerces a non-string id, as the pollTask callers did with String()", () => { + assert.equal(getKieTaskId({ data: { taskId: 42 } }), "42"); +}); + +test("getKieTaskId ignores a non-object data envelope and still checks the top level", () => { + // KIE has returned `data` as a string, an array and null on error responses. + assert.equal(getKieTaskId({ data: "not-an-object", taskId: "fallback" }), "fallback"); + assert.equal(getKieTaskId({ data: ["nope"], taskId: "fallback" }), "fallback"); + assert.equal(getKieTaskId({ data: null, taskId: "fallback" }), "fallback"); + assert.equal(getKieTaskId({ data: "not-an-object" }), null); +}); + +test("getKieTaskId returns null when no id is present, keeping the handlers' 502 branch", () => { + assert.equal(getKieTaskId({}), null); + assert.equal(getKieTaskId({ data: {} }), null); + assert.equal(getKieTaskId({ msg: "createTask failed" }), null); +}); + +test("getKieTaskId treats falsy ids as absent, matching the previous `!taskId` guard", () => { + assert.equal(getKieTaskId({ data: { taskId: "" } }), null); + assert.equal(getKieTaskId({ data: { taskId: 0 } }), null); +}); + +test("getKieTaskId tolerates a non-object response", () => { + assert.equal(getKieTaskId(null), null); + assert.equal(getKieTaskId(undefined), null); + assert.equal(getKieTaskId("boom"), null); +}); + +test("getKieCallbackUrl accepts all three casings the handlers forward", () => { + assert.equal(getKieCallbackUrl({ callBackUrl: "https://a.test/cb" }), "https://a.test/cb"); + assert.equal(getKieCallbackUrl({ callback_url: "https://b.test/cb" }), "https://b.test/cb"); + assert.equal(getKieCallbackUrl({ callbackUrl: "https://c.test/cb" }), "https://c.test/cb"); +}); + +test("getKieCallbackUrl falls back for blank, absent and non-object bodies", () => { + const previous = process.env.KIE_CALLBACK_URL; + process.env.KIE_CALLBACK_URL = "https://configured.test/api/kie/callback"; + try { + const configured = "https://configured.test/api/kie/callback"; + assert.equal(getKieCallbackUrl({ callBackUrl: " " }), configured); + assert.equal(getKieCallbackUrl({}), configured); + assert.equal(getKieCallbackUrl(), configured); + // The music handler passes an untyped request body straight through. + assert.equal(getKieCallbackUrl({ prompt: "a song" }), configured); + assert.equal(getKieCallbackUrl(null), configured); + assert.equal(getKieCallbackUrl("not-an-object"), configured); + } finally { + if (previous === undefined) delete process.env.KIE_CALLBACK_URL; + else process.env.KIE_CALLBACK_URL = previous; + } +});