Skip to content

Fix dangling stream readers, serialization bugs, and refactor abort reducer code - #1647

Merged
pranaygp merged 10 commits into
pgp/serialize-abort-signalfrom
karthik/abort-signal-updates
May 2, 2026
Merged

Fix dangling stream readers, serialization bugs, and refactor abort reducer code#1647
pranaygp merged 10 commits into
pgp/serialize-abort-signalfrom
karthik/abort-signal-updates

Conversation

@karthikscale3

@karthikscale3karthikscale3 commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Description

Follow-up to #1301 (AbortController/AbortSignal serialization). Fixes bugs uncovered while exercising the end-to-end path and cleans up duplicated reducer code.

What changed

Bug fixes:

  • Dangling stream reader hangreviveAbortController starts a stream reader that blocks on reader.read() indefinitely if no abort arrives, keeping the serverless function alive until the platform timeout. Introduced an ABORT_READER_CANCEL symbol stored on the controller/signal and an AbortController the reader races against. The new cancelAbortReaders(...args, thisVal, closureVars) helper walks step arguments after the step function returns (success or failure) and aborts every reader, letting Promise.race resolve cleanly.
  • Request.signal serialization — The Request reducer previously serialized any signal that was aborted or had ABORT_STREAM_NAME set, which caused plain native signals (e.g. from fetch timeouts or user AbortControllers passed via new Request(url, { signal })) to inherit stream+hook infrastructure they did not need. The reducer now only serializes signals tagged with ABORT_STREAM_NAME, so plain native signals are silently stripped during Request serialization.
  • Missing is_system migration — Added the 0010_add_is_system.sql Drizzle migration (workflow_hooks.is_system boolean default false), updated the journal, and plumbed isSystem through the hook_created event in suspension-handler, world-postgres, and world-local so system hooks (currently only the abort hook) persist correctly and do not collide with user hook tokens.
  • getAbortStreamIdFromToken helper — Replaced the fragile queueItem.token.replace('abrt_', '') + manual strm_${id}_system_abort concat in suspension-handler with a shared helper in util.ts that validates the token prefix and reuses getAbortStreamId. Eliminates a latent bug where a token without the abrt_ prefix would silently produce a garbage stream name.
  • DurableAgent timeout path in workflow VM — PR feat: serializable AbortController/AbortSignal #1301 added AbortController to the workflow VM globals, which flipped the existing typeof AbortController !== 'undefined' guard in DurableAgent.generate/.stream to true inside workflows. That caused the timeout block to call setTimeout, which the VM traps, crashing any DurableAgent call from a workflow with a timeout option. Added an inWorkflowVm check based on the presence of Symbol.for('WORKFLOW_CONTEXT') on globalThis (set by the workflow runtime before user code runs). When true, the timeout block is skipped, restoring the pre-feat: serializable AbortController/AbortSignal #1301 behavior (timeouts silently do not fire in workflows; use sleep + abort() for durable time bounds).
  • onabort setter on WorkflowAbortSignal — The VM-side signal only exposed listener-based abort hooks. Added an onabort getter/setter that matches the native AbortSignal contract, fires the handler immediately when assigned if the signal is already aborted, and is invoked by _setAborted alongside the listener list.

Refactors:

  • Deduplicated ~180 lines of reducer code — Extracted reduceAbortWithListener() (symbol mint + listener attach + serialize) and reduceAbortBySymbol() (read existing symbols + serialize) helpers from the three near-identical AbortController/AbortSignal reducer implementations in getExternalReducers, getWorkflowReducers, and getStepReducers. Introduced AbortInternals, AbortSignalLike, and AbortHolder types so the shared helpers work for both controllers and standalone signals.
  • setupAbortStreamReader + tagAbortPair helpers — Extracted the stream-reader wiring and symbol-stamping into focused helpers used by both reviveAbortController and the new reviveAbortSignal.
  • Signal-only revival path — Added reviveAbortSignal() used by getStepRevivers.AbortSignal and getExternalRevivers.AbortSignal. Previously those revived through reviveAbortController(...).signal, which installed the patched abort() method even though nothing could reach the controller. The dedicated path skips the patch overhead and, crucially, still stores ABORT_READER_CANCEL on the returned signal so cancelAbortReaders can clean up standalone-signal readers (fixing a latent leak).

Tests:

  • Added test covering stream reader propagation with a targeted getWorld mock that delivers an actual abort payload (the default mock closes the stream immediately and would mask the code path).
  • Added test verifying plain (non-workflow) Request.signal is stripped during serialization — the hydrated Request gets a fresh default signal.
  • Rewrote the existing "Request with signal round-trip" test to exercise the step hydration path and explicitly tag request.signal with ABORT_STREAM_NAME/ABORT_HOOK_TOKEN (the Request constructor clones the signal, so symbols from the source controller do not transfer automatically).
  • Updated step-handler.test.ts mock to include cancelAbortReaders.

@vercel

vercelBot commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
example-nextjs-workflow-turbopackErrorErrorMay 1, 2026 9:17pm
example-nextjs-workflow-webpackErrorErrorMay 1, 2026 9:17pm
example-workflowErrorErrorMay 1, 2026 9:17pm
workbench-astro-workflowErrorErrorMay 1, 2026 9:17pm
workbench-express-workflowErrorErrorMay 1, 2026 9:17pm
workbench-fastify-workflowErrorErrorMay 1, 2026 9:17pm
workbench-hono-workflowErrorErrorMay 1, 2026 9:17pm
workbench-nitro-workflowErrorErrorMay 1, 2026 9:17pm
workbench-nuxt-workflowErrorErrorMay 1, 2026 9:17pm
workbench-sveltekit-workflowErrorErrorMay 1, 2026 9:17pm
workbench-vite-workflowErrorErrorMay 1, 2026 9:17pm
workflow-docsErrorErrorMay 1, 2026 9:17pm
workflow-swc-playgroundErrorErrorMay 1, 2026 9:17pm
workflow-webErrorErrorMay 1, 2026 9:17pm

@changeset-bot

changeset-botBot commented Apr 7, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: ee30ed7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions

github-actionsBot commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

🧪 E2E Test Results

No test result files found.


Some E2E test jobs failed:

  • Vercel Prod: failure
  • Local Dev: failure
  • Local Prod: failure
  • Local Postgres: failure
  • Windows: failure

Check the workflow run for details.

@karthikscale3
karthikscale3 marked this pull request as ready for review April 7, 2026 21:10
@karthikscale3
karthikscale3 requested a review from a team as a code ownerApril 7, 2026 21:10
@karthikscale3
karthikscale3force-pushed the karthik/abort-signal-updates branch from 414942e to d786ebbCompareApril 21, 2026 19:13
@vercel

vercelBot commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

Deployment failed with the following error:

Vercel employees are required to have Two-Factor Authentication enabled.

View Documentation: https://vercel.com/docs/two-factor-authentication

Two leak paths the prior fix left uncovered:
- External signal aborted after serialization: verifies the listener
attached by reduceAbortWithListener actually fires and writes the
abort packet once the caller aborts later.
- Signal nested inside a Request: exposed a real leak. The Request
constructor copies the signal to an internal AbortSignal, so the
ABORT_READER_CANCEL symbol set by reviveAbortSignal never reached
request.signal, and cancelAbortReaders' walker had no Request case
so Object.values(request) returned []. Fixed both sides:
- Request reviver copies abort-internal symbols via copyAbortInternals
- Walker descends into Request.signal explicitly
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@VaguelySerious

Copy link
Copy Markdown
Member

Should probably be reviewed by Pranay and then merged into pgp/serialize-abort-signal before it's reviewed as a whole

