Uncaughtable error seems to be caughtable when it occurs inside a Promise, is that an expected behavior?

[laishere]

The uncaughtable error can be thrown by an interrupt handler.

Noticed that the fail logic of js_async_function_resume function doesn't check if an exception is caughtable before sending it to the reject function of the Promise.

I wonder if it is an expected behavior.

https://github.com/bellard/quickjs/blob/6e2e68fd0896957f92eb6c242a2e048c1ef3cae0/quickjs.c#L19236-L19239

[XuJiandong]

it can cause this issue: #363

[bnoordhuis]

Can you post a simple example and specify what you think ought to happen and what actually happens?

If this is a dup of #363, then it was fixed a few months back in quickjs-ng in quickjs-ng/quickjs@7ad9807.

[laishere]

An uncaughtable is an error triggered by an interrupt handler:

/* Example: using interrupt handler to limit the execution time. */

static int exec_timeout_check_handler(JSRuntime *rt, void *opaque)
{
    if (timeout) return 1;
    return 0;
}
/* ... */
JS_SetInterruptHandler(rt, exec_timeout_check_handler, opaque);

When running non-async logic via JS_Call, the error won't be caught by try-catch as expected.
However, when running async logic via JS_ExecutePendingJob, the error can be caught by try-catch or Promise.catch, which will continue the logic if try-catch is used regardless of timeout.

Check the differences in the source code:

JS_CallInternal
https://github.com/bellard/quickjs/blob/6e2e68fd0896957f92eb6c242a2e048c1ef3cae0/quickjs.c#L18664C10-L18664C31

js_async_function_resume
https://github.com/bellard/quickjs/blob/6e2e68fd0896957f92eb6c242a2e048c1ef3cae0/quickjs.c#L19236-L19239

I think we should change js_async_function_resume's return type from void to int, and return the errno other than calling the resolve funcs when JS_IsUncatchableError is true.

[bnoordhuis]

Does it work with this patch applied?

diff --git a/quickjs.c b/quickjs.c
index a503eab..0a81a7f 100644
--- a/quickjs.c
+++ b/quickjs.c
@@ -18005,9 +18005,12 @@ static void js_async_function_resume(JSContext *ctx, JSAsyncFunctionData *s)
     if (JS_IsException(func_ret)) {
         JSValue error;
     fail:
+        ret2 = JS_UNDEFINED;
         error = JS_GetException(ctx);
-        ret2 = JS_Call(ctx, s->resolving_funcs[1], JS_UNDEFINED,
-                       1, &error);
+        if (!JS_IsUncatchableError(ctx, error)) {
+            ret2 = JS_Call(ctx, s->resolving_funcs[1], JS_UNDEFINED,
+                           1, &error);
+        }
         JS_FreeValue(ctx, error);
         js_async_function_terminate(ctx->rt, s);
         JS_FreeValue(ctx, ret2); /* XXX: what to do if exception ? */

(Diff against quickjs-ng; you may have to massage it a little for this repo)

[laishere]

I think it will work in some way, but I don't think that's a complete fix. As js_async_function_resolve_call who calls js_async_function_resume always returns JS_UNDEFINED. So we should change that too.

I will post my test here later.

[laishere]

As my project already switched to quickjs-ng, I will continue this issue there quickjs-ng/quickjs#810
Test code and solution is posted.