-
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: fix util.inspect with proxied function #25244
Conversation
It does not hurt to do this but it will not solve the overall issue with proxies: they are difficult to reason with and even more difficult to write in a way that does not interfere with things. We trigger multiple traps while inspecting an object and we can not work around all of the issues that might potentially come up. Another example that would fail is using an empty array instead of the function. |
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. I agree with Ruben's comment but this PR is still an improvement over the status quo so why not?
@BridgeAR Agreed. We can not fix all these issues. For example, an array with a weird proxy is hard to fix. But at least we can make it a little better :-) |
Landed in 6c7c77e |
PR-URL: nodejs#25244 Fixes: nodejs#25212 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Minwoo Jung <minwoo@nodesource.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Fixes: #25212
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes