Skip to content

lib: add a diagnostic channel for observing AsyncContextFrame.set #58229

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

szegedi
Copy link
Contributor

@szegedi szegedi commented May 8, 2025

Adds an async_context_frame.set diagnostic channel for observing AsyncContextFrame.set.

This is helpful for functionality in extensions written in C++ that need to keep some derived native state from one or more AsyncLocalStorages in sync with changes in active frame. It is possible that such an extension needs to use the state when it can not enter V8 to obtain the data through AsyncLocalStorage JS API. A subscriber to this diagnostic channel can be used instead to eagerly keep the native state updated.

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label May 8, 2025
Copy link
Member

@Qard Qard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should also add the channel to the diagnostics_channel docs here: https://github.com/nodejs/node/blob/main/doc/api/diagnostics_channel.md?plain=1#L1104

Copy link

codecov bot commented May 8, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.12%. Comparing base (535c2f7) to head (71104b3).
Report is 12 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #58229   +/-   ##
=======================================
  Coverage   90.12%   90.12%           
=======================================
  Files         629      629           
  Lines      186638   186638           
  Branches    36618    36622    +4     
=======================================
+ Hits       168202   168216   +14     
+ Misses      11222    11201   -21     
- Partials     7214     7221    +7     
Files with missing lines Coverage Δ
lib/internal/async_context_frame.js 100.00% <100.00%> (ø)

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

@szegedi szegedi requested a review from Qard May 8, 2025 12:40
Copy link
Member

@Qard Qard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might also be a good idea to have the tests validate that the frame can be used to look up the state for a given store. The store is the key so it should just be frame.get(store) to get the value.

@szegedi szegedi requested a review from Qard May 8, 2025 13:08
@Qard Qard added request-ci Add this label to start a Jenkins CI on a PR. diagnostics_channel Issues and PRs related to diagnostics channel async_local_storage AsyncLocalStorage doc Issues and PRs related to the documentations. and removed needs-ci PRs that need a full CI run. labels May 8, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 8, 2025
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
async_local_storage AsyncLocalStorage diagnostics_channel Issues and PRs related to diagnostics channel doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants