From 76078e24aab8d7820fd026445311c98cc2e83a72 Mon Sep 17 00:00:00 2001 From: David Witwer Date: Tue, 1 Sep 2026 19:04:01 -0700 Subject: [PATCH 1/4] fix(tools): tell a failed async media generation from a slow one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A caller who launched an async image or video generation could not tell a failed job from a slow one, on either of the two paths that report one. `wait()` treated EVERY non-OK poll response as "still generating", so a job that terminally failed in three seconds burned the caller's whole `timeoutMs` and then threw `Image generation did not complete within 300000ms` — which tells the caller the opposite of what happened. It now reads the queue's terminal state and throws the new `ContentGenerationFailedError` as soon as the job fails, carrying `requestId` and the provider's own `providerError`. A plain timeout `Error` now means only what it says: still running when we stopped waiting. The poll loop is shared between image and video (`poll.ts`) so their terminal semantics cannot drift apart, and `video.create` — which polls the same way — gets the same behavior. A transport blip must still keep polling, so a non-OK result poll (ambiguous on its own: failed, not-done-yet, and blipped all look alike) is disambiguated against the status endpoint, the canonical terminal channel. Anything short of an explicit terminal marker keeps the poll going. `terminalFailureFrom` mirrors the gateway's own `terminalStatusFromQueueResponse` so the two agree. `ImageResultPayload` / `VideoResultPayload` outputs gain `generationError`. The provider error already reached the resume payload, but on `storageError` — the field documented as "persisting this output failed" — so a resumed step concluded storage broke when nothing had been generated at all. The two are now separate fields, never both set on one output, so a step branches without string-matching a message. This is also what makes `VIDEO_RESULT_SIGNAL`'s documented "carries the result either way (ready OR failed)" claim true; the payload previously had nowhere to put the failure. Depends on sapiom/Sapiom#4801, which emits `generationError` on the wire. SAP-3097 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015XHWBRT86s84urfsTY9JJe --- .changeset/tidy-donkeys-report.md | 15 + .../content-repurposing-pipeline/AGENTS.md | 2 +- .../content-repurposing-pipeline/index.ts | 8 +- examples/research-to-microsite/index.ts | 12 +- examples/scene-to-video/AGENTS.md | 2 +- examples/scene-to-video/index.ts | 14 +- .../tools/src/content-generation/README.md | 29 +- .../tools/src/content-generation/errors.ts | 46 ++++ .../src/content-generation/index.spec.ts | 257 ++++++++++++++++++ .../tools/src/content-generation/index.ts | 203 +++++++------- packages/tools/src/content-generation/poll.ts | 218 +++++++++++++++ packages/tools/src/index.ts | 3 + 12 files changed, 705 insertions(+), 104 deletions(-) create mode 100644 .changeset/tidy-donkeys-report.md create mode 100644 packages/tools/src/content-generation/poll.ts diff --git a/.changeset/tidy-donkeys-report.md b/.changeset/tidy-donkeys-report.md new file mode 100644 index 00000000..f8339cf6 --- /dev/null +++ b/.changeset/tidy-donkeys-report.md @@ -0,0 +1,15 @@ +--- +"@sapiom/tools": minor +--- + +`contentGeneration`: an async image or video job that terminally FAILS is now distinguishable from one that is merely slow (SAP-3097). + +**`wait()` fails fast.** Every non-OK poll response used to be treated as "still generating", so a job that failed in three seconds burned the caller's whole `timeoutMs` and then threw `Image generation did not complete within 300000ms` — the opposite of what happened. `wait()` (and `video.create`, which polls the same way) now reads the queue's terminal state and throws the new `ContentGenerationFailedError` as soon as the job fails, carrying `requestId` and the provider's own `providerError`. A plain `Error` about the timeout now means only what it says: the job was still running when you stopped waiting. + +A transport blip still keeps polling. A non-OK result poll is ambiguous on its own, so it is disambiguated against the status endpoint — the canonical terminal channel — and anything short of an explicit terminal marker keeps the poll going. + +**`generationError` on the resume payload.** `ImageResultPayload` and `VideoResultPayload` outputs gain `generationError?: string`. A terminal provider failure used to arrive on `storageError` — the field documented as "persisting this output failed" — so a resumed workflow step concluded storage broke when in fact nothing was ever generated. The two are now separate fields and never both apply to one output: branch on `generationError` for "the model failed", `storageError` for "the model succeeded but we couldn't keep the result". + +This also makes `VIDEO_RESULT_SIGNAL`'s documented "carries the result either way (ready OR failed)" contract true; the payload previously had nowhere to put the failure. `IMAGE_RESULT_SIGNAL` carries the same contract. + +`ContentGenerationFailedError` is exported from `@sapiom/tools`. diff --git a/examples/content-repurposing-pipeline/AGENTS.md b/examples/content-repurposing-pipeline/AGENTS.md index 1bed29c7..b27cc8cd 100644 --- a/examples/content-repurposing-pipeline/AGENTS.md +++ b/examples/content-repurposing-pipeline/AGENTS.md @@ -73,7 +73,7 @@ that wall no longer applies. suspends the step until the job's signal arrives. The step must also **declare** the edge: `pause: { signal: IMAGE_RESULT_SIGNAL, resumeStep }` (or `VIDEO_RESULT_SIGNAL`). The resumed step receives an `ImageResultPayload` / - `VideoResultPayload` (`{ outputs: [{ fileId?, downloadUrl? }] }`). + `VideoResultPayload` (`{ outputs: [{ fileId?, downloadUrl?, generationError? }] }` — the signal fires on either terminal outcome, so check `generationError` before treating a missing `fileId` as a storage problem). - **Structured output, never a hand parse.** `repurpose` sets `output: { name, schema }` on the `llm.run` spec — which appends that tool and forces `tool_choice` onto it — and reads the pack back with `ctx.sapiom.llm.structuredOf(res, name)`. There is no prose to diff --git a/examples/content-repurposing-pipeline/index.ts b/examples/content-repurposing-pipeline/index.ts index 8c6ccc3b..2af0137d 100644 --- a/examples/content-repurposing-pipeline/index.ts +++ b/examples/content-repurposing-pipeline/index.ts @@ -811,9 +811,13 @@ const collectGraphic = defineStep({ const img = result.outputs?.[0]; if (!img?.fileId && !img?.downloadUrl) { - const storageError = img?.storageError ? `: ${img.storageError}` : ""; + // `generationError` means the model itself failed (no asset was ever made); + // `storageError` means it was made but we couldn't keep it. Report whichever applies. + const reason = img?.generationError ?? img?.storageError; throw new Error( - `quote graphic generation completed without a usable output for quote ${index + 1}${storageError}`, + img?.generationError + ? `quote graphic generation failed for quote ${index + 1}: ${img.generationError}` + : `quote graphic generation completed without a usable output for quote ${index + 1}${reason ? `: ${reason}` : ""}`, ); } const graphic: Graphic = { diff --git a/examples/research-to-microsite/index.ts b/examples/research-to-microsite/index.ts index 82e51343..79945d2b 100644 --- a/examples/research-to-microsite/index.ts +++ b/examples/research-to-microsite/index.ts @@ -919,10 +919,18 @@ const collectIllustration = defineStep({ } else { // Best-effort, same as a failed launch: no usable image for this // section, never a failed run. - const storageError = img?.storageError ? `: ${img.storageError}` : ""; ctx.logger.warn( "illustration generation returned no usable output; continuing without it", - { heading: section.heading, storageError }, + { + heading: section.heading, + // The model failed vs. we failed to keep what it made — distinct fields. + ...(img?.generationError !== undefined && { + generationError: img.generationError, + }), + ...(img?.storageError !== undefined && { + storageError: img.storageError, + }), + }, ); } diff --git a/examples/scene-to-video/AGENTS.md b/examples/scene-to-video/AGENTS.md index c06006f9..1c6e4df0 100644 --- a/examples/scene-to-video/AGENTS.md +++ b/examples/scene-to-video/AGENTS.md @@ -25,7 +25,7 @@ decompose ─▶ keyframe ⇄ collectKeyframe ─▶ animate ⇄ collect ─▶ - An agent is `defineAgent({ entry, steps })`; each step is `defineStep({ name, next, run, ... })`. Keep exactly one `defineAgent(...)` export. - **Capabilities come from the types.** What's available on `ctx.sapiom` is defined by `@sapiom/tools` — read the types / use autocomplete rather than guessing. A wrong capability or method name fails typecheck. One-shot LLM work uses `ctx.sapiom.llm.run({ request: { system, messages, max_tokens } })` through the gateway. -- **Async pause/resume.** A launched capability (`images.launch`, `video.launch`) returns a dispatch handle; `return pauseUntilSignal(handle, { resumeStep })` suspends the step until the job's signal arrives. The step must also **declare** the edge: `pause: { signal, resumeStep }`. The resumed step receives an `ImageResultPayload` / `VideoResultPayload` (`{ outputs: [{ fileId?, downloadUrl? }] }`). +- **Async pause/resume.** A launched capability (`images.launch`, `video.launch`) returns a dispatch handle; `return pauseUntilSignal(handle, { resumeStep })` suspends the step until the job's signal arrives. The step must also **declare** the edge: `pause: { signal, resumeStep }`. The resumed step receives an `ImageResultPayload` / `VideoResultPayload` (`{ outputs: [{ fileId?, downloadUrl?, generationError? }] }` — the signal fires on either terminal outcome, so check `generationError` before treating a missing `fileId` as a storage problem). - **Why `keyframe`/`animate` are sequential, not one paused step per job at once.** A paused step waits on a single `(signal, correlationId)` pair. Launching every job up front and then draining would risk one finishing before we've paused on it — its resume signal would have nowhere to land. Launching shot `i` only after shot `i-1` resumes keeps a paused step always waiting before its job can complete. This is also why keyframes use `images.launch` rather than a concurrent `Promise.all` of `images.create`: the synchronous routed call holds its request open for the full generate+store, which meets Core's 30s router cap under fan-out — `launch` submits and returns as soon as the job is enqueued, so it never does. ## Validating diff --git a/examples/scene-to-video/index.ts b/examples/scene-to-video/index.ts index a6f13b07..12d218d5 100644 --- a/examples/scene-to-video/index.ts +++ b/examples/scene-to-video/index.ts @@ -475,9 +475,13 @@ const collectKeyframe = defineStep({ const img = result.outputs?.[0]; if (!img?.fileId && !img?.downloadUrl) { - const storageError = img?.storageError ? `: ${img.storageError}` : ""; + // `generationError` means the model itself failed (no asset was ever made); + // `storageError` means it was made but we couldn't keep it. Report whichever applies. + const reason = img?.generationError ?? img?.storageError; throw new Error( - `keyframe generation completed without a usable output for shot ${index + 1}${storageError}`, + img?.generationError + ? `keyframe generation failed for shot ${index + 1}: ${img.generationError}` + : `keyframe generation completed without a usable output for shot ${index + 1}${reason ? `: ${reason}` : ""}`, ); } const frame: Keyframe = { @@ -553,9 +557,11 @@ const collect = defineStep({ const out = result.outputs?.[0]; if (!out?.fileId && !out?.downloadUrl) { - const storageError = out?.storageError ? `: ${out.storageError}` : ""; + const reason = out?.generationError ?? out?.storageError; throw new Error( - `video generation completed without a usable output for shot ${index + 1}${storageError}`, + out?.generationError + ? `video generation failed for shot ${index + 1}: ${out.generationError}` + : `video generation completed without a usable output for shot ${index + 1}${reason ? `: ${reason}` : ""}`, ); } const clip: Clip = { diff --git a/packages/tools/src/content-generation/README.md b/packages/tools/src/content-generation/README.md index e7d88650..6efb0072 100644 --- a/packages/tools/src/content-generation/README.md +++ b/packages/tools/src/content-generation/README.md @@ -230,7 +230,10 @@ defaults when `wait()` is called without arguments, so the two APIs stay in sync The capability-stable signal constant — declare it in the **pausing** step's static `pause` edge (`signal` and `resumeStep` are both required) so the engine -knows which signal to listen for and which step to resume with the result: +knows which signal to listen for and which step to resume with the result. It +fires on **either** terminal outcome, ready or failed, and the payload carries +which one it was (`generationError` on a failure), so the resumed step always +runs and branches: ```typescript import { defineStep, pauseUntilSignal, terminate } from "@sapiom/agent"; @@ -276,11 +279,28 @@ interface VideoResultPayload { downloadUrl?: string; // ready-to-use short-lived URL (may have expired by resume) downloadUrlExpiresAt?: string; // ISO expiry of downloadUrl, when present downloadUrlUnavailable?: boolean; // fileId is set but no URL could be minted — re-fetch from fileId - storageError?: string; // present when storage was requested but failed + storageError?: string; // the asset WAS generated, but persisting it failed + generationError?: string; // the generation itself failed — no asset ever existed }>; } ``` +`ImageResultPayload` is the same shape (`images.launch` resumes through the same rail). + +`storageError` and `generationError` are different failures and never both apply to one +output. Branch on `generationError` for "the model failed" — retrying the same prompt will +probably fail the same way — and on `storageError` for "the model succeeded but we couldn't +keep the result". Before v0.35 a terminal generation failure arrived as `storageError`, +which said the opposite of what happened. + +```typescript +const out = result.outputs[0]; +if (out?.generationError) + throw new Error(`generation failed: ${out.generationError}`); +if (out?.storageError) + throw new Error(`could not persist the output: ${out.storageError}`); +``` + Import `VideoResultPayload` from `@sapiom/tools` to annotate the resumed step's `input` type; import `toVideoResumePayload` to map a live `VideoGenerationResult` to this shape when wiring local tests. @@ -465,4 +485,9 @@ name the specific `VideoSelect` when you need `requires`. - **Failed requests throw `ContentGenerationHttpError`** (carries `status` + parsed `body`), exported from `@sapiom/tools`. +- **A failed _generation_ throws `ContentGenerationFailedError`** from `wait()` (and + from `video.create`), as soon as the job terminally fails rather than at `timeoutMs`. + It carries `requestId` and the provider's own `providerError`. A plain `Error` + (`"… did not complete within …ms"`) now means only what it says: the job was still + running when you stopped waiting. - **`storage` is reserved** — a same-named field in `params` is ignored. diff --git a/packages/tools/src/content-generation/errors.ts b/packages/tools/src/content-generation/errors.ts index 5c54a8a9..a26a17c0 100644 --- a/packages/tools/src/content-generation/errors.ts +++ b/packages/tools/src/content-generation/errors.ts @@ -15,6 +15,52 @@ export class ContentGenerationHttpError extends Error { } } +/** + * Error thrown when a launched media job reaches a terminal FAILED state — the provider + * job errored, was cancelled, or completed with an error, so no asset was ever produced + * (SAP-3097). + * + * Distinct from {@link ContentGenerationHttpError}, which reports an HTTP request that + * failed, and from the plain `Error` a poll throws when `timeoutMs` elapses with the job + * still in flight. Catching this one means "the generation failed"; catching the timeout + * means "it is still running and I stopped waiting". + * + * try { + * const out = await handle.wait(); + * } catch (err) { + * if (err instanceof ContentGenerationFailedError) { + * // The model failed. `err.providerError` says why; retrying the same prompt + * // will probably fail the same way. + * } + * } + */ +export class ContentGenerationFailedError extends Error { + /** The queue request id of the failed job. */ + readonly requestId: string; + /** The provider's own reason for the failure. */ + readonly providerError: string; + /** The raw polled body the failure was read from, for programmatic inspection. */ + readonly body: unknown; + + /** + * @param mediaLabel How the medium is named in the message, e.g. `"Image"` / `"Video"`. + */ + constructor( + mediaLabel: string, + requestId: string, + providerError: string, + body: unknown, + ) { + super( + `${mediaLabel} generation failed (request id: ${requestId}): ${providerError}`, + ); + this.name = "ContentGenerationFailedError"; + this.requestId = requestId; + this.providerError = providerError; + this.body = body; + } +} + /** * Return the response when 2xx, otherwise throw a {@link ContentGenerationHttpError}. * Parses the error body as JSON when possible; falls back to raw text. diff --git a/packages/tools/src/content-generation/index.spec.ts b/packages/tools/src/content-generation/index.spec.ts index 6d13bb29..14e71fcd 100644 --- a/packages/tools/src/content-generation/index.spec.ts +++ b/packages/tools/src/content-generation/index.spec.ts @@ -14,7 +14,9 @@ import { IMAGE_MODELS, VIDEO_MODEL_ALIASES, ContentGenerationHttpError, + ContentGenerationFailedError, } from "./index.js"; +import { terminalFailureFrom } from "./poll.js"; import type { ImageGenerationResult, VideoGenerationResult, @@ -2160,3 +2162,258 @@ describe("prompt-guard — createImage, launchImage, createVideo, launchVideo th }); } }); + +// --------------------------------------------------------------------------- +// Terminal generation failure (SAP-3097) +// +// A job that terminally fails must surface promptly, with the provider's reason, +// and must stay distinguishable from a transport blip that should keep polling. +// --------------------------------------------------------------------------- + +/** + * A submit + scripted poll sequence. Each poll response is consumed in order; the last + * one repeats. `/status` requests are answered from `statusResponses` when supplied, + * which is how the terminal channel is exercised. + */ +function makePollScript( + submitResponse: unknown, + pollResponses: Array<{ body: unknown; init?: ResponseInit }>, + statusResponses?: Array<{ body: unknown; init?: ResponseInit }>, +): { transport: Transport; calls: FetchCall[] } { + const calls: FetchCall[] = []; + let poll = 0; + let status = 0; + const fetchMock = (async ( + input: Parameters[0], + init: RequestInit = {}, + ): Promise => { + const url = String(input); + calls.push({ url, init }); + if (init.method === "POST") return jsonResponse(submitResponse); + if (url.endsWith("/status")) { + if (!statusResponses?.length) + return jsonResponse({ status: "IN_PROGRESS" }); + const next = + statusResponses[Math.min(status, statusResponses.length - 1)]!; + status += 1; + return jsonResponse(next.body, next.init); + } + const next = pollResponses[Math.min(poll, pollResponses.length - 1)]!; + poll += 1; + return jsonResponse(next.body, next.init); + }) as typeof globalThis.fetch; + return { + transport: new Transport({ apiKey: "test-key", fetch: fetchMock }), + calls, + }; +} + +const pollCount = (calls: FetchCall[]): number => + calls.filter((c) => c.init.method !== "POST" && !c.url.endsWith("/status")) + .length; + +describe("async media wait() — terminal generation failure (SAP-3097)", () => { + describe("image", () => { + const submit = { + requestId: "img-fail", + responseUrl: `${BASE}/queue/img-fail`, + statusUrl: `${BASE}/queue/img-fail/status`, + resolvedModel: "flux-fast", + }; + + it("fails fast with the provider's message when the polled body reports a terminal error", async () => { + const { transport, calls } = makePollScript(submit, [ + { body: { status: "ERROR", error: "content policy violation" } }, + ]); + + const handle = await launchImage({ prompt: "x" }, transport, BASE); + const wait = handle.wait({ timeoutMs: 60_000, pollMs: 1 }); + + await expect(wait).rejects.toBeInstanceOf(ContentGenerationFailedError); + await expect(wait).rejects.toMatchObject({ + requestId: "img-fail", + providerError: "content policy violation", + }); + await expect(wait).rejects.toThrow(/content policy violation/); + // Gave up on the first observation — it did not poll to `timeoutMs`. + expect(pollCount(calls)).toBe(1); + }); + + it("reports a COMPLETED-with-error job as a failure, not as a finished result", async () => { + const { transport } = makePollScript(submit, [ + { + body: { + status: "COMPLETED", + error: { message: "upstream model crashed" }, + }, + }, + ]); + + const handle = await launchImage({ prompt: "x" }, transport, BASE); + await expect( + handle.wait({ timeoutMs: 60_000, pollMs: 1 }), + ).rejects.toMatchObject({ + name: "ContentGenerationFailedError", + providerError: "upstream model crashed", + }); + }); + + it("consults the status endpoint when the result endpoint answers non-OK, and fails on a terminal status", async () => { + const { transport, calls } = makePollScript( + submit, + [{ body: { detail: "no result" }, init: { status: 400 } }], + [{ body: { status: "FAILED", error: "invalid duration enum" } }], + ); + + const handle = await launchImage({ prompt: "x" }, transport, BASE); + await expect( + handle.wait({ timeoutMs: 60_000, pollMs: 1 }), + ).rejects.toMatchObject({ providerError: "invalid duration enum" }); + expect(calls.some((c) => c.url.endsWith("/status"))).toBe(true); + }); + + it("keeps polling through a transient non-OK blip and resolves once the result lands", async () => { + const { transport, calls } = makePollScript( + submit, + [ + { body: { error: "bad gateway" }, init: { status: 502 } }, + { body: { images: [{ url: "https://media/x.png" }] } }, + ], + // The job is still running, so the terminal channel says so. + [{ body: { status: "IN_PROGRESS" } }], + ); + + const handle = await launchImage({ prompt: "x" }, transport, BASE); + await expect( + handle.wait({ timeoutMs: 60_000, pollMs: 1 }), + ).resolves.toMatchObject({ images: [{ url: "https://media/x.png" }] }); + expect(pollCount(calls)).toBe(2); + }); + + it("still resolves a clean success without touching the status endpoint", async () => { + const { transport, calls } = makePollScript(submit, [ + { body: { status: "IN_PROGRESS" } }, + { body: { images: [{ url: "https://media/x.png", fileId: "f-1" }] } }, + ]); + + const handle = await launchImage({ prompt: "x" }, transport, BASE); + await expect( + handle.wait({ timeoutMs: 60_000, pollMs: 1 }), + ).resolves.toMatchObject({ + images: [{ url: "https://media/x.png", fileId: "f-1" }], + }); + expect(calls.some((c) => c.url.endsWith("/status"))).toBe(false); + }); + }); + + describe("video", () => { + const submit = { + requestId: "vid-fail", + responseUrl: `${BASE}/queue/vid-fail`, + statusUrl: `${BASE}/queue/vid-fail/status`, + resolvedModel: "veo3-fast", + }; + + it("launch().wait() fails fast with the provider's message", async () => { + const { transport, calls } = makePollScript(submit, [ + { body: { status: "ERROR", error: "safety filter triggered" } }, + ]); + + const handle = await launchVideo({ prompt: "x" }, transport, BASE); + await expect( + handle.wait({ timeoutMs: 60_000, pollMs: 1 }), + ).rejects.toMatchObject({ + name: "ContentGenerationFailedError", + requestId: "vid-fail", + providerError: "safety filter triggered", + }); + expect(pollCount(calls)).toBe(1); + }); + + it("create() fails fast too — the sync-looking path polls the same way", async () => { + const { transport } = makePollScript(submit, [ + { body: { status: "CANCELLED" } }, + ]); + + await expect( + createVideo( + { prompt: "x", timeoutMs: 60_000, pollIntervalMs: 1 }, + transport, + BASE, + ), + ).rejects.toBeInstanceOf(ContentGenerationFailedError); + }); + + it("keeps polling through a transient non-OK blip and resolves once the result lands", async () => { + const { transport, calls } = makePollScript( + submit, + [ + { body: { error: "service unavailable" }, init: { status: 503 } }, + { body: { video: { url: "https://media/v.mp4" } } }, + ], + [{ body: { status: "IN_QUEUE" } }], + ); + + const handle = await launchVideo({ prompt: "x" }, transport, BASE); + await expect( + handle.wait({ timeoutMs: 60_000, pollMs: 1 }), + ).resolves.toMatchObject({ video: { url: "https://media/v.mp4" } }); + expect(pollCount(calls)).toBe(2); + }); + + it("still times out — with the timeout error, not a failure error — when the job never finishes", async () => { + const { transport } = makePollScript( + submit, + [{ body: { status: "IN_PROGRESS" } }], + [{ body: { status: "IN_PROGRESS" } }], + ); + + const handle = await launchVideo({ prompt: "x" }, transport, BASE); + const wait = handle.wait({ timeoutMs: 20, pollMs: 1 }); + await expect(wait).rejects.toThrow(/did not complete within/); + await expect(wait).rejects.not.toBeInstanceOf( + ContentGenerationFailedError, + ); + }); + }); +}); + +describe("terminalFailureFrom()", () => { + it.each([ + ["IN_QUEUE", { status: "IN_QUEUE" }], + ["IN_PROGRESS", { status: "IN_PROGRESS" }], + ["COMPLETED without an error", { status: "COMPLETED" }], + ["an unrecognized status", { status: "SOMETHING_NEW" }], + [ + "a body with no status at all", + { detail: "Request is still in progress" }, + ], + ["a non-object body", "plain text"], + ["an absent body", undefined], + ])("keeps polling on %s", (_label, body) => { + expect(terminalFailureFrom(body)).toBeNull(); + }); + + it.each([ + ["FAILED", { status: "FAILED", error: "boom" }, "boom"], + ["ERROR", { status: "error", error_type: "ModelError" }, "ModelError"], + [ + "COMPLETED with an error", + { status: "COMPLETED", error: { message: "no capacity" } }, + "no capacity", + ], + [ + "a serialized error object with no message", + { status: "FAILED", error: { code: 42 } }, + '{"code":42}', + ], + ])("reports %s as terminal", (_label, body, expected) => { + expect(terminalFailureFrom(body)).toBe(expected); + }); + + it("names the status when a terminal response carries no error detail", () => { + expect(terminalFailureFrom({ status: "CANCELLED" })).toBe( + "job reported cancelled", + ); + }); +}); diff --git a/packages/tools/src/content-generation/index.ts b/packages/tools/src/content-generation/index.ts index 2dc0ba8d..cabedcec 100644 --- a/packages/tools/src/content-generation/index.ts +++ b/packages/tools/src/content-generation/index.ts @@ -25,25 +25,33 @@ import { defaultTransport, resolveCoreBaseUrl, } from "../_client/index.js"; -import { ContentGenerationHttpError } from "./errors.js"; +import { + ContentGenerationFailedError, + ContentGenerationHttpError, +} from "./errors.js"; +import { pollForResult, statusUrlFromResultUrl } from "./poll.js"; import type { DispatchHandle } from "../dispatch.js"; -export { ContentGenerationHttpError }; +export { ContentGenerationFailedError, ContentGenerationHttpError }; /** * Capability-stable signal a video launch fires when the video reaches a terminal - * state (ready OR failed — it carries the result either way, the resumed step - * branches). A workflow step paused on a launch handle resumes on this; it is the - * value carried in the handle's `dispatch.resultSignal`. + * state — ready OR failed. It carries the result either way and the resumed step + * branches: a failed generation arrives as `outputs: [{ generationError }]` on the + * {@link VideoResultPayload} (SAP-3097), which is what makes that "either way" true. + * A workflow step paused on a launch handle resumes on this; it is the value carried + * in the handle's `dispatch.resultSignal`. */ export const VIDEO_RESULT_SIGNAL = "contentGeneration.video.result"; /** * Capability-stable signal an image launch fires when the image reaches a terminal - * state. The async completion→resume path is media-agnostic: the engine reads this - * name off the paused step row and matches the resume on `correlationId` (the launch - * `requestId`), so images resume through the exact same rail as video — this name is - * just the label carried in the handle's `dispatch.resultSignal`. + * state — ready OR failed, the same contract as {@link VIDEO_RESULT_SIGNAL} (a failed + * generation arrives as `outputs: [{ generationError }]`). The async completion→resume + * path is media-agnostic: the engine reads this name off the paused step row and matches + * the resume on `correlationId` (the launch `requestId`), so images resume through the + * exact same rail as video — this name is just the label carried in the handle's + * `dispatch.resultSignal`. */ export const IMAGE_RESULT_SIGNAL = "contentGeneration.images.result"; @@ -684,8 +692,23 @@ export interface ImageResultPayload extends MediaResumeFields { * `fileStorage.getPublicUrl(fileId)` — rather than treating a missing `downloadUrl` as "no asset". */ downloadUrlUnavailable?: boolean; - /** Present when storage was requested but persisting this output failed. */ + /** + * Present when storage was requested but persisting this output failed. The asset WAS + * generated — this is about keeping it, not making it. For "the model never produced + * anything" see {@link generationError}. + */ storageError?: string; + /** + * Present when the generation itself terminally failed (SAP-3097): the provider job + * errored, was cancelled, or completed with an error, so no asset ever existed. Carries + * the provider's own reason. + * + * Mutually exclusive with `fileId` / `downloadUrl` / `storageError` — there is nothing + * to store when nothing was generated. Branch on this field to tell "generation failed" + * from "storage failed"; before SAP-3097 a terminal generation failure arrived as + * `storageError`, which reported the opposite of what happened. + */ + generationError?: string; }>; } @@ -730,7 +753,9 @@ export function toImageResumePayload( * router cap — the failure mode the blocking sync path hits under a fan-out. * * Pass `storage` to persist the output (the result then carries `fileId`). Throws - * {@link ContentGenerationHttpError} when the submit fails. + * {@link ContentGenerationHttpError} when the submit fails. `wait()` throws + * {@link ContentGenerationFailedError} as soon as the job terminally fails, and a plain + * `Error` only when `timeoutMs` elapses with the job still running. */ export async function launchImage( input: ImageCreateInput, @@ -784,31 +809,24 @@ export async function launchImage( }: { timeoutMs?: number; pollMs?: number; - } = {}): Promise => { - const deadline = Date.now() + timeoutMs; - while (Date.now() < deadline) { - const res = await transport.fetch(responseUrl, { method: "GET" }); - if (res.ok) { - const raw = (await res.json()) as RawImageResult; - // Thread the submit handle's SAP-2576 cost + resolvedModel and E5 preferSatisfied - // onto the polled result. - if (Array.isArray(raw.images)) - return withDispatchMetadata(mapResult(raw), handle); - } else { - // Still generating, or a transient error. Drain the unread body so the - // connection can be reused, then keep polling — `timeoutMs` is the backstop. - try { - await res.body?.cancel(); - } catch { - // best-effort drain - } - } - await sleep(pollMs); - } - throw new Error( - `Image generation did not complete within ${timeoutMs}ms (request id: ${requestId})`, - ); - }; + } = {}): Promise => + pollForResult({ + transport, + resultUrl: responseUrl, + statusUrl: handle.statusUrl || statusUrlFromResultUrl(responseUrl), + requestId, + timeoutMs, + pollMs, + label: "Image", + // Thread the submit handle's SAP-2576 cost + resolvedModel and E5 preferSatisfied + // onto the polled result. + finished: (body) => { + const raw = body as RawImageResult; + return Array.isArray(raw?.images) + ? withDispatchMetadata(mapResult(raw), handle) + : undefined; + }, + }); return { requestId, @@ -1114,16 +1132,14 @@ function mapVideoResult(raw: RawVideoResult): { : { ...rest, video: mapVideo(video) }; } -const sleep = (ms: number): Promise => - new Promise((resolve) => setTimeout(resolve, ms)); - /** * Generate a video from a prompt. Video generation is asynchronous: this submits the * job, then polls the result through Sapiom until it's ready and returns it — so you * `await` it just like {@link createImage}, it just takes longer. Pass `storage` to * persist the output (the returned `video` then carries `fileId`). Throws - * {@link ContentGenerationHttpError} on a failed submit, or an `Error` if the result - * isn't ready within `timeoutMs`. + * {@link ContentGenerationHttpError} on a failed submit, + * {@link ContentGenerationFailedError} as soon as the generation terminally fails, or a + * plain `Error` when the job is still running at `timeoutMs`. * * Routed (SAP-2575): the submit goes through the shared {@link capabilityCall} seam to * `POST /v1/capabilities/content.generation.video` on the single Core base URL — the @@ -1180,32 +1196,23 @@ export async function createVideo( // Poll the result THROUGH Sapiom until it's ready. The poll is what persists the // output when `storage` was requested, so `fileId` is filled in by the time it returns. - const intervalMs = input.pollIntervalMs ?? DEFAULT_VIDEO_POLL_INTERVAL_MS; - const timeoutMs = input.timeoutMs ?? DEFAULT_VIDEO_TIMEOUT_MS; - const deadline = Date.now() + timeoutMs; - while (Date.now() < deadline) { - const res = await transport.fetch(responseUrl, { method: "GET" }); - if (res.ok) { - const raw = (await res.json()) as RawVideoResult; - // Thread the submit handle's SAP-2576 cost + resolvedModel and E5 preferSatisfied onto - // the polled result — the queue passthrough (this `raw`) carries none of them. - if (raw.video?.url) - return withDispatchMetadata(mapVideoResult(raw), handle); - } else { - // Still generating, or a transient error. Drain the unread body so the - // connection can be reused, then keep polling — `timeoutMs` is the backstop - // for a result that never arrives. - try { - await res.body?.cancel(); - } catch { - // best-effort drain - } - } - await sleep(intervalMs); - } - throw new Error( - `Video generation did not complete within ${timeoutMs}ms (request id: ${handle.requestId ?? "unknown"})`, - ); + return pollForResult({ + transport, + resultUrl: responseUrl, + statusUrl: handle.statusUrl || statusUrlFromResultUrl(responseUrl), + requestId: handle.requestId ?? "unknown", + timeoutMs: input.timeoutMs ?? DEFAULT_VIDEO_TIMEOUT_MS, + pollMs: input.pollIntervalMs ?? DEFAULT_VIDEO_POLL_INTERVAL_MS, + label: "Video", + // Thread the submit handle's SAP-2576 cost + resolvedModel and E5 preferSatisfied onto + // the polled result — the queue passthrough (this `raw`) carries none of them. + finished: (body) => { + const raw = body as RawVideoResult; + return raw?.video?.url + ? withDispatchMetadata(mapVideoResult(raw), handle) + : undefined; + }, + }); } /** @@ -1265,8 +1272,23 @@ export interface VideoResultPayload extends MediaResumeFields { * `fileStorage.getPublicUrl(fileId)` — rather than treating a missing `downloadUrl` as "no asset". */ downloadUrlUnavailable?: boolean; - /** Present when storage was requested but persisting this output failed. */ + /** + * Present when storage was requested but persisting this output failed. The asset WAS + * generated — this is about keeping it, not making it. For "the model never produced + * anything" see {@link generationError}. + */ storageError?: string; + /** + * Present when the generation itself terminally failed (SAP-3097): the provider job + * errored, was cancelled, or completed with an error, so no asset ever existed. Carries + * the provider's own reason. + * + * Mutually exclusive with `fileId` / `downloadUrl` / `storageError` — there is nothing + * to store when nothing was generated. Branch on this field to tell "generation failed" + * from "storage failed"; before SAP-3097 a terminal generation failure arrived as + * `storageError`, which reported the opposite of what happened. + */ + generationError?: string; }>; } @@ -1316,7 +1338,9 @@ export function toVideoResumePayload( * with the ability to suspend a running workflow. * * Pass `storage` to persist the output (the result then carries `fileId`). - * Throws {@link ContentGenerationHttpError} when the submit fails. + * Throws {@link ContentGenerationHttpError} when the submit fails. `wait()` throws + * {@link ContentGenerationFailedError} as soon as the job terminally fails, and a plain + * `Error` only when `timeoutMs` elapses with the job still running. * * Routed (SAP-2575): the submit is identical to {@link createVideo}'s — same body, * same `POST /v1/capabilities/content.generation.video` call through the shared @@ -1382,29 +1406,24 @@ export async function launchVideo( }: { timeoutMs?: number; pollMs?: number; - } = {}): Promise => { - const deadline = Date.now() + timeoutMs; - while (Date.now() < deadline) { - const res = await transport.fetch(responseUrl, { method: "GET" }); - if (res.ok) { - const raw = (await res.json()) as RawVideoResult; - // Thread the submit handle's SAP-2576 cost + resolvedModel and E5 preferSatisfied - // onto the polled result. - if (raw.video?.url) - return withDispatchMetadata(mapVideoResult(raw), handle); - } else { - try { - await res.body?.cancel(); - } catch { - // best-effort drain - } - } - await sleep(pollMs); - } - throw new Error( - `Video generation did not complete within ${timeoutMs}ms (request id: ${requestId})`, - ); - }; + } = {}): Promise => + pollForResult({ + transport, + resultUrl: responseUrl, + statusUrl: handle.statusUrl || statusUrlFromResultUrl(responseUrl), + requestId, + timeoutMs, + pollMs, + label: "Video", + // Thread the submit handle's SAP-2576 cost + resolvedModel and E5 preferSatisfied + // onto the polled result. + finished: (body) => { + const raw = body as RawVideoResult; + return raw?.video?.url + ? withDispatchMetadata(mapVideoResult(raw), handle) + : undefined; + }, + }); return { requestId, diff --git a/packages/tools/src/content-generation/poll.ts b/packages/tools/src/content-generation/poll.ts new file mode 100644 index 00000000..67ff8b8c --- /dev/null +++ b/packages/tools/src/content-generation/poll.ts @@ -0,0 +1,218 @@ +/** + * Shared polling for the async media capabilities (SAP-3097). + * + * Image and video generation both submit a job to the same queue and then poll a + * result URL through the Sapiom gateway. Before SAP-3097 every non-OK poll response + * was treated as "still generating", so a job that terminally failed in three seconds + * burned the caller's full `timeoutMs` and then threw `… did not complete within + * 300000ms` — the opposite of what happened. + * + * This module owns the one poll loop both media types use, so their terminal-failure + * semantics cannot drift apart. + * + * Two channels report a terminal failure, and both are read: + * + * 1. **The polled body itself.** A queue response carries `status` plus (on a + * completed-with-error job) `error` / `error_type`. {@link terminalFailureFrom} + * classifies it, mirroring the gateway's own `terminalStatusFromQueueResponse`. + * 2. **The status endpoint**, consulted only when the result endpoint answered non-OK. + * That answer is ambiguous on its own — a failed job never publishes a result body, + * but neither does a job that is merely still running, and neither does a gateway + * that just blipped. The status endpoint is the canonical terminal channel (it is + * what the platform's own settlement path reads), so it is what disambiguates + * "terminally failed" from "keep polling". + */ +import type { Transport } from "../_client/index.js"; +import { ContentGenerationFailedError } from "./errors.js"; + +/** Wait between polls. */ +const sleep = (ms: number): Promise => + new Promise((resolve) => setTimeout(resolve, ms)); + +/** + * Read a response body as JSON without ever throwing, and without leaving an unread + * stream behind (an undrained body keeps the connection from being reused). + * Returns `undefined` when the body is absent or isn't JSON. + */ +async function readJsonBody(res: Response): Promise { + try { + return (await res.json()) as unknown; + } catch { + try { + await res.body?.cancel(); + } catch { + // best-effort drain + } + return undefined; + } +} + +/** `error` / `error_type` on a queue response mean the job produced no asset. */ +function hasQueueError(body: Record): boolean { + return body.error != null || body.error_type != null; +} + +/** + * The provider's own words for why a job failed, from whichever field carries them. + * `undefined` when the response reported a failure but said nothing useful about it. + */ +function providerErrorMessage( + body: Record, +): string | undefined { + for (const raw of [body.error, body.error_type, body.detail]) { + if (typeof raw === "string" && raw.trim()) return raw.trim(); + if (typeof raw === "object" && raw !== null) { + const message = (raw as Record).message; + if (typeof message === "string" && message.trim()) return message.trim(); + try { + const serialized = JSON.stringify(raw); + if (serialized && serialized !== "{}" && serialized !== "[]") + return serialized; + } catch { + // Fall through to the next candidate field. + } + } + } + return undefined; +} + +/** + * Classify a queue response body: the provider's failure message when the job reached a + * terminal FAILED state, `null` when it did not (still queued, still running, finished + * cleanly, or a shape we don't recognize — all of which mean "keep polling"). + * + * Deliberately conservative. Only an explicit terminal marker ends the poll, so an + * unfamiliar body or a transport blip never gets reported to the caller as a generation + * failure. Mirrors `terminalStatusFromQueueResponse` in the x402 gateway — the two read + * the same wire contract and must agree. + * + * @internal Exported for tests. + */ +export function terminalFailureFrom(body: unknown): string | null { + if (typeof body !== "object" || body === null) return null; + const record = body as Record; + const status = + typeof record.status === "string" ? record.status.trim().toUpperCase() : ""; + switch (status) { + case "FAILED": + case "ERROR": + case "CANCELLED": + case "CANCELED": + return ( + providerErrorMessage(record) ?? `job reported ${status.toLowerCase()}` + ); + case "COMPLETED": + // A completed job that still carries an error produced no asset. + if (!hasQueueError(record)) return null; + return ( + providerErrorMessage(record) ?? "job completed with an unnamed error" + ); + default: + // IN_QUEUE / IN_PROGRESS / no status at all — not terminal, keep polling. + return null; + } +} + +/** + * Ask the status endpoint whether the job has terminally failed. Best-effort: a status + * endpoint that is itself unreachable or unparseable answers "don't know", which the + * caller treats as "keep polling" — the same conservative default as an unrecognized body. + */ +async function probeStatusForFailure( + transport: Transport, + statusUrl: string, +): Promise { + try { + const res = await transport.fetch(statusUrl, { method: "GET" }); + return terminalFailureFrom(await readJsonBody(res)); + } catch { + return null; + } +} + +/** + * The status endpoint for a job, given its result endpoint. The platform hands out both, + * but only `responseUrl` is threaded through the poll; this recovers the sibling URL from + * the queue's `.../requests/:id[/status]` convention when a handle carried no `statusUrl`. + */ +export function statusUrlFromResultUrl(resultUrl: string): string | undefined { + try { + const url = new URL(resultUrl); + if (url.pathname.endsWith("/status")) return url.toString(); + url.pathname = `${url.pathname.replace(/\/$/u, "")}/status`; + return url.toString(); + } catch { + return undefined; + } +} + +export interface PollForResultOptions { + transport: Transport; + /** The result endpoint. Polling it is also what persists the output when `storage` was requested. */ + resultUrl: string; + /** The canonical terminal channel, consulted only when `resultUrl` answers non-OK. */ + statusUrl?: string; + /** Queue request id — carried on both the timeout and the failure error. */ + requestId: string; + timeoutMs: number; + pollMs: number; + /** How this medium is named in error messages, e.g. `"Image"`. */ + label: string; + /** The mapped result when this body carries a finished asset; `undefined` while it doesn't. */ + finished: (body: unknown) => T | undefined; +} + +/** + * Poll a launched media job to a terminal state. + * + * Resolves with the mapped result on success. Throws + * {@link ContentGenerationFailedError} — promptly, not at the deadline — when the job + * terminally failed, carrying the provider's own error message. Throws a plain `Error` + * when `timeoutMs` elapses with the job still in flight. + */ +export async function pollForResult({ + transport, + resultUrl, + statusUrl, + requestId, + timeoutMs, + pollMs, + label, + finished, +}: PollForResultOptions): Promise { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + const res = await transport.fetch(resultUrl, { method: "GET" }); + const body = await readJsonBody(res); + + // A body that actually carries the asset wins over any failure marker beside it — + // if the output is here, hand it back rather than throwing about how it got here. + if (res.ok) { + const result = finished(body); + if (result !== undefined) return result; + } + + const failure = terminalFailureFrom(body); + if (failure !== null) + throw new ContentGenerationFailedError(label, requestId, failure, body); + + if (!res.ok && statusUrl) { + // Non-OK is ambiguous — a terminally failed job publishes no result, but neither + // does one that is simply not done yet, nor a gateway that briefly blipped. Ask + // the status endpoint, which reports terminal state unambiguously. + const statusFailure = await probeStatusForFailure(transport, statusUrl); + if (statusFailure !== null) + throw new ContentGenerationFailedError( + label, + requestId, + statusFailure, + body, + ); + } + + await sleep(pollMs); + } + throw new Error( + `${label} generation did not complete within ${timeoutMs}ms (request id: ${requestId})`, + ); +} diff --git a/packages/tools/src/index.ts b/packages/tools/src/index.ts index beeb93d9..afad5843 100644 --- a/packages/tools/src/index.ts +++ b/packages/tools/src/index.ts @@ -98,6 +98,9 @@ export { FileStorageHttpError } from "./file-storage/index.js"; export * as contentGeneration from "./content-generation/index.js"; export { ContentGenerationHttpError } from "./content-generation/index.js"; +// Thrown when a launched image/video job terminally FAILS (as distinct from an HTTP error on +// the request, or a poll that ran out its `timeoutMs` with the job still going) — SAP-3097. +export { ContentGenerationFailedError } from "./content-generation/index.js"; // The PUBLIC semantic model aliases the routed image/video capabilities serve, for callers that // want to pin one. Aliases are the supported input; raw provider ids still work but are deprecated // (SAP-2582), so pin from these maps rather than from the deprecated VIDEO_MODELS. From ab4c917af7b0847ba8ce3bc2df9d94d582a419b8 Mon Sep 17 00:00:00 2001 From: David Witwer Date: Tue, 1 Sep 2026 19:16:00 -0700 Subject: [PATCH 2/4] fix(tools): read the failure marker before the result predicate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on the SAP-3097 poll loop. `{ status: "COMPLETED", error: "…", images: [] }` resolved `wait()` with an empty result instead of throwing: the "an asset beside a failure marker wins" shortcut ran the result predicate first, and the image predicate accepts any array. An empty container is not an asset — it is the ambiguity this change exists to remove — so the terminal check now runs first. A non-terminal `images: []` still resolves, unchanged from before SAP-3097. The status probe ran on every non-OK poll. Some queues report "not ready yet" that way, so a 5-minute video at the 5s default went from ~60 gateway requests to ~120, per waiting caller. It is a tie-break, not a second poll: probe on the first non-OK and every 4th after, which still catches a job that failed immediately, bounds detection to four poll intervals, and holds the extra load to ~25%. Drop a private gateway symbol name and an internal architecture detail from `poll.ts`'s JSDoc — `declaration: true` emits it into the published .d.ts. Scope the published copy to what this release actually does. The `wait()` half is live; `generationError` is a type the platform populates once the gateway change deploys, and a step running against an older gateway still sees the failure on `storageError`. The changeset, README, and field docs now say so and point at the `generationError` ?? `storageError` fallback that reads correctly on both sides of that deploy. SAP-3097 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015XHWBRT86s84urfsTY9JJe --- .changeset/tidy-donkeys-report.md | 10 +-- .../tools/src/content-generation/README.md | 20 +++-- .../src/content-generation/index.spec.ts | 64 ++++++++++++++++ .../tools/src/content-generation/index.ts | 22 ++++-- packages/tools/src/content-generation/poll.ts | 75 ++++++++++++------- 5 files changed, 145 insertions(+), 46 deletions(-) diff --git a/.changeset/tidy-donkeys-report.md b/.changeset/tidy-donkeys-report.md index f8339cf6..00bf58cb 100644 --- a/.changeset/tidy-donkeys-report.md +++ b/.changeset/tidy-donkeys-report.md @@ -4,12 +4,12 @@ `contentGeneration`: an async image or video job that terminally FAILS is now distinguishable from one that is merely slow (SAP-3097). -**`wait()` fails fast.** Every non-OK poll response used to be treated as "still generating", so a job that failed in three seconds burned the caller's whole `timeoutMs` and then threw `Image generation did not complete within 300000ms` — the opposite of what happened. `wait()` (and `video.create`, which polls the same way) now reads the queue's terminal state and throws the new `ContentGenerationFailedError` as soon as the job fails, carrying `requestId` and the provider's own `providerError`. A plain `Error` about the timeout now means only what it says: the job was still running when you stopped waiting. +**`wait()` fails fast — live in this release.** Every non-OK poll response used to be treated as "still generating", so a job that failed in three seconds burned the caller's whole `timeoutMs` and then threw `Image generation did not complete within 300000ms` — the opposite of what happened. `wait()` (and `video.create`, which polls the same way) now reads the queue's terminal state and throws the new `ContentGenerationFailedError` as soon as the job fails, carrying `requestId` and the provider's own `providerError`. A plain `Error` about the timeout now means only what it says: the job was still running when you stopped waiting. `ContentGenerationFailedError` is exported from `@sapiom/tools`. -A transport blip still keeps polling. A non-OK result poll is ambiguous on its own, so it is disambiguated against the status endpoint — the canonical terminal channel — and anything short of an explicit terminal marker keeps the poll going. +A transport blip still keeps polling. A non-OK result poll is ambiguous on its own, so it is disambiguated against the status endpoint, and anything short of an explicit terminal marker keeps the poll going. -**`generationError` on the resume payload.** `ImageResultPayload` and `VideoResultPayload` outputs gain `generationError?: string`. A terminal provider failure used to arrive on `storageError` — the field documented as "persisting this output failed" — so a resumed workflow step concluded storage broke when in fact nothing was ever generated. The two are now separate fields and never both apply to one output: branch on `generationError` for "the model failed", `storageError` for "the model succeeded but we couldn't keep the result". +**`generationError` on the resume payload — a type, populated by the platform.** `ImageResultPayload` and `VideoResultPayload` outputs gain `generationError?: string`. A terminal provider failure has been arriving on `storageError` — the field documented as "persisting this output failed" — so a resumed workflow step concludes storage broke when in fact nothing was ever generated. The platform sends one field or the other, never both, so a step can branch without string-matching a message. -This also makes `VIDEO_RESULT_SIGNAL`'s documented "carries the result either way (ready OR failed)" contract true; the payload previously had nowhere to put the failure. `IMAGE_RESULT_SIGNAL` carries the same contract. +Nothing in this package produces `generationError`; it appears on the wire once the corresponding gateway change is deployed, and a step running against an older gateway still sees a generation failure on `storageError`. Check `generationError` first and fall back to `storageError` — that reads correctly on both sides of the deploy. -`ContentGenerationFailedError` is exported from `@sapiom/tools`. +The type is also what makes `VIDEO_RESULT_SIGNAL`'s documented "carries the result either way (ready OR failed)" contract expressible; the payload previously had nowhere to put the failure. `IMAGE_RESULT_SIGNAL` carries the same contract. diff --git a/packages/tools/src/content-generation/README.md b/packages/tools/src/content-generation/README.md index 6efb0072..0f1e6fbb 100644 --- a/packages/tools/src/content-generation/README.md +++ b/packages/tools/src/content-generation/README.md @@ -287,11 +287,10 @@ interface VideoResultPayload { `ImageResultPayload` is the same shape (`images.launch` resumes through the same rail). -`storageError` and `generationError` are different failures and never both apply to one -output. Branch on `generationError` for "the model failed" — retrying the same prompt will -probably fail the same way — and on `storageError` for "the model succeeded but we couldn't -keep the result". Before v0.35 a terminal generation failure arrived as `storageError`, -which said the opposite of what happened. +`storageError` and `generationError` are different failures, and the platform sends one or +the other, never both. Branch on `generationError` for "the model failed" — retrying the +same prompt will probably fail the same way — and on `storageError` for "the model +succeeded but we couldn't keep the result". ```typescript const out = result.outputs[0]; @@ -301,9 +300,18 @@ if (out?.storageError) throw new Error(`could not persist the output: ${out.storageError}`); ``` +`generationError` is populated by the platform, so it appears once the gateway change that +emits it is deployed. Until then a terminal generation failure still arrives on +`storageError`, saying the opposite of what happened — which is the reason for the split. +Checking `generationError` first and falling back to `storageError`, as above, reads +correctly on both sides of that deploy. + Import `VideoResultPayload` from `@sapiom/tools` to annotate the resumed step's `input` type; import `toVideoResumePayload` to map a live `VideoGenerationResult` -to this shape when wiring local tests. +to this shape when wiring local tests. The mappers carry no `generationError`: a live +`wait()` throws `ContentGenerationFailedError` on a terminal failure rather than +returning one, so a `VideoGenerationResult` has no failure to map. Build that payload +literally when you want to exercise the branch. ## Input params diff --git a/packages/tools/src/content-generation/index.spec.ts b/packages/tools/src/content-generation/index.spec.ts index 14e71fcd..6997f21a 100644 --- a/packages/tools/src/content-generation/index.spec.ts +++ b/packages/tools/src/content-generation/index.spec.ts @@ -2258,6 +2258,39 @@ describe("async media wait() — terminal generation failure (SAP-3097)", () => }); }); + it("reports a terminal body carrying an EMPTY images[] as a failure, not an empty result", async () => { + // `images: []` is a container for the asset the job never produced. Resolving it as + // a success hands the caller back the exact ambiguity this change removes. + const { transport } = makePollScript(submit, [ + { + body: { + status: "COMPLETED", + error: "content policy violation", + images: [], + }, + }, + ]); + + const handle = await launchImage({ prompt: "x" }, transport, BASE); + await expect( + handle.wait({ timeoutMs: 60_000, pollMs: 1 }), + ).rejects.toMatchObject({ + name: "ContentGenerationFailedError", + providerError: "content policy violation", + }); + }); + + it("still resolves an empty images[] when nothing marks the job as failed", async () => { + // Unchanged from before SAP-3097: an `images: []` with no failure marker is a + // finished (if empty) result, not a reason to poll to the deadline. + const { transport } = makePollScript(submit, [{ body: { images: [] } }]); + + const handle = await launchImage({ prompt: "x" }, transport, BASE); + await expect( + handle.wait({ timeoutMs: 60_000, pollMs: 1 }), + ).resolves.toMatchObject({ images: [] }); + }); + it("consults the status endpoint when the result endpoint answers non-OK, and fails on a terminal status", async () => { const { transport, calls } = makePollScript( submit, @@ -2306,6 +2339,37 @@ describe("async media wait() — terminal generation failure (SAP-3097)", () => }); }); + it("probes the status endpoint on a slower cadence than the poll, not on every tick", async () => { + // A queue that reports "not ready yet" as a non-OK result response must not cost two + // gateway requests per tick for the job's whole lifetime. + const { transport, calls } = makePollScript( + { + requestId: "img-slow", + responseUrl: `${BASE}/queue/img-slow`, + statusUrl: `${BASE}/queue/img-slow/status`, + resolvedModel: "flux-fast", + }, + [ + { + body: { detail: "Request is still in progress" }, + init: { status: 400 }, + }, + ], + [{ body: { status: "IN_PROGRESS" } }], + ); + + const handle = await launchImage({ prompt: "x" }, transport, BASE); + await expect(handle.wait({ timeoutMs: 30, pollMs: 1 })).rejects.toThrow( + /did not complete within/, + ); + + const polls = pollCount(calls); + const probes = calls.filter((c) => c.url.endsWith("/status")).length; + expect(polls).toBeGreaterThan(4); + // First non-OK poll, then every 4th — never one probe per poll. + expect(probes).toBe(Math.ceil(polls / 4)); + }); + describe("video", () => { const submit = { requestId: "vid-fail", diff --git a/packages/tools/src/content-generation/index.ts b/packages/tools/src/content-generation/index.ts index cabedcec..b42c7de7 100644 --- a/packages/tools/src/content-generation/index.ts +++ b/packages/tools/src/content-generation/index.ts @@ -703,10 +703,13 @@ export interface ImageResultPayload extends MediaResumeFields { * errored, was cancelled, or completed with an error, so no asset ever existed. Carries * the provider's own reason. * - * Mutually exclusive with `fileId` / `downloadUrl` / `storageError` — there is nothing - * to store when nothing was generated. Branch on this field to tell "generation failed" - * from "storage failed"; before SAP-3097 a terminal generation failure arrived as - * `storageError`, which reported the opposite of what happened. + * The platform sends this INSTEAD of the storage fields, not alongside them — there is + * nothing to store when nothing was generated — so branching on it tells "generation + * failed" from "storage failed" without reading an error message. Populated by the + * platform; nothing in this package produces it, and a resumed step running against a + * gateway that predates the change still sees the old shape, where a terminal + * generation failure arrived as `storageError` and reported the opposite of what + * happened. Check `generationError` first and fall back to `storageError`. */ generationError?: string; }>; @@ -1283,10 +1286,13 @@ export interface VideoResultPayload extends MediaResumeFields { * errored, was cancelled, or completed with an error, so no asset ever existed. Carries * the provider's own reason. * - * Mutually exclusive with `fileId` / `downloadUrl` / `storageError` — there is nothing - * to store when nothing was generated. Branch on this field to tell "generation failed" - * from "storage failed"; before SAP-3097 a terminal generation failure arrived as - * `storageError`, which reported the opposite of what happened. + * The platform sends this INSTEAD of the storage fields, not alongside them — there is + * nothing to store when nothing was generated — so branching on it tells "generation + * failed" from "storage failed" without reading an error message. Populated by the + * platform; nothing in this package produces it, and a resumed step running against a + * gateway that predates the change still sees the old shape, where a terminal + * generation failure arrived as `storageError` and reported the opposite of what + * happened. Check `generationError` first and fall back to `storageError`. */ generationError?: string; }>; diff --git a/packages/tools/src/content-generation/poll.ts b/packages/tools/src/content-generation/poll.ts index 67ff8b8c..e44c0cdd 100644 --- a/packages/tools/src/content-generation/poll.ts +++ b/packages/tools/src/content-generation/poll.ts @@ -14,13 +14,14 @@ * * 1. **The polled body itself.** A queue response carries `status` plus (on a * completed-with-error job) `error` / `error_type`. {@link terminalFailureFrom} - * classifies it, mirroring the gateway's own `terminalStatusFromQueueResponse`. - * 2. **The status endpoint**, consulted only when the result endpoint answered non-OK. - * That answer is ambiguous on its own — a failed job never publishes a result body, - * but neither does a job that is merely still running, and neither does a gateway - * that just blipped. The status endpoint is the canonical terminal channel (it is - * what the platform's own settlement path reads), so it is what disambiguates - * "terminally failed" from "keep polling". + * classifies it, mirroring the gateway's own terminal-status classification of the + * same wire contract. + * 2. **The status endpoint**, consulted when the result endpoint answers non-OK. That + * answer is ambiguous on its own — a failed job never publishes a result body, but + * neither does a job that is merely still running, and neither does a gateway that + * just blipped. The status endpoint reports terminal state unambiguously, so it is + * what breaks the tie between "terminally failed" and "keep polling". It is a tie + * break, not a second poll: see {@link STATUS_PROBE_EVERY}. */ import type { Transport } from "../_client/index.js"; import { ContentGenerationFailedError } from "./errors.js"; @@ -29,6 +30,19 @@ import { ContentGenerationFailedError } from "./errors.js"; const sleep = (ms: number): Promise => new Promise((resolve) => setTimeout(resolve, ms)); +/** + * How often to spend a status request while the result endpoint keeps answering non-OK: + * on the first such poll, then every Nth after it. + * + * Some queues report "not ready yet" as a non-OK result response, so an unthrottled probe + * would double the request count for a job's entire lifetime — a 5-minute video at the 5s + * default goes from ~60 gateway requests to ~120, per waiting caller. The probe exists to + * break a tie, not to be a second poll: probing on the first non-OK still catches a job + * that failed immediately, and every 4th after that bounds detection to four poll + * intervals while holding the extra load to ~25%. + */ +const STATUS_PROBE_EVERY = 4; + /** * Read a response body as JSON without ever throwing, and without leaving an unread * stream behind (an undrained body keeps the connection from being reused). @@ -83,8 +97,8 @@ function providerErrorMessage( * * Deliberately conservative. Only an explicit terminal marker ends the poll, so an * unfamiliar body or a transport blip never gets reported to the caller as a generation - * failure. Mirrors `terminalStatusFromQueueResponse` in the x402 gateway — the two read - * the same wire contract and must agree. + * failure. Mirrors the gateway's own classification of the same wire contract — the two + * must agree. * * @internal Exported for tests. */ @@ -181,33 +195,40 @@ export async function pollForResult({ finished, }: PollForResultOptions): Promise { const deadline = Date.now() + timeoutMs; + let nonOkPolls = 0; while (Date.now() < deadline) { const res = await transport.fetch(resultUrl, { method: "GET" }); const body = await readJsonBody(res); - // A body that actually carries the asset wins over any failure marker beside it — - // if the output is here, hand it back rather than throwing about how it got here. - if (res.ok) { - const result = finished(body); - if (result !== undefined) return result; - } - + // The failure marker is read BEFORE the result predicate. A terminal body can still + // carry an empty container for the asset it never produced — `{ status: "COMPLETED", + // error: "…", images: [] }` — and reporting that as a successful empty result is the + // exact ambiguity this module exists to remove. const failure = terminalFailureFrom(body); if (failure !== null) throw new ContentGenerationFailedError(label, requestId, failure, body); - if (!res.ok && statusUrl) { + if (res.ok) { + const result = finished(body); + if (result !== undefined) return result; + // A 2xx that isn't the finished asset is an in-progress status body. Keep polling. + nonOkPolls = 0; + } else { // Non-OK is ambiguous — a terminally failed job publishes no result, but neither - // does one that is simply not done yet, nor a gateway that briefly blipped. Ask - // the status endpoint, which reports terminal state unambiguously. - const statusFailure = await probeStatusForFailure(transport, statusUrl); - if (statusFailure !== null) - throw new ContentGenerationFailedError( - label, - requestId, - statusFailure, - body, - ); + // does one that is simply not done yet, nor a gateway that briefly blipped. The + // status endpoint breaks the tie; it is consulted on a slower cadence than the + // poll so a queue that reports in-progress this way doesn't cost double. + nonOkPolls += 1; + if (statusUrl && nonOkPolls % STATUS_PROBE_EVERY === 1) { + const statusFailure = await probeStatusForFailure(transport, statusUrl); + if (statusFailure !== null) + throw new ContentGenerationFailedError( + label, + requestId, + statusFailure, + body, + ); + } } await sleep(pollMs); From 726cb6641601db60066469d765f6db73c5f51f14 Mon Sep 17 00:00:00 2001 From: David Witwer Date: Tue, 1 Sep 2026 19:21:41 -0700 Subject: [PATCH 3/4] fix(tools): only real error content makes a COMPLETED job terminal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 review follow-ups. `hasQueueError` tested key presence (`!= null`), so `error: ""` counted as a failure. Harmless while the success path ran first, but the previous commit moved the terminal check ahead of the result predicate — so a queue that emits `error: ""` on a clean completion would now throw `ContentGenerationFailedError` while holding the asset in hand. COMPLETED is terminal-SUCCESS by default and the error content is the only thing that makes it a failure, so that content has to be real: the branch now gates on `providerErrorMessage`, which already required a non-empty trimmed value. The statuses that are terminal on their own (FAILED / ERROR / CANCELLED) are unaffected and keep their "job reported " fallback. The probe-cadence test asserted `polls > 4` inside a 30ms budget against real timers, which a loaded CI box can miss for no reason. It now scripts 8 non-OK polls followed by the result and asserts exact counts — 9 polls, 2 probes. SAP-3097 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015XHWBRT86s84urfsTY9JJe --- .../src/content-generation/index.spec.ts | 58 ++++++++++++++----- packages/tools/src/content-generation/poll.ts | 21 +++---- 2 files changed, 54 insertions(+), 25 deletions(-) diff --git a/packages/tools/src/content-generation/index.spec.ts b/packages/tools/src/content-generation/index.spec.ts index 6997f21a..8028358c 100644 --- a/packages/tools/src/content-generation/index.spec.ts +++ b/packages/tools/src/content-generation/index.spec.ts @@ -2280,6 +2280,25 @@ describe("async media wait() — terminal generation failure (SAP-3097)", () => }); }); + it("resolves a completed job that carries an empty error string alongside the asset", async () => { + // Some queues emit `error: ""` on the happy path. Since the terminal check now runs + // ahead of the result predicate, a present-but-empty key must not fail the job. + const { transport } = makePollScript(submit, [ + { + body: { + status: "COMPLETED", + error: "", + images: [{ url: "https://media/x.png" }], + }, + }, + ]); + + const handle = await launchImage({ prompt: "x" }, transport, BASE); + await expect( + handle.wait({ timeoutMs: 60_000, pollMs: 1 }), + ).resolves.toMatchObject({ images: [{ url: "https://media/x.png" }] }); + }); + it("still resolves an empty images[] when nothing marks the job as failed", async () => { // Unchanged from before SAP-3097: an `images: []` with no failure marker is a // finished (if empty) result, not a reason to poll to the deadline. @@ -2341,7 +2360,12 @@ describe("async media wait() — terminal generation failure (SAP-3097)", () => it("probes the status endpoint on a slower cadence than the poll, not on every tick", async () => { // A queue that reports "not ready yet" as a non-OK result response must not cost two - // gateway requests per tick for the job's whole lifetime. + // gateway requests per tick for the job's whole lifetime. Driven by a scripted poll + // count, not a time budget, so the assertion doesn't depend on how fast CI runs. + const inProgress = { + body: { detail: "Request is still in progress" }, + init: { status: 400 }, + }; const { transport, calls } = makePollScript( { requestId: "img-slow", @@ -2350,24 +2374,20 @@ describe("async media wait() — terminal generation failure (SAP-3097)", () => resolvedModel: "flux-fast", }, [ - { - body: { detail: "Request is still in progress" }, - init: { status: 400 }, - }, + ...Array.from({ length: 8 }, () => inProgress), + { body: { images: [{ url: "https://media/x.png" }] } }, ], [{ body: { status: "IN_PROGRESS" } }], ); const handle = await launchImage({ prompt: "x" }, transport, BASE); - await expect(handle.wait({ timeoutMs: 30, pollMs: 1 })).rejects.toThrow( - /did not complete within/, - ); + await expect( + handle.wait({ timeoutMs: 60_000, pollMs: 1 }), + ).resolves.toMatchObject({ images: [{ url: "https://media/x.png" }] }); - const polls = pollCount(calls); - const probes = calls.filter((c) => c.url.endsWith("/status")).length; - expect(polls).toBeGreaterThan(4); - // First non-OK poll, then every 4th — never one probe per poll. - expect(probes).toBe(Math.ceil(polls / 4)); + // 8 non-OK polls then the result: probed on non-OK #1 and #5 only, never per-poll. + expect(pollCount(calls)).toBe(9); + expect(calls.filter((c) => c.url.endsWith("/status"))).toHaveLength(2); }); describe("video", () => { @@ -2480,4 +2500,16 @@ describe("terminalFailureFrom()", () => { "job reported cancelled", ); }); + + it.each([ + ["an empty error string", { status: "COMPLETED", error: "" }], + ["an empty error_type", { status: "COMPLETED", error_type: "" }], + ["a whitespace-only error", { status: "COMPLETED", error: " " }], + ["a contentless error object", { status: "COMPLETED", error: {} }], + ])( + "does not fail a COMPLETED job on %s — only real error content is terminal", + (_label, body) => { + expect(terminalFailureFrom(body)).toBeNull(); + }, + ); }); diff --git a/packages/tools/src/content-generation/poll.ts b/packages/tools/src/content-generation/poll.ts index e44c0cdd..7177e619 100644 --- a/packages/tools/src/content-generation/poll.ts +++ b/packages/tools/src/content-generation/poll.ts @@ -61,14 +61,10 @@ async function readJsonBody(res: Response): Promise { } } -/** `error` / `error_type` on a queue response mean the job produced no asset. */ -function hasQueueError(body: Record): boolean { - return body.error != null || body.error_type != null; -} - /** * The provider's own words for why a job failed, from whichever field carries them. - * `undefined` when the response reported a failure but said nothing useful about it. + * `undefined` when the response carries no error content — which covers both "said nothing + * useful" and the empty `error: ""` / `error_type: ""` a queue can emit on the happy path. */ function providerErrorMessage( body: Record, @@ -115,12 +111,13 @@ export function terminalFailureFrom(body: unknown): string | null { return ( providerErrorMessage(record) ?? `job reported ${status.toLowerCase()}` ); - case "COMPLETED": - // A completed job that still carries an error produced no asset. - if (!hasQueueError(record)) return null; - return ( - providerErrorMessage(record) ?? "job completed with an unnamed error" - ); + case "COMPLETED": { + // Unlike the statuses above, COMPLETED is terminal-SUCCESS by default: the error + // content is the only thing that makes it a failure. So it has to be real content, + // not merely a present key — a queue that emits `error: ""` on a clean completion + // would otherwise fail a job while it is handing back the asset. + return providerErrorMessage(record) ?? null; + } default: // IN_QUEUE / IN_PROGRESS / no status at all — not terminal, keep polling. return null; From 21cf178c50dc5906c60b8f22f94ca59cd3701d67 Mon Sep 17 00:00:00 2001 From: David Witwer Date: Tue, 1 Sep 2026 20:57:37 -0700 Subject: [PATCH 4/4] docs(tools): reconcile the README with the images.launch docs from #770 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rebase follow-up. SAP-3098 (#770) landed a full `images.launch` section on main while this branch was open, and three parts of it describe the behaviour this PR changes: - It documents the bug as current: "a terminal provider failure is not currently distinguishable from a slow one … surfaces as the deadline Error". That is the before-state; `wait()` now throws `ContentGenerationFailedError` on the failure. - Its new `ImageResultPayload` block predates `generationError`, so the field was documented on the video payload only. - Its `IMAGE_RESULT_SIGNAL` section says the signal fires "when the job reaches a terminal state" without saying the payload can now carry the failure — the same either-outcome note `VIDEO_RESULT_SIGNAL` gets. The conflict itself was one paragraph: #770 rewrote the `VIDEO_RESULT_SIGNAL` lead-in to describe the `pause` edge, this branch added the either-outcome sentence. Both kept. SAP-3097 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015XHWBRT86s84urfsTY9JJe --- .../tools/src/content-generation/README.md | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/packages/tools/src/content-generation/README.md b/packages/tools/src/content-generation/README.md index 0f1e6fbb..95e0e7d2 100644 --- a/packages/tools/src/content-generation/README.md +++ b/packages/tools/src/content-generation/README.md @@ -96,17 +96,20 @@ values onto its result. video, `ImageCreateInput` has no `timeoutMs` / `pollIntervalMs` fields, so those are the only defaults and any override goes on the `wait()` call. -On the `wait()` path a terminal provider failure is not currently distinguishable -from a slow one: a non-OK poll is treated as "still generating", so a failed job -surfaces as the deadline `Error` (`Image generation did not complete within -…ms`) rather than a failure-specific error. +On the `wait()` path a terminal provider failure throws +`ContentGenerationFailedError` as soon as the job fails, carrying `requestId` and +the provider's own `providerError` — it does not run out the clock first. The +deadline `Error` (`Image generation did not complete within …ms`) now means only +what it says: the job was still running when you stopped waiting. ### `IMAGE_RESULT_SIGNAL` The capability-stable signal constant (`"contentGeneration.images.result"`), -fired when the job reaches a terminal state. Declare it in the **pausing** step's -static `pause` edge — `signal` and `resumeStep` are both required — so the engine -knows which signal to listen for and which step to resume with the result: +fired when the job reaches a terminal state — **either** outcome, ready or failed. +The payload carries which one it was (`generationError` on a failure), so the +resumed step always runs and branches. Declare it in the **pausing** step's static +`pause` edge — `signal` and `resumeStep` are both required — so the engine knows +which signal to listen for and which step to resume with the result: ```typescript import { defineStep, pauseUntilSignal, terminate } from "@sapiom/agent"; @@ -161,7 +164,8 @@ interface ImageResultPayload { downloadUrl?: string; // ready-to-use short-lived URL (may have expired by resume) downloadUrlExpiresAt?: string; // ISO expiry of downloadUrl, when present downloadUrlUnavailable?: boolean; // fileId is set but no URL could be minted — re-fetch from fileId - storageError?: string; // present when storage was requested but failed + storageError?: string; // the asset WAS generated, but persisting it failed + generationError?: string; // the generation itself failed — no asset ever existed }>; } ``` @@ -169,6 +173,11 @@ interface ImageResultPayload { The per-image `width` / `height` / `url` fields of `images.create` are not on the resume payload — `fileId` is the durable handle to re-fetch from. +`storageError` and `generationError` are different failures, and the platform sends +one or the other, never both — see [the video payload](#videoresultpayload) for the +branch that reads correctly on both sides of the gateway deploy that starts emitting +`generationError`. + Import `ImageResultPayload` from `@sapiom/tools` to annotate the resumed step's `input` type; import `toImageResumePayload` to map a live `ImageGenerationResult` to this shape when wiring local tests. @@ -285,7 +294,8 @@ interface VideoResultPayload { } ``` -`ImageResultPayload` is the same shape (`images.launch` resumes through the same rail). +[`ImageResultPayload`](#imageresultpayload) is the same shape — `images.launch` resumes +through the same rail. `storageError` and `generationError` are different failures, and the platform sends one or the other, never both. Branch on `generationError` for "the model failed" — retrying the