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.

[hongzhidao]

Hi @laishere,
It looks like not an issue.
#395

[laishere]

@hongzhidao This is not the same problem. The case you mentioned in another issue is actually a normal error which is catchable if you use catch after then. However, the interruption should never be caught by design. For example, when we use interruption to check if the program cost too much resource like cpu time, we wouldn't want it to ignore the timeout regulation by catch the error. It's the same behavior for non promise operations, but it wasn't being properly implemented for promise.

[hongzhidao]

Good to know, thanks.