[Question] Incorrect line when throwing string

[ABBAPOH]

Let's say I have a file test.js

function foo() {
    var path = "foo";
    if (true)
        throw "fail1";
    return path;
}
foo();

Now I call JS_EvalThis on this piece of code and get a backtrace

    at /Users/abbapoh/Developer/qbs/qbs/tests/auto/language/testdata/test.js:1:1
    at <eval> (/Users/abbapoh/Developer/qbs/qbs/tests/auto/language/testdata/test.js:7:1

When changing the line to new throw Error("fail1");, I get test.js:4:19, as expected.
In order to catch strings as errors, we added a wrapper to turn them into Errors like this https://github.com/qbs/quickjs-ng/blob/qbs/quickjs.c#L17483-L17489
This used to be working with the old engine. From what I debugged, find_line_num can't guess the line anymore.
Am I doing this wrong?

[bnoordhuis]

This used to be working with the old engine

What engine/commit is that? bellard/quickjs@...? And what API did you use to obtain that stack trace?

[ABBAPOH]

Yes, Bellard's one.
We get the "stack" property from the exception object and parse it using regexps^^

[bnoordhuis]

You're patching quickjs.c so I suspect it's something on your side. I don't think bellard/master and quickjs-ng have diverged in that respect.

[ABBAPOH]

I tried applying the patch on top of vanilla master and then run git bisect with those changes.
I bisected the behaviour change to this commit 73cc00e.
Though, the reported line is not entirely correct (from the next instruction?), it's not 1.
I used the qsj binary with the above script to test.

[bnoordhuis]

Can you demonstrate the issue with an unmodified copy of quickjs? Because I'd be happy to take a look in that case.

[ABBAPOH]

That lead us to the second issue - QuickJS does not record stack trace when throwing primitive types.
We do throw strings a lot (rather than throw new Error("message") mostly from user code, but also when reporting errors from c++ extensions.
Maybe fix that first? It might be our patch is not entirely correct.
nodejs reports the line in the example above; this also worked with v8 engine.

[bnoordhuis]

nodejs reports the line in the example above; this also worked with v8 engine.

I'm aware - I wrote that code for node circa 2010, 2011 :-)

Recording the stack trace for primitives: I opened #805. It's not perfect yet but it's 80% there.

[ABBAPOH]

Looks promising! I'll try updating out code to extract backtrace from the new variable and see it it works in our tests.

[saghul]

TBH I'm not 100% convinced this needs fixing. Node seems to be the only thing out there which gets it right:

[ABBAPOH]

That works, thanks! And the lines are correct now, as they were.

[bnoordhuis]

I'm not 100% convinced this needs fixing

The reason I added it to node back then is because people sometimes throw what they think is an exception object but isn't, and that's very hard to debug when you don't get a stack trace. It's a quality-of-life improvement.

[saghul]

Ah I can see how that makes sense!

How did you do that?

[bnoordhuis]

In node? Honestly, I don't remember, too long ago. It's possible V8 already had an API for it but maybe I added one.