Make build_backtrace public

[ABBAPOH]

When using QuickJS as a JS engine it is desired sometimes to get a backtrace from outside JS and present it to user

[saghul]

I don't think it's a good idea to make this public. What problem do you need to solve?

[ABBAPOH]

We do some additional checks with the JS code - e.g. highlight an attempt to access filesystem functions or calling processes from contexts where users are not supposed to do so (e.g. do not call compiler during the "configure" stage).
In order to provide more info, we need a backtrace since it might not be clear how users end up doing what they did.
Basically, we need it^^
https://github.com/qbs/qbs/blob/master/src/lib/corelib/language/scriptengine.cpp#L344
We can always apply some changes in our fork though.

[saghul]

Would it help if you could create an Error object which collected the backtrace right there? The C equivalent to new Error() in JS, that is.

[ABBAPOH]

Do you mean JS_MakeError? Old API didn't have that...
We could probably use it, but that would require adding file/line numbers to its signature.
I can postpone this until I am sure there is no other way. We do create a temp object after all, similar to what JS_MakeError does...

[saghul]

Do you mean JS_MakeError? Old API didn't have that...

KInd of, yeah.

We could probably use it, but that would require adding file/line numbers to its signature.

How come? Won't the engine get it right since it's running the source? Would the numbers be wrong if you computed the stack trace right there and then? I suppose the 1st frame would just be <native> since you generated the error object there...

[bnoordhuis]

This PR does not expose the JS_BACKTRACE_FLAG constants. Should we? I'm unsure.

It's odd though that you need to pass in the filter_func argument even though it's never used because you can't specify JS_BACKTRACE_FLAG_FILTER_FUNC.

[jiangzhhhh]

没必要暴露build_backtrace这个函数。
只需要使用c api完成执行new Error("").stack就可以获得当前堆栈了，大概如下：

There is no need to expose build_backtrace.
Simply execute 'new Error("").stack' using c api to get the current stack, like this:

    // new Error("").stack
    JSValue message = JS_NewString(ctx, "");
    JSValue argv[] = {message};
    JSValue error = JS_CallConstructor(ctx, error_t, 1, argv);
    JS_FreeValue(ctx, message);
    JS_FreeValue(ctx, error_t);
    JSValue stack = JS_GetPropertyStr(ctx, error, "stack");
    JS_FreeValue(ctx, error);
    const char* cstr = JS_ToCString(ctx, stack);
    std::string ret;
    if (cstr) {
        ret = cstr;
        JS_FreeCString(ctx, cstr);
    }
    JS_FreeValue(ctx, stack);
    return ret;

[ABBAPOH]

So, I took another look and I noticed that we pass additional filename and line_num to the build_backtrace function adding info where the script on RHS of the property is located:

Product {
    name: { // we explicitly add this line to the stack
        foo = function() {
            return File.directoryEntries(sourceDirectory, File.Files)[0]; // warning with the backtrace is "triggered" here
        }
        return foo();
    }
}

Any ideas how to achieve this without using build_backtrace?

[bnoordhuis]

That's only done for parse errors, not runtime errors. I think that speaks for not exposing build_backtrace directly but only through a public API wrapper that takes fewer arguments.

I do wonder now if Error.prepareStackTrace always works since we're skipping creating a CallSite object when no filename is passed in. @saghul?

[ABBAPOH]

So, I exposed the JS_MakeError function and created an error:

const JSValue exVal = JS_MakeError(m_context, JS_PLAIN_ERROR, "", true);
The stack seems to be fine (except we lose that additional line, but it's OK I suppose). This however also requires exposing the JSErrorEnum.
Maybe add a simpler function? Something like JS_MakePlainError(JSContext *ctx, const char *message)?

[saghul]

If that works, can you try this? #809

Then just creating a new Error with JS_NewError should be equivalent.

[saghul]

I do wonder now if Error.prepareStackTrace always works since we're skipping creating a CallSite object when no filename is passed in. @saghul?

filename != NULL is only used for parse error. Since we failed at parsing I guess prepareStackTrace is not expected to function.

[ABBAPOH]

If that works, can you try this? #809

Yup, this also works, thanks.

[saghul]

Excellent! Shall we close this then?

[ABBAPOH]

Sure

[bnoordhuis]

FWIW, I verified that Error.prepareStackTrace functions correctly for parse errors thrown from (for example) eval().