stream: preserve asynclocalstorage context in finished()#57865

gurgunday · 2025-04-13T17:28:52Z

nodejs-github-bot · 2025-04-13T17:28:57Z

Review requested:

@nodejs/streams

jasnell · 2025-04-13T17:48:08Z

Inside the finished(...) function definition, you ought to be able to simply use AsyncLocalStore.bind(...) to wrap the callback function such that the current context is preserved. Essentially equivalent to the following except the AsyncLocalStorage.bind(...) call would be inside the finished(...) function body.

const{ finished, Readable }=require('stream');const{ AsyncLocalStorage }=require('async_hooks');constals=newAsyncLocalStorage();constreadable=newReadable();als.run(123,()=>{finished(readable,AsyncLocalStorage.bind(()=>console.log(als.getStore())));});readable.destroy();

gurgunday · 2025-04-13T18:16:47Z

@jasnell oh you're absolutely right, I missed that, thanks!

lib/internal/streams/end-of-stream.js

test/async-hooks/test-async-local-storage-stream-finished.js

codecov · 2025-04-13T19:57:08Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.25%. Comparing base (964e41c) to head (cd26628).
Report is 444 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #57865 +/- ## ========================================== + Coverage 90.23% 90.25% +0.02%  ========================================== Files 630 630 Lines 185518 185674 +156 Branches 36369 36401 +32 ========================================== + Hits 167401 167580 +179 + Misses 11005 10992 -13 + Partials 7112 7102 -10

Files with missing lines	Coverage Δ
lib/internal/streams/end-of-stream.js	`97.28% <100.00%> (+0.03%)`	⬆️

... and 27 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.

bjohansebas

It looks good, thanks for working on this

Also, lgtm if the CI passes

github-actions · 2025-04-13T22:20:59Z

Failed to start CI

 ⚠ No approving reviews found ✘ Refusing to run CI on potentially unsafe PR

https://github.com/nodejs/node/actions/runs/14433881994

rluvaton

LGTM

lib/internal/streams/end-of-stream.js

mcollina

lgtm

nodejs-github-bot · 2025-04-14T07:42:48Z

CI: https://ci.nodejs.org/job/node-test-pull-request/66248/ 💛

nodejs-github-bot · 2025-04-16T09:27:39Z

Landed in 4b2b3c0

PR-URL: #57865 Reviewed-By: Raz Luvaton <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>

Notable changes: deps: * update timezone to 2025b (Node.js GitHub Bot) #57857 doc: * add dario-piotrowicz to collaborators (Dario Piotrowicz) #58102 * (SEMVER-MINOR) graduate multiple experimental apis (James M Snell) #57765 esm: * (SEMVER-MINOR) graduate import.meta properties (James M Snell) #58011 * (SEMVER-MINOR) support top-level Wasm without package type (Guy Bedford) #57610 sqlite: * (SEMVER-MINOR) add StatementSync.prototype.columns() (Colin Ihrig) #57490 src: * (SEMVER-MINOR) set default config as `node.config.json` (Marco Ippolito) #57171 * (SEMVER-MINOR) create `THROW_ERR_OPTIONS_BEFORE_BOOTSTRAPPING` (Marco Ippolito) #57016 * (SEMVER-MINOR) add config file support (Marco Ippolito) #57016 * (SEMVER-MINOR) add ExecutionAsyncId getter for any Context (Attila Szegedi) #57820 stream: * (SEMVER-MINOR) preserve AsyncLocalStorage context in finished() (Gürgün Dayıoğlu) #57865 util: * (SEMVER-MINOR) add `types.isFloat16Array()` (Livia Medeiros) #57879 worker: * (SEMVER-MINOR) add worker.getHeapStatistics() (Matteo Collina) #57888 PR-URL: #58388

PR-URL: #57865 Reviewed-By: Raz Luvaton <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>

