Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.4k
util: avoid inline access to Symbol.iterator#43683
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
util: avoid inline access to Symbol.iterator #43683
Uh oh!
There was an error while loading. Please reload this page.
Conversation
cola119 commented Jul 5, 2022 • edited by BridgeAR
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by BridgeAR
Uh oh!
There was an error while loading. Please reload this page.
bnoordhuis left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, although I can't reproduce the reported behavior (which is against v17.x) with v18.2.0. This is what I see instead:
> util.inspect({get [Symbol.iterator](){throw Error("boom") }}) Uncaught Error: boom at get [Symbol.iterator] (REPL1:1:46) at formatRaw (node:internal/util/inspect:857:12) at formatValue (node:internal/util/inspect:817:10) at Object.inspect (node:internal/util/inspect:347:10) (Same with node -p '...')
BridgeAR left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
BridgeAR commented Jul 5, 2022
It does not fix: #41244 I removed that from the description above. I have a fix for that and am going to open a PR later on. |
cola119 commented Jul 5, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
@bnoordhuis The below reproduces the reported behavior (v18.4.0) for the same reason. util.inspect({a: {get[Symbol.iterator](){throwError('boom');},},}); |
cola119 commented Jul 5, 2022
Maybe I'm misunderstanding the cause, but anyway @BridgeAR, thanks for the confirmation. |
BridgeAR commented Jul 5, 2022
@cola119 it is one possibility to trigger the issue. There are other ways to trigger the internal error as well. |
nodejs-github-bot commented Jul 7, 2022
nodejs-github-bot commented Jul 8, 2022
nodejs-github-bot commented Jul 8, 2022
nodejs-github-bot commented Jul 9, 2022
Landed in 550e814 |
PR-URL: #43683 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Darshan Sen <[email protected]>
PR-URL: #43683 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Darshan Sen <[email protected]>
PR-URL: #43683 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Darshan Sen <[email protected]>
PR-URL: nodejs/node#43683 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Darshan Sen <[email protected]>
Currently
util.inspecthas the inline access toSymbol.iteratorto check if the value to be inspected is an iterator, but it could throw when the value of[Symbol.iterator]throws(has) an error.This PR replaced the inline access with
inoperator to be able to inspect correctly.