Repository navigation
util.inspect can throw when passed a Proxy #18036
Description
- Version: v9.3.0
- Platform: Linux daurn-z170 4.14.11-1-ARCH deps: update openssl to 1.0.1j #1 SMP PREEMPT Wed Jan 3 07:02:42 UTC 2018 x86_64 GNU/Linux
- Subsystem: util
> console.log(require("util").inspect(new Proxy({}, {"ownKeys": function() {}})))
TypeError: CreateListFromArrayLike called on non-object
at Function.getOwnPropertySymbols (<anonymous>)
at formatValue (util.js:422:24)
at Object.inspect (util.js:298:10)
As util.inspect is used by console.log in a recursive manner, such a proxy object acts as a "poison pill" for printing an object.
Activity
bnoordhuis commentedon Jan 8, 2018
Node.js 10 will have a
{showProxy: true}option that you can pass as the second argument toutil.inspect(). That could be back-ported to older releases if there is enough demand. It prints something like this:In general though, what you have there is a buggy proxy. Throwing an exception when it doesn't honor the contract is the right thing to do.
daurnimator commentedon Jan 8, 2018
IMO this new option should be the default for
console.log.Right. But how do I track down the bug? By printing the proxy (and things that may contain it)... For other issues related to proxys this is a good answer. But
console.logis one of the debugging primitives.I also think it's bad that such a buggy Proxy can act as a "poison pill" for trying to debug any surrounding code by default.
benjamingr commentedon Jan 8, 2018
I don't think we've ever made any throw-safety guarantees about
util.inspect:Is an easy example of how someone might do it. I think the point of proxies is to be able to override behavior like this - and if someone is using them they should know what they're doing when they override a hook like
ownKeys.Since this is
util.inspectwe should do what users find the most useful - but in terms of overall correctness I like the current (node 10) behavior.bnoordhuis commentedon Jan 11, 2018
The exception message could be better but that's a V8 issue. I took a quick look and it's probably not hard to fix but I don't have time. If anyone wants to work on that and needs some pointers, let me know.
Re: switching to
{showProxy: true}inconsole.log- I'd suggest opening a pull request and see how it's received.Barring further movement, I'll close this in a few days.
BridgeAR commentedon Jan 11, 2018
One problem with
showProxy: trueis that is it a huge performance overhead.I also think that the current behaviour is correct to throw a error.
TimothyGu commentedon Jan 11, 2018
@bnoordhuis
Ironically, this has already been done by you on master in #16485.
@BridgeAR
I agree. Hence, #16485 (review).
bnoordhuis commentedon Jan 11, 2018
@TimothyGu Only in the repl, not console.log.
Does it? Not saying it doesn't but I haven't noticed it.
TimothyGu commentedon Jan 11, 2018
It does call into a C++ runtime function. But on the other hand, calling Proxy hooks isn’t too fast either. I’m not sure which one is faster, depends on the circumstances probably.
bnoordhuis commentedon Jan 17, 2018
No further movement. I'll go ahead and close this out.