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');

const als = new AsyncLocalStorage();
const readable = new Readable();

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 <rluvaton@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>

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 <rluvaton@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>

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 <rluvaton@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>

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 a29d44d Compare April 13, 2025 18:16

jasnell reviewed Apr 13, 2025

View reviewed changes

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

jasnell reviewed Apr 13, 2025

View reviewed changes

test/async-hooks/test-async-local-storage-stream-finished.js Outdated Show resolved Hide 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-actions bot 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 resolved Hide 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-actions bot 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-actions bot 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:main Apr 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

Open

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-actions bot 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

AuntyEms mentioned this pull request Jun 8, 2025

### Notable Changes autofix-ci/action#28

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

stream: preserve asynclocalstorage context in finished() #57865

stream: preserve asynclocalstorage context in finished() #57865

Uh oh!

gurgunday commented Apr 13, 2025 •

edited

Loading

Uh oh!

nodejs-github-bot commented Apr 13, 2025 •

edited by rluvaton

Loading

Uh oh!

jasnell commented Apr 13, 2025

Uh oh!

gurgunday commented Apr 13, 2025

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Apr 13, 2025 •

edited

Loading

Uh oh!

bjohansebas left a comment •

edited

Loading

Uh oh!

github-actions bot commented Apr 13, 2025

Uh oh!

rluvaton left a comment

Uh oh!

Uh oh!

This comment was marked as outdated.

mcollina left a comment

Uh oh!

nodejs-github-bot commented Apr 14, 2025 •

edited by jasnell

Loading

Uh oh!

Uh oh!

nodejs-github-bot commented Apr 16, 2025

Uh oh!

Uh oh!

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!

codecov bot 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-actions bot 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!

Uh oh!

gurgunday commented Apr 13, 2025 •

edited

Loading

nodejs-github-bot commented Apr 13, 2025 •

edited by rluvaton

Loading

codecov bot commented Apr 13, 2025 •

edited

Loading

bjohansebas left a comment •

edited

Loading

nodejs-github-bot commented Apr 14, 2025 •

edited by jasnell

Loading