Open
Conversation
Collaborator
|
Review requested:
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #61095 +/- ##
==========================================
- Coverage 88.53% 88.51% -0.03%
==========================================
Files 703 703
Lines 208538 208662 +124
Branches 40224 40235 +11
==========================================
+ Hits 184629 184696 +67
- Misses 15912 15975 +63
+ Partials 7997 7991 -6
🚀 New features to boost your workflow:
|
RafaelGSS
reviewed
Dec 17, 2025
anonrig
approved these changes
Dec 18, 2025
8a8cf7f to
b307f20
Compare
Port the domain module from createHook (async_hooks) to AsyncLocalStorage using the AsyncContextFrame-based implementation. Key changes: - Use AsyncLocalStorage for domain context propagation instead of async_hooks.createHook() - Lazy initialization that triggers AsyncContextFrame prototype swap on first domain use - Use enterWith instead of ALS.run() so domain context is NOT automatically restored on exception - this matches the original domain.run() behavior where exit() only runs on success - Add ERR_ASYNC_RESOURCE_DOMAIN_REMOVED error for AsyncResource.domain - Update DEP0097 to End-of-Life status - Remove tests that relied on the removed MakeCallback domain property The domain module now uses the AsyncContextFrame version of AsyncLocalStorage directly for proper context propagation across async boundaries.
b307f20 to
1d24947
Compare
RafaelGSS
approved these changes
Dec 18, 2025
Collaborator
Flarna
reviewed
Dec 19, 2025
Member
Flarna
left a comment
There was a problem hiding this comment.
I have to admit that I do not fully understand domains so take my review with a grain of salt.
Remove the useDomainTrampoline function and related domain_cb variable that are no longer used after domain was ported to AsyncLocalStorage.
The test no longer tests async-id map leaks. Rename to reflect its actual purpose: verifying that accessing domain property on AsyncResource throws ERR_ASYNC_RESOURCE_DOMAIN_REMOVED.
Use require('async_hooks').AsyncLocalStorage instead of directly
importing internal modules. This allows proper auto-selection of
the ALS implementation based on --async-context-frame flag.
Extract common domain context setup/teardown logic into a single runInDomain helper function, reducing code duplication.
When an EventEmitter emits 'error' and the domain's error handler throws, the exception should propagate to the parent domain, not recursively call the same domain's error handler. The bug was that currentDomain/currentStack were not updated when temporarily switching to the parent domain context during error emission. Since domainExceptionHandler checks currentDomain first, exceptions would incorrectly route back to the same domain. Fixes by updating currentDomain/currentStack alongside the ALS store.
ronag
approved these changes
Jan 11, 2026
This comment was marked as outdated.
This comment was marked as outdated.
Collaborator
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.
Port the domain module from createHook (async_hooks) to AsyncLocalStorage using the AsyncContextFrame-based implementation.
Key changes:
The domain module now uses the AsyncContextFrame version of AsyncLocalStorage directly for proper context propagation across async boundaries.