Notable changes: deps: * update timezone to 2025b (Node.js GitHub Bot) #57857 doc: * add dario-piotrowicz to collaborators (Dario Piotrowicz) #58102 * (SEMVER-MINOR) graduate multiple experimental apis (James M Snell) #57765 esm: * (SEMVER-MINOR) graduate import.meta properties (James M Snell) #58011 * (SEMVER-MINOR) support top-level Wasm without package type (Guy Bedford) #57610 sqlite: * (SEMVER-MINOR) add StatementSync.prototype.columns() (Colin Ihrig) #57490 src: * (SEMVER-MINOR) set default config as `node.config.json` (Marco Ippolito) #57171 * (SEMVER-MINOR) create `THROW_ERR_OPTIONS_BEFORE_BOOTSTRAPPING` (Marco Ippolito) #57016 * (SEMVER-MINOR) add config file support (Marco Ippolito) #57016 * (SEMVER-MINOR) add ExecutionAsyncId getter for any Context (Attila Szegedi) #57820 stream: * (SEMVER-MINOR) preserve AsyncLocalStorage context in finished() (Gürgün Dayıoğlu) #57865 util: * (SEMVER-MINOR) add `types.isFloat16Array()` (Livia Medeiros) #57879 worker: * (SEMVER-MINOR) add worker.getHeapStatistics() (Matteo Collina) #57888 PR-URL: TODO

Notable changes: deps: * update timezone to 2025b (Node.js GitHub Bot) #57857 doc: * add dario-piotrowicz to collaborators (Dario Piotrowicz) #58102 * (SEMVER-MINOR) graduate multiple experimental apis (James M Snell) #57765 esm: * (SEMVER-MINOR) graduate import.meta properties (James M Snell) #58011 * (SEMVER-MINOR) support top-level Wasm without package type (Guy Bedford) #57610 sqlite: * (SEMVER-MINOR) add StatementSync.prototype.columns() (Colin Ihrig) #57490 src: * (SEMVER-MINOR) set default config as `node.config.json` (Marco Ippolito) #57171 * (SEMVER-MINOR) create `THROW_ERR_OPTIONS_BEFORE_BOOTSTRAPPING` (Marco Ippolito) #57016 * (SEMVER-MINOR) add config file support (Marco Ippolito) #57016 * (SEMVER-MINOR) add ExecutionAsyncId getter for any Context (Attila Szegedi) #57820 stream: * (SEMVER-MINOR) preserve AsyncLocalStorage context in finished() (Gürgün Dayıoğlu) #57865 util: * (SEMVER-MINOR) add `types.isFloat16Array()` (Livia Medeiros) #57879 worker: * (SEMVER-MINOR) add worker.getHeapStatistics() (Matteo Collina) #57888 PR-URL: #58388

Notable changes: deps: * update timezone to 2025b (Node.js GitHub Bot) nodejs#57857 doc: * add dario-piotrowicz to collaborators (Dario Piotrowicz) nodejs#58102 * (SEMVER-MINOR) graduate multiple experimental apis (James M Snell) nodejs#57765 esm: * (SEMVER-MINOR) graduate import.meta properties (James M Snell) nodejs#58011 * (SEMVER-MINOR) support top-level Wasm without package type (Guy Bedford) nodejs#57610 sqlite: * (SEMVER-MINOR) add StatementSync.prototype.columns() (Colin Ihrig) nodejs#57490 src: * (SEMVER-MINOR) set default config as `node.config.json` (Marco Ippolito) nodejs#57171 * (SEMVER-MINOR) create `THROW_ERR_OPTIONS_BEFORE_BOOTSTRAPPING` (Marco Ippolito) nodejs#57016 * (SEMVER-MINOR) add config file support (Marco Ippolito) nodejs#57016 * (SEMVER-MINOR) add ExecutionAsyncId getter for any Context (Attila Szegedi) nodejs#57820 stream: * (SEMVER-MINOR) preserve AsyncLocalStorage context in finished() (Gürgün Dayıoğlu) nodejs#57865 util: * (SEMVER-MINOR) add `types.isFloat16Array()` (Livia Medeiros) nodejs#57879 worker: * (SEMVER-MINOR) add worker.getHeapStatistics() (Matteo Collina) nodejs#57888 PR-URL: nodejs#58388

Notable changes: deps: * update timezone to 2025b (Node.js GitHub Bot) #57857 doc: * add dario-piotrowicz to collaborators (Dario Piotrowicz) #58102 * (SEMVER-MINOR) graduate multiple experimental apis (James M Snell) #57765 esm: * (SEMVER-MINOR) graduate import.meta properties (James M Snell) #58011 * (SEMVER-MINOR) support top-level Wasm without package type (Guy Bedford) #57610 sqlite: * (SEMVER-MINOR) add StatementSync.prototype.columns() (Colin Ihrig) #57490 src: * (SEMVER-MINOR) set default config as `node.config.json` (Marco Ippolito) #57171 * (SEMVER-MINOR) create `THROW_ERR_OPTIONS_BEFORE_BOOTSTRAPPING` (Marco Ippolito) #57016 * (SEMVER-MINOR) add config file support (Marco Ippolito) #57016 * (SEMVER-MINOR) add ExecutionAsyncId getter for any Context (Attila Szegedi) #57820 stream: * (SEMVER-MINOR) preserve AsyncLocalStorage context in finished() (Gürgün Dayıoğlu) #57865 util: * (SEMVER-MINOR) add `types.isFloat16Array()` (Livia Medeiros) #57879 worker: * (SEMVER-MINOR) add worker.getHeapStatistics() (Matteo Collina) #57888 PR-URL: #58388