Conflict resolution:
- packages/core/src/serialization.ts: kept PR's reviveAbortSignal() function
(PR #1647's signal-only revival path) alongside the base branch's
getCommonRevivers() function from the modular refactor
Co-authored-by: Cursor <cursoragent@cursor.com>
@pranaygp

Copy link
Copy Markdown
Contributor

I'm resolving conflicts and merging your commits into my base PR from CLI @karthikscale3, ty!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@karthikscale3@VaguelySerious@pranaygp
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Fix dangling stream readers, serialization bugs, and refactor abort reducer code by karthikscale3 · Pull Request #1647 · vercel/workflow · GitHub
Skip to content

Fix dangling stream readers, serialization bugs, and refactor abort reducer code - #1647

Merged
pranaygp merged 10 commits into
pgp/serialize-abort-signalfrom
karthik/abort-signal-updates
May 2, 2026
Merged

Fix dangling stream readers, serialization bugs, and refactor abort reducer code#1647
pranaygp merged 10 commits into
pgp/serialize-abort-signalfrom
karthik/abort-signal-updates

Conversation

@karthikscale3

@karthikscale3karthikscale3 commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Description

Follow-up to #1301 (AbortController/AbortSignal serialization). Fixes bugs uncovered while exercising the end-to-end path and cleans up duplicated reducer code.

What changed

Bug fixes:

  • Dangling stream reader hangreviveAbortController starts a stream reader that blocks on reader.read() indefinitely if no abort arrives, keeping the serverless function alive until the platform timeout. Introduced an ABORT_READER_CANCEL symbol stored on the controller/signal and an AbortController the reader races against. The new cancelAbortReaders(...args, thisVal, closureVars) helper walks step arguments after the step function returns (success or failure) and aborts every reader, letting Promise.race resolve cleanly.
  • Request.signal serialization — The Request reducer previously serialized any signal that was aborted or had ABORT_STREAM_NAME set, which caused plain native signals (e.g. from fetch timeouts or user AbortControllers passed via new Request(url, { signal })) to inherit stream+hook infrastructure they did not need. The reducer now only serializes signals tagged with ABORT_STREAM_NAME, so plain native signals are silently stripped during Request serialization.
  • Missing is_system migration — Added the 0010_add_is_system.sql Drizzle migration (workflow_hooks.is_system boolean default false), updated the journal, and plumbed isSystem through the hook_created event in suspension-handler, world-postgres, and world-local so system hooks (currently only the abort hook) persist correctly and do not collide with user hook tokens.
  • getAbortStreamIdFromToken helper — Replaced the fragile queueItem.token.replace('abrt_', '') + manual strm_${id}_system_abort concat in suspension-handler with a shared helper in util.ts that validates the token prefix and reuses getAbortStreamId. Eliminates a latent bug where a token without the abrt_ prefix would silently produce a garbage stream name.
  • DurableAgent timeout path in workflow VM — PR feat: serializable AbortController/AbortSignal #1301 added AbortController to the workflow VM globals, which flipped the existing typeof AbortController !== 'undefined' guard in DurableAgent.generate/.stream to true inside workflows. That caused the timeout block to call setTimeout, which the VM traps, crashing any DurableAgent call from a workflow with a timeout option. Added an inWorkflowVm check based on the presence of Symbol.for('WORKFLOW_CONTEXT') on globalThis (set by the workflow runtime before user code runs). When true, the timeout block is skipped, restoring the pre-feat: serializable AbortController/AbortSignal #1301 behavior (timeouts silently do not fire in workflows; use sleep + abort() for durable time bounds).
  • onabort setter on WorkflowAbortSignal — The VM-side signal only exposed listener-based abort hooks. Added an onabort getter/setter that matches the native AbortSignal contract, fires the handler immediately when assigned if the signal is already aborted, and is invoked by _setAborted alongside the listener list.

Refactors:

  • Deduplicated ~180 lines of reducer code — Extracted reduceAbortWithListener() (symbol mint + listener attach + serialize) and reduceAbortBySymbol() (read existing symbols + serialize) helpers from the three near-identical AbortController/AbortSignal reducer implementations in getExternalReducers, getWorkflowReducers, and getStepReducers. Introduced AbortInternals, AbortSignalLike, and AbortHolder types so the shared helpers work for both controllers and standalone signals.
  • setupAbortStreamReader + tagAbortPair helpers — Extracted the stream-reader wiring and symbol-stamping into focused helpers used by both reviveAbortController and the new reviveAbortSignal.
  • Signal-only revival path — Added reviveAbortSignal() used by getStepRevivers.AbortSignal and getExternalRevivers.AbortSignal. Previously those revived through reviveAbortController(...).signal, which installed the patched abort() method even though nothing could reach the controller. The dedicated path skips the patch overhead and, crucially, still stores ABORT_READER_CANCEL on the returned signal so cancelAbortReaders can clean up standalone-signal readers (fixing a latent leak).

Tests:

  • Added test covering stream reader propagation with a targeted getWorld mock that delivers an actual abort payload (the default mock closes the stream immediately and would mask the code path).
  • Added test verifying plain (non-workflow) Request.signal is stripped during serialization — the hydrated Request gets a fresh default signal.
  • Rewrote the existing "Request with signal round-trip" test to exercise the step hydration path and explicitly tag request.signal with ABORT_STREAM_NAME/ABORT_HOOK_TOKEN (the Request constructor clones the signal, so symbols from the source controller do not transfer automatically).
  • Updated step-handler.test.ts mock to include cancelAbortReaders.

@vercel

vercelBot commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
example-nextjs-workflow-turbopackErrorErrorMay 1, 2026 9:17pm
example-nextjs-workflow-webpackErrorErrorMay 1, 2026 9:17pm
example-workflowErrorErrorMay 1, 2026 9:17pm
workbench-astro-workflowErrorErrorMay 1, 2026 9:17pm
workbench-express-workflowErrorErrorMay 1, 2026 9:17pm
workbench-fastify-workflowErrorErrorMay 1, 2026 9:17pm
workbench-hono-workflowErrorErrorMay 1, 2026 9:17pm
workbench-nitro-workflowErrorErrorMay 1, 2026 9:17pm
workbench-nuxt-workflowErrorErrorMay 1, 2026 9:17pm
workbench-sveltekit-workflowErrorErrorMay 1, 2026 9:17pm
workbench-vite-workflowErrorErrorMay 1, 2026 9:17pm
workflow-docsErrorErrorMay 1, 2026 9:17pm
workflow-swc-playgroundErrorErrorMay 1, 2026 9:17pm
workflow-webErrorErrorMay 1, 2026 9:17pm

@changeset-bot

changeset-botBot commented Apr 7, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: ee30ed7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions

github-actionsBot commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

🧪 E2E Test Results

No test result files found.


Some E2E test jobs failed:

  • Vercel Prod: failure
  • Local Dev: failure
  • Local Prod: failure
  • Local Postgres: failure
  • Windows: failure

Check the workflow run for details.

@karthikscale3
karthikscale3 marked this pull request as ready for review April 7, 2026 21:10
@karthikscale3
karthikscale3 requested a review from a team as a code ownerApril 7, 2026 21:10
@karthikscale3
karthikscale3force-pushed the karthik/abort-signal-updates branch from 414942e to d786ebbCompareApril 21, 2026 19:13
@vercel

vercelBot commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

Deployment failed with the following error:

Vercel employees are required to have Two-Factor Authentication enabled.

View Documentation: https://vercel.com/docs/two-factor-authentication

Two leak paths the prior fix left uncovered:
- External signal aborted after serialization: verifies the listener
attached by reduceAbortWithListener actually fires and writes the
abort packet once the caller aborts later.
- Signal nested inside a Request: exposed a real leak. The Request
constructor copies the signal to an internal AbortSignal, so the
ABORT_READER_CANCEL symbol set by reviveAbortSignal never reached
request.signal, and cancelAbortReaders' walker had no Request case
so Object.values(request) returned []. Fixed both sides:
- Request reviver copies abort-internal symbols via copyAbortInternals
- Walker descends into Request.signal explicitly
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@VaguelySerious

Copy link
Copy Markdown
Member

Should probably be reviewed by Pranay and then merged into pgp/serialize-abort-signal before it's reviewed as a whole

Conflict resolution:
- packages/core/src/serialization.ts: kept PR's reviveAbortSignal() function
(PR #1647's signal-only revival path) alongside the base branch's
getCommonRevivers() function from the modular refactor
Co-authored-by: Cursor <cursoragent@cursor.com>
@pranaygp

Copy link
Copy Markdown
Contributor

I'm resolving conflicts and merging your commits into my base PR from CLI @karthikscale3, ty!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@karthikscale3@VaguelySerious@pranaygp
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Fix dangling stream readers, serialization bugs, and refactor abort reducer code by karthikscale3 · Pull Request #1647 · vercel/workflow · GitHub
Skip to content

Fix dangling stream readers, serialization bugs, and refactor abort reducer code - #1647

Merged
pranaygp merged 10 commits into
pgp/serialize-abort-signalfrom
karthik/abort-signal-updates
May 2, 2026
Merged

Fix dangling stream readers, serialization bugs, and refactor abort reducer code#1647
pranaygp merged 10 commits into
pgp/serialize-abort-signalfrom
karthik/abort-signal-updates

Conversation

@karthikscale3

@karthikscale3karthikscale3 commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Description

Follow-up to #1301 (AbortController/AbortSignal serialization). Fixes bugs uncovered while exercising the end-to-end path and cleans up duplicated reducer code.

What changed

Bug fixes:

  • Dangling stream reader hangreviveAbortController starts a stream reader that blocks on reader.read() indefinitely if no abort arrives, keeping the serverless function alive until the platform timeout. Introduced an ABORT_READER_CANCEL symbol stored on the controller/signal and an AbortController the reader races against. The new cancelAbortReaders(...args, thisVal, closureVars) helper walks step arguments after the step function returns (success or failure) and aborts every reader, letting Promise.race resolve cleanly.
  • Request.signal serialization — The Request reducer previously serialized any signal that was aborted or had ABORT_STREAM_NAME set, which caused plain native signals (e.g. from fetch timeouts or user AbortControllers passed via new Request(url, { signal })) to inherit stream+hook infrastructure they did not need. The reducer now only serializes signals tagged with ABORT_STREAM_NAME, so plain native signals are silently stripped during Request serialization.
  • Missing is_system migration — Added the 0010_add_is_system.sql Drizzle migration (workflow_hooks.is_system boolean default false), updated the journal, and plumbed isSystem through the hook_created event in suspension-handler, world-postgres, and world-local so system hooks (currently only the abort hook) persist correctly and do not collide with user hook tokens.
  • getAbortStreamIdFromToken helper — Replaced the fragile queueItem.token.replace('abrt_', '') + manual strm_${id}_system_abort concat in suspension-handler with a shared helper in util.ts that validates the token prefix and reuses getAbortStreamId. Eliminates a latent bug where a token without the abrt_ prefix would silently produce a garbage stream name.
  • DurableAgent timeout path in workflow VM — PR feat: serializable AbortController/AbortSignal #1301 added AbortController to the workflow VM globals, which flipped the existing typeof AbortController !== 'undefined' guard in DurableAgent.generate/.stream to true inside workflows. That caused the timeout block to call setTimeout, which the VM traps, crashing any DurableAgent call from a workflow with a timeout option. Added an inWorkflowVm check based on the presence of Symbol.for('WORKFLOW_CONTEXT') on globalThis (set by the workflow runtime before user code runs). When true, the timeout block is skipped, restoring the pre-feat: serializable AbortController/AbortSignal #1301 behavior (timeouts silently do not fire in workflows; use sleep + abort() for durable time bounds).
  • onabort setter on WorkflowAbortSignal — The VM-side signal only exposed listener-based abort hooks. Added an onabort getter/setter that matches the native AbortSignal contract, fires the handler immediately when assigned if the signal is already aborted, and is invoked by _setAborted alongside the listener list.

Refactors:

  • Deduplicated ~180 lines of reducer code — Extracted reduceAbortWithListener() (symbol mint + listener attach + serialize) and reduceAbortBySymbol() (read existing symbols + serialize) helpers from the three near-identical AbortController/AbortSignal reducer implementations in getExternalReducers, getWorkflowReducers, and getStepReducers. Introduced AbortInternals, AbortSignalLike, and AbortHolder types so the shared helpers work for both controllers and standalone signals.
  • setupAbortStreamReader + tagAbortPair helpers — Extracted the stream-reader wiring and symbol-stamping into focused helpers used by both reviveAbortController and the new reviveAbortSignal.
  • Signal-only revival path — Added reviveAbortSignal() used by getStepRevivers.AbortSignal and getExternalRevivers.AbortSignal. Previously those revived through reviveAbortController(...).signal, which installed the patched abort() method even though nothing could reach the controller. The dedicated path skips the patch overhead and, crucially, still stores ABORT_READER_CANCEL on the returned signal so cancelAbortReaders can clean up standalone-signal readers (fixing a latent leak).

Tests:

  • Added test covering stream reader propagation with a targeted getWorld mock that delivers an actual abort payload (the default mock closes the stream immediately and would mask the code path).
  • Added test verifying plain (non-workflow) Request.signal is stripped during serialization — the hydrated Request gets a fresh default signal.
  • Rewrote the existing "Request with signal round-trip" test to exercise the step hydration path and explicitly tag request.signal with ABORT_STREAM_NAME/ABORT_HOOK_TOKEN (the Request constructor clones the signal, so symbols from the source controller do not transfer automatically).
  • Updated step-handler.test.ts mock to include cancelAbortReaders.

@vercel

vercelBot commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
example-nextjs-workflow-turbopackErrorErrorMay 1, 2026 9:17pm
example-nextjs-workflow-webpackErrorErrorMay 1, 2026 9:17pm
example-workflowErrorErrorMay 1, 2026 9:17pm
workbench-astro-workflowErrorErrorMay 1, 2026 9:17pm
workbench-express-workflowErrorErrorMay 1, 2026 9:17pm
workbench-fastify-workflowErrorErrorMay 1, 2026 9:17pm
workbench-hono-workflowErrorErrorMay 1, 2026 9:17pm
workbench-nitro-workflowErrorErrorMay 1, 2026 9:17pm
workbench-nuxt-workflowErrorErrorMay 1, 2026 9:17pm
workbench-sveltekit-workflowErrorErrorMay 1, 2026 9:17pm
workbench-vite-workflowErrorErrorMay 1, 2026 9:17pm
workflow-docsErrorErrorMay 1, 2026 9:17pm
workflow-swc-playgroundErrorErrorMay 1, 2026 9:17pm
workflow-webErrorErrorMay 1, 2026 9:17pm

@changeset-bot

changeset-botBot commented Apr 7, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: ee30ed7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions

github-actionsBot commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

🧪 E2E Test Results

No test result files found.


Some E2E test jobs failed:

  • Vercel Prod: failure
  • Local Dev: failure
  • Local Prod: failure
  • Local Postgres: failure
  • Windows: failure

Check the workflow run for details.

@karthikscale3
karthikscale3 marked this pull request as ready for review April 7, 2026 21:10
@karthikscale3
karthikscale3 requested a review from a team as a code ownerApril 7, 2026 21:10
@karthikscale3
karthikscale3force-pushed the karthik/abort-signal-updates branch from 414942e to d786ebbCompareApril 21, 2026 19:13
@vercel

vercelBot commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

Deployment failed with the following error:

Vercel employees are required to have Two-Factor Authentication enabled.

View Documentation: https://vercel.com/docs/two-factor-authentication

Two leak paths the prior fix left uncovered:
- External signal aborted after serialization: verifies the listener
attached by reduceAbortWithListener actually fires and writes the
abort packet once the caller aborts later.
- Signal nested inside a Request: exposed a real leak. The Request
constructor copies the signal to an internal AbortSignal, so the
ABORT_READER_CANCEL symbol set by reviveAbortSignal never reached
request.signal, and cancelAbortReaders' walker had no Request case
so Object.values(request) returned []. Fixed both sides:
- Request reviver copies abort-internal symbols via copyAbortInternals
- Walker descends into Request.signal explicitly
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@VaguelySerious

Copy link
Copy Markdown
Member

Should probably be reviewed by Pranay and then merged into pgp/serialize-abort-signal before it's reviewed as a whole

Conflict resolution:
- packages/core/src/serialization.ts: kept PR's reviveAbortSignal() function
(PR #1647's signal-only revival path) alongside the base branch's
getCommonRevivers() function from the modular refactor
Co-authored-by: Cursor <cursoragent@cursor.com>
@pranaygp

Copy link
Copy Markdown
Contributor

I'm resolving conflicts and merging your commits into my base PR from CLI @karthikscale3, ty!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@karthikscale3@VaguelySerious@pranaygp
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Fix dangling stream readers, serialization bugs, and refactor abort reducer code by karthikscale3 · Pull Request #1647 · vercel/workflow · GitHub
Skip to content

Fix dangling stream readers, serialization bugs, and refactor abort reducer code - #1647

Merged
pranaygp merged 10 commits into
pgp/serialize-abort-signalfrom
karthik/abort-signal-updates
May 2, 2026
Merged

Fix dangling stream readers, serialization bugs, and refactor abort reducer code#1647
pranaygp merged 10 commits into
pgp/serialize-abort-signalfrom
karthik/abort-signal-updates

Conversation

@karthikscale3

@karthikscale3karthikscale3 commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Description

Follow-up to #1301 (AbortController/AbortSignal serialization). Fixes bugs uncovered while exercising the end-to-end path and cleans up duplicated reducer code.

What changed

Bug fixes:

  • Dangling stream reader hangreviveAbortController starts a stream reader that blocks on reader.read() indefinitely if no abort arrives, keeping the serverless function alive until the platform timeout. Introduced an ABORT_READER_CANCEL symbol stored on the controller/signal and an AbortController the reader races against. The new cancelAbortReaders(...args, thisVal, closureVars) helper walks step arguments after the step function returns (success or failure) and aborts every reader, letting Promise.race resolve cleanly.
  • Request.signal serialization — The Request reducer previously serialized any signal that was aborted or had ABORT_STREAM_NAME set, which caused plain native signals (e.g. from fetch timeouts or user AbortControllers passed via new Request(url, { signal })) to inherit stream+hook infrastructure they did not need. The reducer now only serializes signals tagged with ABORT_STREAM_NAME, so plain native signals are silently stripped during Request serialization.
  • Missing is_system migration — Added the 0010_add_is_system.sql Drizzle migration (workflow_hooks.is_system boolean default false), updated the journal, and plumbed isSystem through the hook_created event in suspension-handler, world-postgres, and world-local so system hooks (currently only the abort hook) persist correctly and do not collide with user hook tokens.
  • getAbortStreamIdFromToken helper — Replaced the fragile queueItem.token.replace('abrt_', '') + manual strm_${id}_system_abort concat in suspension-handler with a shared helper in util.ts that validates the token prefix and reuses getAbortStreamId. Eliminates a latent bug where a token without the abrt_ prefix would silently produce a garbage stream name.
  • DurableAgent timeout path in workflow VM — PR feat: serializable AbortController/AbortSignal #1301 added AbortController to the workflow VM globals, which flipped the existing typeof AbortController !== 'undefined' guard in DurableAgent.generate/.stream to true inside workflows. That caused the timeout block to call setTimeout, which the VM traps, crashing any DurableAgent call from a workflow with a timeout option. Added an inWorkflowVm check based on the presence of Symbol.for('WORKFLOW_CONTEXT') on globalThis (set by the workflow runtime before user code runs). When true, the timeout block is skipped, restoring the pre-feat: serializable AbortController/AbortSignal #1301 behavior (timeouts silently do not fire in workflows; use sleep + abort() for durable time bounds).
  • onabort setter on WorkflowAbortSignal — The VM-side signal only exposed listener-based abort hooks. Added an onabort getter/setter that matches the native AbortSignal contract, fires the handler immediately when assigned if the signal is already aborted, and is invoked by _setAborted alongside the listener list.

Refactors:

  • Deduplicated ~180 lines of reducer code — Extracted reduceAbortWithListener() (symbol mint + listener attach + serialize) and reduceAbortBySymbol() (read existing symbols + serialize) helpers from the three near-identical AbortController/AbortSignal reducer implementations in getExternalReducers, getWorkflowReducers, and getStepReducers. Introduced AbortInternals, AbortSignalLike, and AbortHolder types so the shared helpers work for both controllers and standalone signals.
  • setupAbortStreamReader + tagAbortPair helpers — Extracted the stream-reader wiring and symbol-stamping into focused helpers used by both reviveAbortController and the new reviveAbortSignal.
  • Signal-only revival path — Added reviveAbortSignal() used by getStepRevivers.AbortSignal and getExternalRevivers.AbortSignal. Previously those revived through reviveAbortController(...).signal, which installed the patched abort() method even though nothing could reach the controller. The dedicated path skips the patch overhead and, crucially, still stores ABORT_READER_CANCEL on the returned signal so cancelAbortReaders can clean up standalone-signal readers (fixing a latent leak).

Tests:

  • Added test covering stream reader propagation with a targeted getWorld mock that delivers an actual abort payload (the default mock closes the stream immediately and would mask the code path).
  • Added test verifying plain (non-workflow) Request.signal is stripped during serialization — the hydrated Request gets a fresh default signal.
  • Rewrote the existing "Request with signal round-trip" test to exercise the step hydration path and explicitly tag request.signal with ABORT_STREAM_NAME/ABORT_HOOK_TOKEN (the Request constructor clones the signal, so symbols from the source controller do not transfer automatically).
  • Updated step-handler.test.ts mock to include cancelAbortReaders.

@vercel

vercelBot commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
example-nextjs-workflow-turbopackErrorErrorMay 1, 2026 9:17pm
example-nextjs-workflow-webpackErrorErrorMay 1, 2026 9:17pm
example-workflowErrorErrorMay 1, 2026 9:17pm
workbench-astro-workflowErrorErrorMay 1, 2026 9:17pm
workbench-express-workflowErrorErrorMay 1, 2026 9:17pm
workbench-fastify-workflowErrorErrorMay 1, 2026 9:17pm
workbench-hono-workflowErrorErrorMay 1, 2026 9:17pm
workbench-nitro-workflowErrorErrorMay 1, 2026 9:17pm
workbench-nuxt-workflowErrorErrorMay 1, 2026 9:17pm
workbench-sveltekit-workflowErrorErrorMay 1, 2026 9:17pm
workbench-vite-workflowErrorErrorMay 1, 2026 9:17pm
workflow-docsErrorErrorMay 1, 2026 9:17pm
workflow-swc-playgroundErrorErrorMay 1, 2026 9:17pm
workflow-webErrorErrorMay 1, 2026 9:17pm

@changeset-bot

changeset-botBot commented Apr 7, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: ee30ed7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions

github-actionsBot commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

🧪 E2E Test Results

No test result files found.


Some E2E test jobs failed:

  • Vercel Prod: failure
  • Local Dev: failure
  • Local Prod: failure
  • Local Postgres: failure
  • Windows: failure

Check the workflow run for details.

@karthikscale3
karthikscale3 marked this pull request as ready for review April 7, 2026 21:10
@karthikscale3
karthikscale3 requested a review from a team as a code ownerApril 7, 2026 21:10
@karthikscale3
karthikscale3force-pushed the karthik/abort-signal-updates branch from 414942e to d786ebbCompareApril 21, 2026 19:13
@vercel

vercelBot commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

Deployment failed with the following error:

Vercel employees are required to have Two-Factor Authentication enabled.

View Documentation: https://vercel.com/docs/two-factor-authentication

Two leak paths the prior fix left uncovered:
- External signal aborted after serialization: verifies the listener
attached by reduceAbortWithListener actually fires and writes the
abort packet once the caller aborts later.
- Signal nested inside a Request: exposed a real leak. The Request
constructor copies the signal to an internal AbortSignal, so the
ABORT_READER_CANCEL symbol set by reviveAbortSignal never reached
request.signal, and cancelAbortReaders' walker had no Request case
so Object.values(request) returned []. Fixed both sides:
- Request reviver copies abort-internal symbols via copyAbortInternals
- Walker descends into Request.signal explicitly
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@VaguelySerious

Copy link
Copy Markdown
Member

Should probably be reviewed by Pranay and then merged into pgp/serialize-abort-signal before it's reviewed as a whole

Conflict resolution:
- packages/core/src/serialization.ts: kept PR's reviveAbortSignal() function
(PR #1647's signal-only revival path) alongside the base branch's
getCommonRevivers() function from the modular refactor
Co-authored-by: Cursor <cursoragent@cursor.com>
@pranaygp

Copy link
Copy Markdown
Contributor

I'm resolving conflicts and merging your commits into my base PR from CLI @karthikscale3, ty!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@karthikscale3@VaguelySerious@pranaygp
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' Fix dangling stream readers, serialization bugs, and refactor abort reducer code by karthikscale3 · Pull Request #1647 · vercel/workflow · GitHub
Skip to content

Fix dangling stream readers, serialization bugs, and refactor abort reducer code - #1647

Merged
pranaygp merged 10 commits into
pgp/serialize-abort-signalfrom
karthik/abort-signal-updates
May 2, 2026
Merged

Fix dangling stream readers, serialization bugs, and refactor abort reducer code#1647
pranaygp merged 10 commits into
pgp/serialize-abort-signalfrom
karthik/abort-signal-updates

Conversation

@karthikscale3

@karthikscale3karthikscale3 commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Description

Follow-up to #1301 (AbortController/AbortSignal serialization). Fixes bugs uncovered while exercising the end-to-end path and cleans up duplicated reducer code.

What changed

Bug fixes:

  • Dangling stream reader hangreviveAbortController starts a stream reader that blocks on reader.read() indefinitely if no abort arrives, keeping the serverless function alive until the platform timeout. Introduced an ABORT_READER_CANCEL symbol stored on the controller/signal and an AbortController the reader races against. The new cancelAbortReaders(...args, thisVal, closureVars) helper walks step arguments after the step function returns (success or failure) and aborts every reader, letting Promise.race resolve cleanly.
  • Request.signal serialization — The Request reducer previously serialized any signal that was aborted or had ABORT_STREAM_NAME set, which caused plain native signals (e.g. from fetch timeouts or user AbortControllers passed via new Request(url, { signal })) to inherit stream+hook infrastructure they did not need. The reducer now only serializes signals tagged with ABORT_STREAM_NAME, so plain native signals are silently stripped during Request serialization.
  • Missing is_system migration — Added the 0010_add_is_system.sql Drizzle migration (workflow_hooks.is_system boolean default false), updated the journal, and plumbed isSystem through the hook_created event in suspension-handler, world-postgres, and world-local so system hooks (currently only the abort hook) persist correctly and do not collide with user hook tokens.
  • getAbortStreamIdFromToken helper — Replaced the fragile queueItem.token.replace('abrt_', '') + manual strm_${id}_system_abort concat in suspension-handler with a shared helper in util.ts that validates the token prefix and reuses getAbortStreamId. Eliminates a latent bug where a token without the abrt_ prefix would silently produce a garbage stream name.
  • DurableAgent timeout path in workflow VM — PR feat: serializable AbortController/AbortSignal #1301 added AbortController to the workflow VM globals, which flipped the existing typeof AbortController !== 'undefined' guard in DurableAgent.generate/.stream to true inside workflows. That caused the timeout block to call setTimeout, which the VM traps, crashing any DurableAgent call from a workflow with a timeout option. Added an inWorkflowVm check based on the presence of Symbol.for('WORKFLOW_CONTEXT') on globalThis (set by the workflow runtime before user code runs). When true, the timeout block is skipped, restoring the pre-feat: serializable AbortController/AbortSignal #1301 behavior (timeouts silently do not fire in workflows; use sleep + abort() for durable time bounds).
  • onabort setter on WorkflowAbortSignal — The VM-side signal only exposed listener-based abort hooks. Added an onabort getter/setter that matches the native AbortSignal contract, fires the handler immediately when assigned if the signal is already aborted, and is invoked by _setAborted alongside the listener list.

Refactors:

  • Deduplicated ~180 lines of reducer code — Extracted reduceAbortWithListener() (symbol mint + listener attach + serialize) and reduceAbortBySymbol() (read existing symbols + serialize) helpers from the three near-identical AbortController/AbortSignal reducer implementations in getExternalReducers, getWorkflowReducers, and getStepReducers. Introduced AbortInternals, AbortSignalLike, and AbortHolder types so the shared helpers work for both controllers and standalone signals.
  • setupAbortStreamReader + tagAbortPair helpers — Extracted the stream-reader wiring and symbol-stamping into focused helpers used by both reviveAbortController and the new reviveAbortSignal.
  • Signal-only revival path — Added reviveAbortSignal() used by getStepRevivers.AbortSignal and getExternalRevivers.AbortSignal. Previously those revived through reviveAbortController(...).signal, which installed the patched abort() method even though nothing could reach the controller. The dedicated path skips the patch overhead and, crucially, still stores ABORT_READER_CANCEL on the returned signal so cancelAbortReaders can clean up standalone-signal readers (fixing a latent leak).

Tests:

  • Added test covering stream reader propagation with a targeted getWorld mock that delivers an actual abort payload (the default mock closes the stream immediately and would mask the code path).
  • Added test verifying plain (non-workflow) Request.signal is stripped during serialization — the hydrated Request gets a fresh default signal.
  • Rewrote the existing "Request with signal round-trip" test to exercise the step hydration path and explicitly tag request.signal with ABORT_STREAM_NAME/ABORT_HOOK_TOKEN (the Request constructor clones the signal, so symbols from the source controller do not transfer automatically).
  • Updated step-handler.test.ts mock to include cancelAbortReaders.

@vercel

vercelBot commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
example-nextjs-workflow-turbopackErrorErrorMay 1, 2026 9:17pm
example-nextjs-workflow-webpackErrorErrorMay 1, 2026 9:17pm
example-workflowErrorErrorMay 1, 2026 9:17pm
workbench-astro-workflowErrorErrorMay 1, 2026 9:17pm
workbench-express-workflowErrorErrorMay 1, 2026 9:17pm
workbench-fastify-workflowErrorErrorMay 1, 2026 9:17pm
workbench-hono-workflowErrorErrorMay 1, 2026 9:17pm
workbench-nitro-workflowErrorErrorMay 1, 2026 9:17pm
workbench-nuxt-workflowErrorErrorMay 1, 2026 9:17pm
workbench-sveltekit-workflowErrorErrorMay 1, 2026 9:17pm
workbench-vite-workflowErrorErrorMay 1, 2026 9:17pm
workflow-docsErrorErrorMay 1, 2026 9:17pm
workflow-swc-playgroundErrorErrorMay 1, 2026 9:17pm
workflow-webErrorErrorMay 1, 2026 9:17pm

@changeset-bot

changeset-botBot commented Apr 7, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: ee30ed7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions

github-actionsBot commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

🧪 E2E Test Results

No test result files found.


Some E2E test jobs failed:

  • Vercel Prod: failure
  • Local Dev: failure
  • Local Prod: failure
  • Local Postgres: failure
  • Windows: failure

Check the workflow run for details.

@karthikscale3
karthikscale3 marked this pull request as ready for review April 7, 2026 21:10
@karthikscale3
karthikscale3 requested a review from a team as a code ownerApril 7, 2026 21:10
@karthikscale3
karthikscale3force-pushed the karthik/abort-signal-updates branch from 414942e to d786ebbCompareApril 21, 2026 19:13
@vercel

vercelBot commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

Deployment failed with the following error:

Vercel employees are required to have Two-Factor Authentication enabled.

View Documentation: https://vercel.com/docs/two-factor-authentication

Two leak paths the prior fix left uncovered:
- External signal aborted after serialization: verifies the listener
attached by reduceAbortWithListener actually fires and writes the
abort packet once the caller aborts later.
- Signal nested inside a Request: exposed a real leak. The Request
constructor copies the signal to an internal AbortSignal, so the
ABORT_READER_CANCEL symbol set by reviveAbortSignal never reached
request.signal, and cancelAbortReaders' walker had no Request case
so Object.values(request) returned []. Fixed both sides:
- Request reviver copies abort-internal symbols via copyAbortInternals
- Walker descends into Request.signal explicitly
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@VaguelySerious

Copy link
Copy Markdown
Member

Should probably be reviewed by Pranay and then merged into pgp/serialize-abort-signal before it's reviewed as a whole

Conflict resolution:
- packages/core/src/serialization.ts: kept PR's reviveAbortSignal() function
(PR #1647's signal-only revival path) alongside the base branch's
getCommonRevivers() function from the modular refactor
Co-authored-by: Cursor <cursoragent@cursor.com>
@pranaygp

Copy link
Copy Markdown
Contributor

I'm resolving conflicts and merging your commits into my base PR from CLI @karthikscale3, ty!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@karthikscale3@VaguelySerious@pranaygp
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Fix dangling stream readers, serialization bugs, and refactor abort reducer code by karthikscale3 · Pull Request #1647 · vercel/workflow · GitHub
Skip to content

Fix dangling stream readers, serialization bugs, and refactor abort reducer code - #1647

Merged
pranaygp merged 10 commits into
pgp/serialize-abort-signalfrom
karthik/abort-signal-updates
May 2, 2026
Merged

Fix dangling stream readers, serialization bugs, and refactor abort reducer code#1647
pranaygp merged 10 commits into
pgp/serialize-abort-signalfrom
karthik/abort-signal-updates

Conversation

@karthikscale3

@karthikscale3karthikscale3 commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Description

Follow-up to #1301 (AbortController/AbortSignal serialization). Fixes bugs uncovered while exercising the end-to-end path and cleans up duplicated reducer code.

What changed

Bug fixes:

  • Dangling stream reader hangreviveAbortController starts a stream reader that blocks on reader.read() indefinitely if no abort arrives, keeping the serverless function alive until the platform timeout. Introduced an ABORT_READER_CANCEL symbol stored on the controller/signal and an AbortController the reader races against. The new cancelAbortReaders(...args, thisVal, closureVars) helper walks step arguments after the step function returns (success or failure) and aborts every reader, letting Promise.race resolve cleanly.
  • Request.signal serialization — The Request reducer previously serialized any signal that was aborted or had ABORT_STREAM_NAME set, which caused plain native signals (e.g. from fetch timeouts or user AbortControllers passed via new Request(url, { signal })) to inherit stream+hook infrastructure they did not need. The reducer now only serializes signals tagged with ABORT_STREAM_NAME, so plain native signals are silently stripped during Request serialization.
  • Missing is_system migration — Added the 0010_add_is_system.sql Drizzle migration (workflow_hooks.is_system boolean default false), updated the journal, and plumbed isSystem through the hook_created event in suspension-handler, world-postgres, and world-local so system hooks (currently only the abort hook) persist correctly and do not collide with user hook tokens.
  • getAbortStreamIdFromToken helper — Replaced the fragile queueItem.token.replace('abrt_', '') + manual strm_${id}_system_abort concat in suspension-handler with a shared helper in util.ts that validates the token prefix and reuses getAbortStreamId. Eliminates a latent bug where a token without the abrt_ prefix would silently produce a garbage stream name.
  • DurableAgent timeout path in workflow VM — PR feat: serializable AbortController/AbortSignal #1301 added AbortController to the workflow VM globals, which flipped the existing typeof AbortController !== 'undefined' guard in DurableAgent.generate/.stream to true inside workflows. That caused the timeout block to call setTimeout, which the VM traps, crashing any DurableAgent call from a workflow with a timeout option. Added an inWorkflowVm check based on the presence of Symbol.for('WORKFLOW_CONTEXT') on globalThis (set by the workflow runtime before user code runs). When true, the timeout block is skipped, restoring the pre-feat: serializable AbortController/AbortSignal #1301 behavior (timeouts silently do not fire in workflows; use sleep + abort() for durable time bounds).
  • onabort setter on WorkflowAbortSignal — The VM-side signal only exposed listener-based abort hooks. Added an onabort getter/setter that matches the native AbortSignal contract, fires the handler immediately when assigned if the signal is already aborted, and is invoked by _setAborted alongside the listener list.

Refactors:

  • Deduplicated ~180 lines of reducer code — Extracted reduceAbortWithListener() (symbol mint + listener attach + serialize) and reduceAbortBySymbol() (read existing symbols + serialize) helpers from the three near-identical AbortController/AbortSignal reducer implementations in getExternalReducers, getWorkflowReducers, and getStepReducers. Introduced AbortInternals, AbortSignalLike, and AbortHolder types so the shared helpers work for both controllers and standalone signals.
  • setupAbortStreamReader + tagAbortPair helpers — Extracted the stream-reader wiring and symbol-stamping into focused helpers used by both reviveAbortController and the new reviveAbortSignal.
  • Signal-only revival path — Added reviveAbortSignal() used by getStepRevivers.AbortSignal and getExternalRevivers.AbortSignal. Previously those revived through reviveAbortController(...).signal, which installed the patched abort() method even though nothing could reach the controller. The dedicated path skips the patch overhead and, crucially, still stores ABORT_READER_CANCEL on the returned signal so cancelAbortReaders can clean up standalone-signal readers (fixing a latent leak).

Tests:

  • Added test covering stream reader propagation with a targeted getWorld mock that delivers an actual abort payload (the default mock closes the stream immediately and would mask the code path).
  • Added test verifying plain (non-workflow) Request.signal is stripped during serialization — the hydrated Request gets a fresh default signal.
  • Rewrote the existing "Request with signal round-trip" test to exercise the step hydration path and explicitly tag request.signal with ABORT_STREAM_NAME/ABORT_HOOK_TOKEN (the Request constructor clones the signal, so symbols from the source controller do not transfer automatically).
  • Updated step-handler.test.ts mock to include cancelAbortReaders.

@vercel

vercelBot commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
example-nextjs-workflow-turbopackErrorErrorMay 1, 2026 9:17pm
example-nextjs-workflow-webpackErrorErrorMay 1, 2026 9:17pm
example-workflowErrorErrorMay 1, 2026 9:17pm
workbench-astro-workflowErrorErrorMay 1, 2026 9:17pm
workbench-express-workflowErrorErrorMay 1, 2026 9:17pm
workbench-fastify-workflowErrorErrorMay 1, 2026 9:17pm
workbench-hono-workflowErrorErrorMay 1, 2026 9:17pm
workbench-nitro-workflowErrorErrorMay 1, 2026 9:17pm
workbench-nuxt-workflowErrorErrorMay 1, 2026 9:17pm
workbench-sveltekit-workflowErrorErrorMay 1, 2026 9:17pm
workbench-vite-workflowErrorErrorMay 1, 2026 9:17pm
workflow-docsErrorErrorMay 1, 2026 9:17pm
workflow-swc-playgroundErrorErrorMay 1, 2026 9:17pm
workflow-webErrorErrorMay 1, 2026 9:17pm

@changeset-bot

changeset-botBot commented Apr 7, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: ee30ed7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions

github-actionsBot commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

🧪 E2E Test Results

No test result files found.


Some E2E test jobs failed:

  • Vercel Prod: failure
  • Local Dev: failure
  • Local Prod: failure
  • Local Postgres: failure
  • Windows: failure

Check the workflow run for details.

@karthikscale3
karthikscale3 marked this pull request as ready for review April 7, 2026 21:10
@karthikscale3
karthikscale3 requested a review from a team as a code ownerApril 7, 2026 21:10
@karthikscale3
karthikscale3force-pushed the karthik/abort-signal-updates branch from 414942e to d786ebbCompareApril 21, 2026 19:13
@vercel

vercelBot commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

Deployment failed with the following error:

Vercel employees are required to have Two-Factor Authentication enabled.

View Documentation: https://vercel.com/docs/two-factor-authentication

Two leak paths the prior fix left uncovered:
- External signal aborted after serialization: verifies the listener
attached by reduceAbortWithListener actually fires and writes the
abort packet once the caller aborts later.
- Signal nested inside a Request: exposed a real leak. The Request
constructor copies the signal to an internal AbortSignal, so the
ABORT_READER_CANCEL symbol set by reviveAbortSignal never reached
request.signal, and cancelAbortReaders' walker had no Request case
so Object.values(request) returned []. Fixed both sides:
- Request reviver copies abort-internal symbols via copyAbortInternals
- Walker descends into Request.signal explicitly
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@VaguelySerious

Copy link
Copy Markdown
Member

Should probably be reviewed by Pranay and then merged into pgp/serialize-abort-signal before it's reviewed as a whole

Conflict resolution:
- packages/core/src/serialization.ts: kept PR's reviveAbortSignal() function
(PR #1647's signal-only revival path) alongside the base branch's
getCommonRevivers() function from the modular refactor
Co-authored-by: Cursor <cursoragent@cursor.com>
@pranaygp

Copy link
Copy Markdown
Contributor

I'm resolving conflicts and merging your commits into my base PR from CLI @karthikscale3, ty!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@karthikscale3@VaguelySerious@pranaygp
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Fix dangling stream readers, serialization bugs, and refactor abort reducer code by karthikscale3 · Pull Request #1647 · vercel/workflow · GitHub
Skip to content

Fix dangling stream readers, serialization bugs, and refactor abort reducer code - #1647

Merged
pranaygp merged 10 commits into
pgp/serialize-abort-signalfrom
karthik/abort-signal-updates
May 2, 2026
Merged

Fix dangling stream readers, serialization bugs, and refactor abort reducer code#1647
pranaygp merged 10 commits into
pgp/serialize-abort-signalfrom
karthik/abort-signal-updates

Conversation

@karthikscale3

@karthikscale3karthikscale3 commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Description

Follow-up to #1301 (AbortController/AbortSignal serialization). Fixes bugs uncovered while exercising the end-to-end path and cleans up duplicated reducer code.

What changed

Bug fixes:

  • Dangling stream reader hangreviveAbortController starts a stream reader that blocks on reader.read() indefinitely if no abort arrives, keeping the serverless function alive until the platform timeout. Introduced an ABORT_READER_CANCEL symbol stored on the controller/signal and an AbortController the reader races against. The new cancelAbortReaders(...args, thisVal, closureVars) helper walks step arguments after the step function returns (success or failure) and aborts every reader, letting Promise.race resolve cleanly.
  • Request.signal serialization — The Request reducer previously serialized any signal that was aborted or had ABORT_STREAM_NAME set, which caused plain native signals (e.g. from fetch timeouts or user AbortControllers passed via new Request(url, { signal })) to inherit stream+hook infrastructure they did not need. The reducer now only serializes signals tagged with ABORT_STREAM_NAME, so plain native signals are silently stripped during Request serialization.
  • Missing is_system migration — Added the 0010_add_is_system.sql Drizzle migration (workflow_hooks.is_system boolean default false), updated the journal, and plumbed isSystem through the hook_created event in suspension-handler, world-postgres, and world-local so system hooks (currently only the abort hook) persist correctly and do not collide with user hook tokens.
  • getAbortStreamIdFromToken helper — Replaced the fragile queueItem.token.replace('abrt_', '') + manual strm_${id}_system_abort concat in suspension-handler with a shared helper in util.ts that validates the token prefix and reuses getAbortStreamId. Eliminates a latent bug where a token without the abrt_ prefix would silently produce a garbage stream name.
  • DurableAgent timeout path in workflow VM — PR feat: serializable AbortController/AbortSignal #1301 added AbortController to the workflow VM globals, which flipped the existing typeof AbortController !== 'undefined' guard in DurableAgent.generate/.stream to true inside workflows. That caused the timeout block to call setTimeout, which the VM traps, crashing any DurableAgent call from a workflow with a timeout option. Added an inWorkflowVm check based on the presence of Symbol.for('WORKFLOW_CONTEXT') on globalThis (set by the workflow runtime before user code runs). When true, the timeout block is skipped, restoring the pre-feat: serializable AbortController/AbortSignal #1301 behavior (timeouts silently do not fire in workflows; use sleep + abort() for durable time bounds).
  • onabort setter on WorkflowAbortSignal — The VM-side signal only exposed listener-based abort hooks. Added an onabort getter/setter that matches the native AbortSignal contract, fires the handler immediately when assigned if the signal is already aborted, and is invoked by _setAborted alongside the listener list.

Refactors:

  • Deduplicated ~180 lines of reducer code — Extracted reduceAbortWithListener() (symbol mint + listener attach + serialize) and reduceAbortBySymbol() (read existing symbols + serialize) helpers from the three near-identical AbortController/AbortSignal reducer implementations in getExternalReducers, getWorkflowReducers, and getStepReducers. Introduced AbortInternals, AbortSignalLike, and AbortHolder types so the shared helpers work for both controllers and standalone signals.
  • setupAbortStreamReader + tagAbortPair helpers — Extracted the stream-reader wiring and symbol-stamping into focused helpers used by both reviveAbortController and the new reviveAbortSignal.
  • Signal-only revival path — Added reviveAbortSignal() used by getStepRevivers.AbortSignal and getExternalRevivers.AbortSignal. Previously those revived through reviveAbortController(...).signal, which installed the patched abort() method even though nothing could reach the controller. The dedicated path skips the patch overhead and, crucially, still stores ABORT_READER_CANCEL on the returned signal so cancelAbortReaders can clean up standalone-signal readers (fixing a latent leak).

Tests:

  • Added test covering stream reader propagation with a targeted getWorld mock that delivers an actual abort payload (the default mock closes the stream immediately and would mask the code path).
  • Added test verifying plain (non-workflow) Request.signal is stripped during serialization — the hydrated Request gets a fresh default signal.
  • Rewrote the existing "Request with signal round-trip" test to exercise the step hydration path and explicitly tag request.signal with ABORT_STREAM_NAME/ABORT_HOOK_TOKEN (the Request constructor clones the signal, so symbols from the source controller do not transfer automatically).
  • Updated step-handler.test.ts mock to include cancelAbortReaders.

@vercel

vercelBot commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
example-nextjs-workflow-turbopackErrorErrorMay 1, 2026 9:17pm
example-nextjs-workflow-webpackErrorErrorMay 1, 2026 9:17pm
example-workflowErrorErrorMay 1, 2026 9:17pm
workbench-astro-workflowErrorErrorMay 1, 2026 9:17pm
workbench-express-workflowErrorErrorMay 1, 2026 9:17pm
workbench-fastify-workflowErrorErrorMay 1, 2026 9:17pm
workbench-hono-workflowErrorErrorMay 1, 2026 9:17pm
workbench-nitro-workflowErrorErrorMay 1, 2026 9:17pm
workbench-nuxt-workflowErrorErrorMay 1, 2026 9:17pm
workbench-sveltekit-workflowErrorErrorMay 1, 2026 9:17pm
workbench-vite-workflowErrorErrorMay 1, 2026 9:17pm
workflow-docsErrorErrorMay 1, 2026 9:17pm
workflow-swc-playgroundErrorErrorMay 1, 2026 9:17pm
workflow-webErrorErrorMay 1, 2026 9:17pm

@changeset-bot

changeset-botBot commented Apr 7, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: ee30ed7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions

github-actionsBot commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

🧪 E2E Test Results

No test result files found.


Some E2E test jobs failed:

  • Vercel Prod: failure
  • Local Dev: failure
  • Local Prod: failure
  • Local Postgres: failure
  • Windows: failure

Check the workflow run for details.

@karthikscale3
karthikscale3 marked this pull request as ready for review April 7, 2026 21:10
@karthikscale3
karthikscale3 requested a review from a team as a code ownerApril 7, 2026 21:10
@karthikscale3
karthikscale3force-pushed the karthik/abort-signal-updates branch from 414942e to d786ebbCompareApril 21, 2026 19:13
@vercel

vercelBot commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

Deployment failed with the following error:

Vercel employees are required to have Two-Factor Authentication enabled.

View Documentation: https://vercel.com/docs/two-factor-authentication

Two leak paths the prior fix left uncovered:
- External signal aborted after serialization: verifies the listener
attached by reduceAbortWithListener actually fires and writes the
abort packet once the caller aborts later.
- Signal nested inside a Request: exposed a real leak. The Request
constructor copies the signal to an internal AbortSignal, so the
ABORT_READER_CANCEL symbol set by reviveAbortSignal never reached
request.signal, and cancelAbortReaders' walker had no Request case
so Object.values(request) returned []. Fixed both sides:
- Request reviver copies abort-internal symbols via copyAbortInternals
- Walker descends into Request.signal explicitly
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@VaguelySerious

Copy link
Copy Markdown
Member

Should probably be reviewed by Pranay and then merged into pgp/serialize-abort-signal before it's reviewed as a whole

Conflict resolution:
- packages/core/src/serialization.ts: kept PR's reviveAbortSignal() function
(PR #1647's signal-only revival path) alongside the base branch's
getCommonRevivers() function from the modular refactor
Co-authored-by: Cursor <cursoragent@cursor.com>
@pranaygp

Copy link
Copy Markdown
Contributor

I'm resolving conflicts and merging your commits into my base PR from CLI @karthikscale3, ty!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@karthikscale3@VaguelySerious@pranaygp
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); Fix dangling stream readers, serialization bugs, and refactor abort reducer code by karthikscale3 · Pull Request #1647 · vercel/workflow · GitHub
Skip to content

Fix dangling stream readers, serialization bugs, and refactor abort reducer code - #1647

Merged
pranaygp merged 10 commits into
pgp/serialize-abort-signalfrom
karthik/abort-signal-updates
May 2, 2026
Merged

Fix dangling stream readers, serialization bugs, and refactor abort reducer code#1647
pranaygp merged 10 commits into
pgp/serialize-abort-signalfrom
karthik/abort-signal-updates

Conversation

@karthikscale3

@karthikscale3karthikscale3 commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Description

Follow-up to #1301 (AbortController/AbortSignal serialization). Fixes bugs uncovered while exercising the end-to-end path and cleans up duplicated reducer code.

What changed

Bug fixes:

  • Dangling stream reader hangreviveAbortController starts a stream reader that blocks on reader.read() indefinitely if no abort arrives, keeping the serverless function alive until the platform timeout. Introduced an ABORT_READER_CANCEL symbol stored on the controller/signal and an AbortController the reader races against. The new cancelAbortReaders(...args, thisVal, closureVars) helper walks step arguments after the step function returns (success or failure) and aborts every reader, letting Promise.race resolve cleanly.
  • Request.signal serialization — The Request reducer previously serialized any signal that was aborted or had ABORT_STREAM_NAME set, which caused plain native signals (e.g. from fetch timeouts or user AbortControllers passed via new Request(url, { signal })) to inherit stream+hook infrastructure they did not need. The reducer now only serializes signals tagged with ABORT_STREAM_NAME, so plain native signals are silently stripped during Request serialization.
  • Missing is_system migration — Added the 0010_add_is_system.sql Drizzle migration (workflow_hooks.is_system boolean default false), updated the journal, and plumbed isSystem through the hook_created event in suspension-handler, world-postgres, and world-local so system hooks (currently only the abort hook) persist correctly and do not collide with user hook tokens.
  • getAbortStreamIdFromToken helper — Replaced the fragile queueItem.token.replace('abrt_', '') + manual strm_${id}_system_abort concat in suspension-handler with a shared helper in util.ts that validates the token prefix and reuses getAbortStreamId. Eliminates a latent bug where a token without the abrt_ prefix would silently produce a garbage stream name.
  • DurableAgent timeout path in workflow VM — PR feat: serializable AbortController/AbortSignal #1301 added AbortController to the workflow VM globals, which flipped the existing typeof AbortController !== 'undefined' guard in DurableAgent.generate/.stream to true inside workflows. That caused the timeout block to call setTimeout, which the VM traps, crashing any DurableAgent call from a workflow with a timeout option. Added an inWorkflowVm check based on the presence of Symbol.for('WORKFLOW_CONTEXT') on globalThis (set by the workflow runtime before user code runs). When true, the timeout block is skipped, restoring the pre-feat: serializable AbortController/AbortSignal #1301 behavior (timeouts silently do not fire in workflows; use sleep + abort() for durable time bounds).
  • onabort setter on WorkflowAbortSignal — The VM-side signal only exposed listener-based abort hooks. Added an onabort getter/setter that matches the native AbortSignal contract, fires the handler immediately when assigned if the signal is already aborted, and is invoked by _setAborted alongside the listener list.

Refactors:

  • Deduplicated ~180 lines of reducer code — Extracted reduceAbortWithListener() (symbol mint + listener attach + serialize) and reduceAbortBySymbol() (read existing symbols + serialize) helpers from the three near-identical AbortController/AbortSignal reducer implementations in getExternalReducers, getWorkflowReducers, and getStepReducers. Introduced AbortInternals, AbortSignalLike, and AbortHolder types so the shared helpers work for both controllers and standalone signals.
  • setupAbortStreamReader + tagAbortPair helpers — Extracted the stream-reader wiring and symbol-stamping into focused helpers used by both reviveAbortController and the new reviveAbortSignal.
  • Signal-only revival path — Added reviveAbortSignal() used by getStepRevivers.AbortSignal and getExternalRevivers.AbortSignal. Previously those revived through reviveAbortController(...).signal, which installed the patched abort() method even though nothing could reach the controller. The dedicated path skips the patch overhead and, crucially, still stores ABORT_READER_CANCEL on the returned signal so cancelAbortReaders can clean up standalone-signal readers (fixing a latent leak).

Tests:

  • Added test covering stream reader propagation with a targeted getWorld mock that delivers an actual abort payload (the default mock closes the stream immediately and would mask the code path).
  • Added test verifying plain (non-workflow) Request.signal is stripped during serialization — the hydrated Request gets a fresh default signal.
  • Rewrote the existing "Request with signal round-trip" test to exercise the step hydration path and explicitly tag request.signal with ABORT_STREAM_NAME/ABORT_HOOK_TOKEN (the Request constructor clones the signal, so symbols from the source controller do not transfer automatically).
  • Updated step-handler.test.ts mock to include cancelAbortReaders.

@vercel

vercelBot commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
example-nextjs-workflow-turbopackErrorErrorMay 1, 2026 9:17pm
example-nextjs-workflow-webpackErrorErrorMay 1, 2026 9:17pm
example-workflowErrorErrorMay 1, 2026 9:17pm
workbench-astro-workflowErrorErrorMay 1, 2026 9:17pm
workbench-express-workflowErrorErrorMay 1, 2026 9:17pm
workbench-fastify-workflowErrorErrorMay 1, 2026 9:17pm
workbench-hono-workflowErrorErrorMay 1, 2026 9:17pm
workbench-nitro-workflowErrorErrorMay 1, 2026 9:17pm
workbench-nuxt-workflowErrorErrorMay 1, 2026 9:17pm
workbench-sveltekit-workflowErrorErrorMay 1, 2026 9:17pm
workbench-vite-workflowErrorErrorMay 1, 2026 9:17pm
workflow-docsErrorErrorMay 1, 2026 9:17pm
workflow-swc-playgroundErrorErrorMay 1, 2026 9:17pm
workflow-webErrorErrorMay 1, 2026 9:17pm

@changeset-bot

changeset-botBot commented Apr 7, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: ee30ed7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions

github-actionsBot commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

🧪 E2E Test Results

No test result files found.


Some E2E test jobs failed:

  • Vercel Prod: failure
  • Local Dev: failure
  • Local Prod: failure
  • Local Postgres: failure
  • Windows: failure

Check the workflow run for details.

@karthikscale3
karthikscale3 marked this pull request as ready for review April 7, 2026 21:10
@karthikscale3
karthikscale3 requested a review from a team as a code ownerApril 7, 2026 21:10
@karthikscale3
karthikscale3force-pushed the karthik/abort-signal-updates branch from 414942e to d786ebbCompareApril 21, 2026 19:13
@vercel

vercelBot commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

Deployment failed with the following error:

Vercel employees are required to have Two-Factor Authentication enabled.

View Documentation: https://vercel.com/docs/two-factor-authentication

Two leak paths the prior fix left uncovered:
- External signal aborted after serialization: verifies the listener
attached by reduceAbortWithListener actually fires and writes the
abort packet once the caller aborts later.
- Signal nested inside a Request: exposed a real leak. The Request
constructor copies the signal to an internal AbortSignal, so the
ABORT_READER_CANCEL symbol set by reviveAbortSignal never reached
request.signal, and cancelAbortReaders' walker had no Request case
so Object.values(request) returned []. Fixed both sides:
- Request reviver copies abort-internal symbols via copyAbortInternals
- Walker descends into Request.signal explicitly
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@VaguelySerious

Copy link
Copy Markdown
Member

Should probably be reviewed by Pranay and then merged into pgp/serialize-abort-signal before it's reviewed as a whole

Conflict resolution:
- packages/core/src/serialization.ts: kept PR's reviveAbortSignal() function
(PR #1647's signal-only revival path) alongside the base branch's
getCommonRevivers() function from the modular refactor
Co-authored-by: Cursor <cursoragent@cursor.com>
@pranaygp

Copy link
Copy Markdown
Contributor

I'm resolving conflicts and merging your commits into my base PR from CLI @karthikscale3, ty!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@karthikscale3@VaguelySerious@pranaygp