Uh oh!
There was an error while loading. Please reload this page.
fix(core): make step-argument serialization failures catchable in workflow code - #3675
Conversation
…kflow code A step whose arguments fail to serialize is now finalized by the suspension handler as step_created + step_failed (mirroring a step-body failure) instead of rejecting the whole suspension. The next replay — forced in-process, since no step message is dispatched for the failed step — rejects the step's promise with the SerializationError, so a try/catch around the step call observes it. Uncaught, the error propagates out of the workflow body and fails the run as a fatal USER_ERROR immediately, instead of redelivering the orchestrator message until max deliveries (49/48) as reported in production on v4.
🦋 Changeset detectedLatest commit: 00b30cf The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Pull request overview
This PR fixes a workflow-runtime observability gap where step argument serialization failures occurred after the workflow parked, causing repeated redeliveries and making the failure uncatchable from "use workflow" code. It changes suspension handling so step-argument serialization failures are finalized as step failures (step_created + step_failed), allowing workflows to catch SerializationError the same way they can catch step-body failures and step return-value serialization failures.
Changes:
- Catch
SerializationErrorfrom step-argument dehydration inhandleSuspension, finalize the step as failed, and report failed step correlation IDs. - Force an in-process cold replay when any step-argument serialization failures were finalized, ensuring the workflow observes the
step_failedevent. - Add unit + e2e coverage for caught/uncaught step-argument and step return-value serialization failures, plus a changeset entry.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| workbench/example/workflows/99_e2e.ts | Adds workflow/step fixtures used by the e2e tests to trigger step-arg and step-return serialization failures (caught and uncaught variants). |
| packages/core/src/runtime/suspension-handler.ts | Implements per-step handling of step-argument serialization failures by writing step_created (placeholder input) + step_failed, and returns failedStepCorrelationIds. |
| packages/core/src/runtime/suspension-handler.test.ts | Adds unit tests covering single/batched scenarios, payload round-trip, and conflict/run-expired tolerances. |
| packages/core/src/runtime.ts | Forces an in-process replay when failedStepCorrelationIds is non-empty so workflow code can observe the failure deterministically. |
| packages/core/e2e/e2e.test.ts | Adds end-to-end regression coverage for the new catchable behavior and for “fail fast as USER_ERROR” semantics. |
| .changeset/catchable-step-arg-serialization-errors.md | Documents the user-facing behavior change as a patch bump for @workflow/core. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Uh oh!
There was an error while loading. Please reload this page.
…geset Addresses review feedback: dehydrateStepError in finalizeUnserializableStep now receives suspension.globalThis like every other dehydration in this file. Error detection is realm-independent, so the host-created SerializationError serializes identically, but VM-realm values guest code threw into the cause chain are now detected by the realm-sensitive reducers.
…ep-arg-serialization-errors # Conflicts: # packages/core/src/runtime/suspension-handler.test.ts
Uh oh!
There was an error while loading. Please reload this page.
- A crash or transient failure between finalization's two durable writes leaves a lone placeholder step_created, and redelivery then dispatches the step through normal crash recovery — previously running user code with the placeholder arguments. The placeholder now carries a structural flag on the input triple's top level (which user code never controls, so no false positives), and the step executor checks it after hydration: instead of running the body, it throws the intended fatal SerializationError, completing the interrupted finalization as step_failed. Applies to both engines (they share the placeholder and the executor). - Regression tests: executor fails a placeholder-input step without running the body (and doesn't trip on a genuine argument equal to the display marker); handleSuspension rejects for redelivery when step_failed can't be written after step_created landed, leaving the recoverable placeholder behind; mixed bad-step + large fan-out returns the failure set alongside still-pending deferredBatchWork whose rejection surfaces — the contract the runtime's failed-step join (added previously) relies on. - Docs: the two new code samples are now self-contained so the docs code-sample typecheck passes.
karthikscale3
left a comment
There was a problem hiding this comment.
AI Review: Re-reviewed the latest head. The interrupted-finalization placeholder is now safely recovered without executing user code, deferred batch work is joined before replay, QuickJS has equivalent behavior, and targeted regression coverage is present. No blocking issues remain.
VaguelySerious
left a comment
There was a problem hiding this comment.
AI review: no blocking issues
| // finalizeUnserializableStep. In the terminal drain | ||
| // (finalizeUnserializableSteps unset), skip entirely — see the | ||
| // param docs. | ||
| if (step.serializationError) { |
There was a problem hiding this comment.
AI Review: Note
Verified this round, against the head SHA:
- CI is green on
00b30cf— all 163 checks pass,E2E Required Checkincluded, and all 57 QuickJS lanes (Local Dev / Postgres / Prod, Windows, Vercel Prod, Multi-Region) that were red last round now pass. - Locally,
nextjs-turbopackdev server withWORKFLOW_VM=quickjs: the four serialization e2e tests pass, and the dev log shows this path firing (Step arguments failed to serialize; failing the step so the workflow can observe the error, three runs) — so the QuickJS engine really is exercising the new finalize rather than passing by luck. - Added a fan-out fixture the current e2e fixtures don't cover:
Promise.allSettled([badStep, ...five healthy steps]), so the bad step coexists with pair-folded inline claims and eager batched creates. Green on both engines, and the event log is exactly what the design predicts:
6 step_created (5 healthy + 1 placeholder)
5 step_started (one per healthy step — no double claim)
5 step_completed
1 step_failed
1 run_completed run status: completed, ~4.6s wall, no stall
with the workflow observing ['rejected','fulfilled','fulfilled','fulfilled','fulfilled','fulfilled'] and SerializationError on the rejected one. That settles what was behind my earlier point (2): healthy siblings are neither stranded nor executed twice, and the pass costs one extra replay rather than a stall. Not committed.
| * Absent when {@link serializationError} is set — the input is precisely | ||
| * what refused to serialize. | ||
| */ | ||
| input?: Uint8Array; |
There was a problem hiding this comment.
AI Review: Nit
Making input optional doesn't get you a compiler guard at either deref site: encrypt(data: Uint8Array | unknown, …) returns its argument unchanged when it isn't a Uint8Array, so an undefined input would sail through encryptSerializedData(step.input, …) in dispatchPendingOps (and through lazyStepInput on the inline path) as undefined rather than failing to typecheck. Both sites are correct today — the serializationError early return covers one, healthySteps the other — but the invariant rests on those two filters, not on the type. A narrowing throw at the deref would turn a future caller's mistake into a build error instead of a step created with no input.
Backport PR opened against |
Problem
A customer reported that passing an unserializable value to a step produced
The failure happens in the host after the workflow function has parked, so a
try/catchin the"use workflow"body never sees it — the run just redelivers until max-deliveries exhaustion (on v4; onmainsince #1849 it fails fast, but is still unobservable from workflow code).This is asymmetric with step return-value serialization failures, which already write a
step_failedevent and reject into the workflow catchably.Fix
handleSuspensionnow catches theSerializationErrorfromdehydrateStepArgumentsper-step and, instead of rejecting the whole suspension:step_createdwith a placeholder input (every World requires the step entity to exist before a terminal step event — and the real input is precisely what refused to serialize), thenstep_failedcarrying the dehydratedSerializationError— the same two-sequential-createGuardedshape as the hook token-group opscreateBatchfan-out fold (which only accepts creation events); healthy sibling steps proceed normallyfailedStepCorrelationIdsresult field; the runtime then forces an in-process replay over the reloaded log (nothing else would re-invoke the run when the failed step was the only pending work), demoting any retained VM session to a cold replayEntityConflictError/RunExpiredErroron both writes (concurrent handlers race toward the same deterministic failure)SerializationErrorfailures (e.g.RuntimeDecryptionError) keep their current behavior andRUNTIME_ERRORidentityResulting semantics:
step_failedconsumer; atry/catcharound theawait step(...)observes theSerializationError(e.g. to report to Sentry from application code)USER_ERROR— no queue-redelivery retry loopTests
suspension-handler.test.ts): single bad step, bad + healthy siblings, batched fan-out drop-out, error payload round-trip viahydrateStepError, concurrent-finalization conflict tolerance, run-expired skip99_e2e.ts, symlinked to all workbenches): caught/uncaught × step-args/step-return-value — the return-value side already worked but had no e2e coverage. The uncaught-args test is the regression check for the production failure mode (assertsUSER_ERROR, notMAX_DELIVERIES_EXCEEDED, well within a 60s timeout)Verified locally: full
packages/coreunit suite (2185 passed), and e2e against a localnextjs-turbopackdev server (serialization failure,FatalError, andparallelslices all green).Backport
This is a stability fix and should backport to
stable(v4) — that's the branch the reporting customer is on. Stable has drifted here (noSerializationErrorclass, older suspension handler), so the backport will likely need conflict resolution; if the catchable behavior doesn't port cleanly, the minimal stable fix is to treat serialization errors as fatal in stable's suspension catch so runs fail fast instead of burning 48 deliveries.Follow-up
A separate PR will add user-registerable workflow lifecycle hooks (
onRunCompleted/onRunFailedpassing the lazily-hydratedRuninstance and, for failures, the hydrated fatal error with its error code) so runtime-level failures can be reported to e.g. Sentry from a central place such asinstrumentation.ts.Docs Preview
(The preview deployment sits behind deployment protection, so the links require Vercel team access.)