Uh oh!
There was an error while loading. Please reload this page.
Backport #3675: fix(core): make step-argument serialization failures catchable in workflow code - #3687
Merged
Merged
Conversation
…kflow code (#3675) * fix(core): make step-argument serialization failures catchable in workflow 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. * Serialize the step_failed error with the VM global; one-sentence changeset 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. * Address review: QuickJS engine support, deferred-batch join, drain gate, placeholder marker, telemetry, docs - QuickJS: dumpPendingOps now catches a step input's serialization failure per-op, reframes it as a SerializationError with the same framed message as dehydrateStepArguments, and surfaces it on the pending op instead of failing the whole collection. The entrypoint's dispatchPendingOps finalizes such steps as step_created (placeholder input) + step_failed, excludes them from inline claims and queue publishes, marks them handled, and raises the requeue signal so the failure is observed even when the feed lags — mirroring the node:vm engine, so both engines agree: catchable in workflow code, USER_ERROR with the framed message when uncaught. Both step-argument e2e tests now pass on WORKFLOW_VM=quickjs. - runtime.ts: the failed-step replay path now joins suspensionResult.deferredBatchWork before continuing, so a trailing chunk commit or step-message publish rejection propagates instead of being swallowed after ack; committed inline claims are documented as deliberately handed to owned recovery. - Terminal drain: finalization is gated on a stepDispatch target. The drain caller has no replay to observe a finalization, so a completed run no longer gains failed-step rows for an unawaited unserializable step — the rethrown error is swallowed by the drain's catch, preserving its pre-existing behavior. - The placeholder input now carries a marker string ('[input unavailable: step argument serialization failed]', shared via runtime/unserializable-step.ts) so inspect/o11y don't render the failed step as a genuine zero-argument call. - New workflow.steps.failed_serialization span attribute on the suspension span, so occurrence is measurable without log search. - Docs: v5 serialization-failed error page documents where each boundary's failure surfaces (catchable step failure vs run failure) and the no-retry USER_ERROR semantics; foundations/errors-and-retries gains a Serialization Failures section with the try/catch shape. * Guard the finalization crash window; self-contained docs samples - 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. Signed-off-by: Nathan Rajlich <n@n8.io>
🦋 Changeset detectedLatest commit: 6ff2141 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 |
Contributor
TooTallNate
left a comment
Member
There was a problem hiding this comment.
Reviewed thoroughly for correctness and faithfulness to #3675 (original author here). No issues found — the AI port is a correct v4 rewrite of the fix. Verified locally on the branch: full workspace build, all 844 packages/core unit tests (including the 12 ported tests in suspension-handler.test.ts / step-handler.test.ts), and e2e against a local nextjs-turbopack dev server — both new serialization failure tests plus FatalError/parallel smoke slices green.
Faithful core semantics, correctly translated to stable's architecture:
handleSuspensioncatches the per-step dehydration failure (everything exceptRuntimeDecryptionError— correct on v4, wheredehydrateStepArgumentswraps all non-decryption throws in aWorkflowRuntimeErrorwith the framedFailed to serialize step arguments…message) and finalizes the step asstep_created(marker placeholder + structural flag) +step_failed, toleratingEntityConflictError/RunExpiredErroron both writes.- The
step_failedpayload uses v4's event shape ({ error: message, stack }string form), which the v4 step consumer rejects into workflow code as aFatalErrorcarrying the framed message — the right v4 analog of main's hydratedSerializationError(which doesn't exist on this branch). - Replay-to-observe uses
{ timeoutSeconds: 0 }(immediate queue re-invocation) instead of main's in-process replay — matching stable's ownPreconditionFailedErroridiom, and the caller atruntime.tspropagates it. When the failed step was the only pending work, this is what wakes the run. - The crash-window guard landed in
step-handler.ts(v4's executor): the structural placeholder flag check fires after hydration, throwsFatalError→ the existing fatal path writesstep_failedwithout running the body and re-queues the workflow. Important detail the port got right: on redelivery after an interrupted finalization, stable's suspension handler queues the already-created step unconditionally, so the guard is the recovery mechanism — and it's covered by both halves of the regression tests (interruption → suspension rejects; redelivery → body never runs), plus the negative test (a genuine argument equal to the display marker executes normally).
Deliberate scope reductions, all correct for stable: no QuickJS engine, no batch-fanout/lazy-inline/deferredBatchWork handling, and no drain gate (none of those exist on this branch — handleSuspension has exactly one caller). Step-return-value e2e was dropped because v4 retries those before step_failed (different semantics than main's fatal-skip); the docs correctly omit that claim and use message-based detection + FatalError framing instead of the v5-only SerializationError name.
One pre-existing note (not this PR):packages/docs-typecheck has 12 failing tests on origin/stable today (nitro ModuleOptions.sourcemap, world.streams samples). This PR's two new doc samples typecheck cleanly; the totals go 463→465 passed with the same 12 pre-existing failures.
ContributorAuthor
🧪 E2E Test Results❌ Some tests failed Summary
❌ Failed Tests🌍 Community Worlds (106 failed)redis (21 failed):
turso (85 failed):
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
❌ 🌍 Community Worlds
✅ 📋 Other
|
TooTallNate
approved these changes
Aug 20, 2026
TooTallNate
enabled auto-merge (squash)
August 20, 2026 18:43
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated backport of #3675 to
stable(backport job run).AI recommendation: This is a stability fix for a defect that exists on
stable: a step whose arguments fail to serialize fails the run from outside the workflow, unobservable to user code, and on v4 the orchestrator message redelivers until max-deliveries exhaustion — the production failure a customer onstablereported. The non-test changes are confined topackages/coreruntime paths plus adocs/content/correction, all of which are maintained onstable, and the only additive bits (a telemetry attribute, e2e fixtures) are incidental to the fix. Note for the reviewer:stablehas drifted substantially here — noSerializationErrorclass inpackages/errors, an oldersuspension-handler.ts, and nostep-executor.tsor QuickJS engine files at all — so most of this diff has no target and the port will be a rewrite of the same fix rather than a cherry-pick.Merge conflicts were resolved by AI (opencode with
anthropic/claude-opus-5). Please review the conflict resolution carefully before merging.