Skip to content

stream: keep async context of pipeTo watchers - #67052

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
mcollina:webstreams-als-tests
Oct 29, 2026
Merged

nodejs-github-bot merged 1 commit into
nodejs:mainfrom
mcollina:webstreams-als-tests

Conversation

@mcollina

@mcollina mcollina commented Oct 27, 2026 •

Copy link
Copy Markdown
Member

The ClosedPromiseHook records that pipeTo() and tee() install as the [[closedPromise]] of their reader and writer (#66424) enqueue the watcher from whoever settles the record. As a result, the source-closed, source-errored and dest-errored steps of pipeTo() run in the async context of the code that closed or errored the stream: the destination's close()/abort() and the source's cancel() observe the wrong AsyncLocalStorage store. This is the same class of bug as #66906.

const ts = new TransformStream();
const ws = new WritableStream({ close() { console.log(als.getStore()); } });
als.run('piper', () => ts.readable.pipeTo(ws));
const w = ts.writable.getWriter();
await als.run('closer', () => w.close()); // logs 'closer', v24/v26.5 log 'piper'

This captures the async context frame when the hook is installed and registers the reaction in it, as the spec's promise reaction would.

It also adds test-whatwg-webstreams-async-context.js, which records the context each underlying source, sink and transformer callback observes across 29 readable, writable, transform, pipe and tee scenarios, so scheduling shortcuts that drop the context get caught. The expectations were taken from v24.18.0 (with and without --no-async-context-frame) and v26.5.1, which agree on all of them. Only the context of each callback is compared, not how callbacks of different streams interleave. The test also fails on the parent of 60e6c84 (the #66906 fix).

Refs: #66906
Refs: #66424


AI generated, reviewed by me

The ClosedPromiseHook records that pipeTo() and tee() install as the
[[closedPromise]] of their reader and writer enqueued the watcher from
whoever settled the record. The source-closed, source-errored and
dest-errored steps of pipeTo() therefore ran in the async context of
the code that closed or errored the stream, so the destination's
close() and abort() and the source's cancel() observed the wrong
AsyncLocalStorage store. Capture the async context frame when the
hook is installed and register the reaction in it, as the spec's
promise reaction would.

Add a test that records the context each underlying source, sink and
transformer callback observes across readable, writable, transform,
pipe and tee scenarios, so scheduling shortcuts that drop the context
are caught.

Refs: nodejs#66906
Refs: nodejs#66424
Signed-off-by: Matteo Collina <hello@matteocollina.com>
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. web streams Issues and PRs related to the Web Streams API. labels Oct 27, 2026
@codecov

codecov Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.65%. Comparing base (29bd740) to head (e15df5f).
⚠️ Report is 223 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #67052      +/-   ##
==========================================
+ Coverage   90.47%   90.65%   +0.18%     
==========================================
  Files         791      797       +6     
  Lines      277245   278279    +1034     
  Branches    53310    53519     +209     
==========================================
+ Hits       250834   252283    +1449     
+ Misses      16811    16371     -440     
- Partials     9600     9625      +25     
Files with missing lines Coverage Δ
lib/internal/webstreams/readablestream.js 98.36% <100.00%> (+0.25%) ⬆️

... and 140 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@aduh95 aduh95 added author ready PRs with CI started, the required approvals, and no outstanding review comments. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels Oct 27, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Oct 27, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@aduh95 aduh95 added the commit-queue PRs queued for automated landing through the Commit Queue. label Oct 28, 2026
@nodejs-github-bot
nodejs-github-bot merged commit d56c163 into nodejs:main Oct 29, 2026
80 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in d56c163

@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Oct 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. needs-ci PRs that need a full CI run. web streams Issues and PRs related to the Web Streams API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants