Record stack trace for non-object exceptions#805
Record stack trace for non-object exceptions#805bnoordhuis merged 4 commits intoquickjs-ng:masterfrom
Conversation
|
@bnoordhuis Do you reckon we could perhaps wrap that in some special object (with toString() returning the wrapped value) so we can attach the stack property instead? |
|
I'm not quite sure what you're asking. What does "that" in "wrap that" refer to? |
|
The primitive, so basically if you throw "foo" we wrap foo into some object class so we can attach the stack, but the objet's toString returns "foo". |
|
You mean in a way that's observable to JS? No. It needs to be some kind of side table/lookup/thingy. |
|
I though we could keep it in the engine and not surface it but use it as a way to store the stack a bit more "cleanly". I haven't really thought this all the way through 😅 More of an out-loud thinking. If / once you think this is ready let's go with it. |
|
What about public API? I added a function similar to JS_GetException that "steals" current backtrace. Any other options? |
| // FIXME(bnoordhuis) Missing `sf->cur_pc = pc` in bytecode | ||
| // handler in JS_CallInternal. Almost never user observable | ||
| // except with intercepting JS proxies that throw exceptions. | ||
| dbuf_printf(&dbuf, " (missing)"); |
There was a problem hiding this comment.
@saghul ☝️ is a pre-existing bug that is more visible now
An example is:
- the OP_define_field handler in JS_CallInternal, that
- calls JS_SetProperty, that
- calls a proxy method, that
- calls JS_CallInternal again, that
- raises an exception, that then
- calls build_backtrace
The JSStackFrame from (4) has cur_pc set but not the one from (1).
For that particular bytecode handler it's an easy fix of doing sf->cur_pc = pc right before the JS_DefineProperty call but we should do a more rigorous review because it's definitely not the only affected bytecode handler.
There was a problem hiding this comment.
Good catch! I went through a bunch and fixed them. The abort was there to help us fix whatever else we might have missed 😅
Consider this script that throws an uncaught exception:
function f() {
throw "fail"
}
f()
Running with `qjs --script t.js` used to print merely:
fail
But now prints:
fail
at f (t.js:2:11)
at f (t.js:4:1)
qjs has been updated but no public API yet because I still need to think
through the corner cases.
Refs: quickjs-ng#799
Eventually, maybe. Not right now but you can of course expose it in your local fork. |
Consider this script that throws an uncaught exception:
Running with
qjs --script t.jsused to print merely:But now prints:
qjs has been updated but no public API yet because I still need to think
through the corner cases.
Refs: #799
Some caveats:
ctx->error_back_traceandrt->current_exceptionto get out of sync when another exception is thrown without callingbuild_backtrace(), meaning you might see the previous exception's stack trace, but only if a lot of preconditions are met.I'd say that's good enough for qjs for now? No stack trace is even less helpful.
ctx->error_back_traceis whateverError.prepareStackTracereturns. No different fromnew Error().stackbut I'd be remiss if I didn't point it out.I just realized this needs some tweaks to interact properly with
Error.captureStackTrace. Tomorrow, not tonight.