[fix] make test/create-websocket-stream.test.js compatible with domains #2126
Conversation
This file contains 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
Fix a failure in test/create-websocket-stream.test.js if the node:domain module is loaded (e.g. due to NODE_OPTIONS in environment). The cause of the failure was that installing an 'uncaughtException' event handler on process will cause node:domain to prepend its own handler for the same event, which confused the test. Fixes: websockets#2124
lpinca
reviewed
Mar 9, 2023
|
|
||
| const [listener] = process.listeners('uncaughtException'); | ||
| const listener = process.listeners('uncaughtException').pop(); |
There was a problem hiding this comment.
The domain added 'uncaughtException' listener is always prepended and kept, right?
There was a problem hiding this comment.
It is prepended when you add the first listener for 'uncaughtException' and removed when you remove the last listener for it (excluding the one from domain)
> let foo = () => {}
> let bar = () => {}
> process.listenerCount('uncaughtException')
0
> process.on('uncaughtException', foo); process.listenerCount('uncaughtException')
2
> process.on('uncaughtException', bar); process.listenerCount('uncaughtException')
3
> process.off('uncaughtException', foo); process.listenerCount('uncaughtException')
2
> process.off('uncaughtException', bar); process.listenerCount('uncaughtException')
0
lpinca
approved these changes
Mar 9, 2023
|
Thank you. |
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.
Fix a failure in test/create-websocket-stream.test.js if the node:domain module is loaded (e.g. due to NODE_OPTIONS in environment).
The cause of the failure was that installing an 'uncaughtException' event handler on process will cause node:domain to prepend its own handler for the same event, which confused the test.
Fixes: #2124