PR-URL: #57865 Reviewed-By: Raz Luvaton <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>

nodejs-github-bot added needs-ci PRs that need a full CI run. stream Issues and PRs related to the stream subsystem. labels Apr 13, 2025

stream: preserve AsyncLocalStorage context in finished()
a29d44d

gurgunday force-pushed the fix/async-storage-stream-finished branch from 783785d to a29d44dCompare April 13, 2025 18:16

jasnell reviewed Apr 13, 2025
View reviewed changes

lib/internal/streams/end-of-stream.js Outdated Show resolvedHide resolved

jasnell reviewed Apr 13, 2025
View reviewed changes

test/async-hooks/test-async-local-storage-stream-finished.js Outdated Show resolvedHide resolved

gurgunday added 2 commits April 13, 2025 21:06

lazy load asynclocalstorage and make the test more compact
f1277a9

simplify
c27a36f

bjohansebas approved these changes Apr 13, 2025
View reviewed changes

bjohansebas added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 13, 2025

github-actionsbot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Apr 13, 2025

rluvaton approved these changes Apr 13, 2025
View reviewed changes

lib/internal/streams/end-of-stream.js Outdated Show resolvedHide resolved

rluvaton added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels Apr 13, 2025

github-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 13, 2025

This comment was marked as outdated.
Sign in to view

revert formatting change
cd26628

mcollina approved these changes Apr 14, 2025
View reviewed changes

mcollina added semver-minor PRs that contain new features and should be released in the next minor version. request-ci Add this label to start a Jenkins CI on a PR. labels Apr 14, 2025

github-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 14, 2025

legendecas approved these changes Apr 14, 2025
View reviewed changes

mcollina added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 14, 2025

legendecas added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 16, 2025

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 16, 2025

nodejs-github-bot merged commit 4b2b3c0 into nodejs:mainApr 16, 2025
83 checks passed

RafaelGSS mentioned this pull request May 1, 2025
2025-05-06, Version 24.0.0 (Current) #57609
Merged

slagiewka mentioned this pull request May 12, 2025
deprecate shows increased use of CPU #58218
Closed

bjohansebas added lts-watch-v20.x PRs that may need to be released in v20.x lts-watch-v22.x PRs that may need to be released in v22.x labels May 15, 2025

aduh95 removed the lts-watch-v22.x PRs that may need to be released in v22.x label May 19, 2025

github-actionsbot mentioned this pull request May 19, 2025
2025-05-21, Version 22.16.0 'Jod' (LTS) #58388
Merged

marco-ippolito removed the lts-watch-v20.x PRs that may need to be released in v20.x label Jun 5, 2025

marco-ippolito added the dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. label Jun 5, 2025

ghost mentioned this pull request Jun 8, 2025
### Notable Changes autofix-ci/action#28
Closed

Uh oh!

stream: preserve asynclocalstorage context in finished()#57865

stream: preserve asynclocalstorage context in finished() #57865

Uh oh!

Conversation

gurgunday commented Apr 13, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Apr 13, 2025• edited by rluvaton Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jasnell commented Apr 13, 2025

Uh oh!

gurgunday commented Apr 13, 2025

Uh oh!

Uh oh!

Uh oh!

codecovbot commented Apr 13, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bjohansebas left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actionsbot commented Apr 13, 2025

Uh oh!

rluvaton left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment was marked as outdated.

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Apr 14, 2025• edited by jasnell Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

nodejs-github-bot commented Apr 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

gurgunday commented Apr 13, 2025•
edited
Loading

nodejs-github-bot commented Apr 13, 2025•
edited by rluvaton
Loading

codecovbot commented Apr 13, 2025•
edited
Loading

bjohansebas left a comment •
edited
Loading

nodejs-github-bot commented Apr 14, 2025•
edited by jasnell
Loading