-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
util: avoid inline access to Symbol.iterator #43683
Conversation
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 '...'
)
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
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. |
@bnoordhuis The below reproduces the reported behavior (v18.4.0) for the same reason. util.inspect({
a: {
get [Symbol.iterator]() {
throw Error('boom');
},
},
});
|
Maybe I'm misunderstanding the cause, but anyway @BridgeAR, thanks for the confirmation. |
@cola119 it is one possibility to trigger the issue. There are other ways to trigger the internal error as well. |
Landed in 550e814 |
PR-URL: #43683 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Darshan Sen <raisinten@gmail.com>
PR-URL: #43683 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Darshan Sen <raisinten@gmail.com>
PR-URL: #43683 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Darshan Sen <raisinten@gmail.com>
PR-URL: nodejs/node#43683 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Darshan Sen <raisinten@gmail.com>
Currently
util.inspect
has the inline access toSymbol.iterator
to 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
in
operator to be able to inspect correctly.