Repository navigation
stream: keep async context of pipeTo watchers - #67052
Merged
Merged
Conversation
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>
This was referenced Oct 27, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. 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
🚀 New features to boost your workflow:
|
aduh95
approved these changes
Oct 27, 2026
Collaborator
ShogunPanda
approved these changes
Oct 28, 2026
Collaborator
|
Landed in d56c163 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
ClosedPromiseHookrecords thatpipeTo()andtee()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 ofpipeTo()run in the async context of the code that closed or errored the stream: the destination'sclose()/abort()and the source'scancel()observe the wrongAsyncLocalStoragestore. This is the same class of bug as #66906.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