Fix Error.stackTraceLimit = Infinity #861
Conversation
ammarahm-ed
force-pushed
the
fix-stacktracelimit
branch
from
e458251 to
d80d0f1
Compare
|
It seems that this works on mac but fails on Linux and Windows. |
|
Yes it seems like you'll have to go back to explicitly testing for infinity. |
ammarahm-ed
force-pushed
the
fix-stacktracelimit
branch
from
038177a to
340a457
Compare
|
@saghul can you start the tests again |
|
Hey @saghul I didn't look closely at your diff in review, fixed now and all tests now pass. |
|
On a second thought, if we want to 100% mimic how v8 behaves we should store the JSValue and extract it when necessary. Right now if you set it to -Infinity you'll get back 0, whereas Chrome gets you back the same value you set. Thoughts @bnoordhuis ? |
|
That means calling JS_ToFloat64, which can throw an exception, when we're already in an exception state. What is the expected behavior of this? Error.stackTraceLimit = {valueOf(){ throw "evil" }}
try{ throw new Error("fail") }
catch(e){ console.log(e) } // "fail" or "evil"? |
|
I Node one gets "fail". We do save the previous exception not to clobber it, so it seems like we should be good to do that? |
|
Yeah, that seems reasonable. Let's follow what node/chrome do then. FWIW, I'm okay with landing this PR as-is and then one of us doing a follow-up fix-up. |
|
Sounds good! I can work on it. |
Closes #858