Callback in JS_SetHostPromiseRejectionTracker is called even on handled promises

[rFlex]

There seems to be no reliable way right know to know when a promise is rejected but not handled. The engine exposes a JS_SetHostPromiseRejectionTracker to know when a promise is rejected. It provides a parameter is_handled that should tell whether the promise was handled or not. However in the following case, the callback will be called with the is_handled parameter set to 0, even if the JS code is actually handling the promise:

new Promise((resolve, reject) => {
  reject(42)
}).catch(() => {
})

This makes it impossible to make a distinction between the code above, and this code:

new Promise((resolve, reject) => {
  resolve();
}).then(() => {
  throw new Error('bad');
}

or even this code:

new Promise((resolve, reject) => {
  reject(42);
});

In all 3 cases, is_handled is 0, although in the former example, the callback is called twice: once with 0, once with 1, but I don't see a way to know whether a second callback should be expected.

In JavaScriptCore, the callback given to JSGlobalContextSetUnhandledRejectionCallback is only called in the last two examples. (https://github.com/WebKit/WebKit/blob/main/Source/JavaScriptCore/API/JSContextRefPrivate.h#L139)

[HarlonWang]

I'm facing the same problem. I think JS_SetHostPromiseRejectionTracker doesn't handle it well. I looked at the code. In this case：

(function(){
  return new Promise((resolve,reject)=>{
    reject(1)
  })
})().catch(() => {
  
});

Expected results:

// console
nothing to log

Actual results:

// console
Possibly unhandled promise rejection:

I found that the reason was host_promise_rejection_tracker before js_ promise_ catch called , so even if catch is used, it will be reported, some codes are as follows:

static void fulfill_or_reject_promise(JSContext *ctx, JSValueConst promise,
                                      JSValueConst value, BOOL is_reject)
{
    ......
    if (s->promise_state == JS_PROMISE_REJECTED && !s->is_handled) {
        JSRuntime *rt = ctx->rt;
        if (rt->host_promise_rejection_tracker) {
            rt->host_promise_rejection_tracker(ctx, promise, value, FALSE,
                                               rt->host_promise_rejection_tracker_opaque);
        }
    }

}

static JSValue js_promise_catch(JSContext *ctx, JSValueConst this_val,
                                int argc, JSValueConst *argv)
{
    ......
    return JS_Invoke(ctx, this_val, JS_ATOM_then, 2, args);
}

At present, I haven't thought of a relatively simple solution. One solution is to temporarily store rejection_tracker, which can be detected after JS_ExecutePendingJob.

It is better to hope that the official fix the problem :)

[HarlonWang]

I fixed it in a simple way. In the source code dependency of my own project, see for details: HarlonWang/quickjs-wrapper@c4f1ba9

[saghul]

Is that correct though? If you get an un-rejected one you discard the previous one.

[HarlonWang]

Is that correct though? If you get an un-rejected one you discard the previous one.

I don't quite understand what you mean. Can you explain it in detail? My solution is to temporarily store the unhandled reject, and remove it if it is subsequently handled, and I passed the test.

[saghul]

I was just wondering if that is the correct behavior, that is if a promise is rejected and then handled you'll get called twice first with a reject, then without, right?

What if those 2 events are separate? Aka a rejection on a timer and then a resolution on a different timer a bit later.

[HarlonWang]

I was just wondering if that is the correct behavior, that is if a promise is rejected and then handled you'll get called twice first with a reject, then without, right?

Yes, it will get called twice first with a reject, then without

What if those 2 events are separate? Aka a rejection on a timer and then a resolution on a different timer a bit later.

It will throw an exception immediately. I tested it in node, and the result is the same. I tested the chrome console to throw an exception, but it will continue to execute the timer

Test code:

var p = new Promise((resolve, reject) => { reject(1); });
setTimeout(() => {
	p.catch(() => { console.log(11); });
}, 1000);

I don't know what this situation should be. I think it may be more related to the platform implementation.

[saghul]

I was thinking about something like this:

async function bad() {
    console.log('BAD');
    throw 'AAAAAA';
}

async function good() {
    console.log('GOOD');
    return 42;
}

setTimeout(() => {
    bad();
}, 1000);

setTimeout(() => {
    good();
}, 2000);

But I think I know what you say now. In my example it only gets called once, which is correct because there is only one rejection in that chain.

But if I do this:

async function bad() {
    console.log('BAD');
    throw 'AAAAAA';
}

async function good() {
    console.log('GOOD');
    return 42;
}

setTimeout(() => {
    bad().catch(() => {
        console.log('CAUGHT');
    });
}, 1000);

setTimeout(() => {
    good();
}, 2000);

Then it gets called twice indeed!

[HarlonWang]

I test it, It has no problem.

Sorry, I may not really understand what your question is :)

[saghul]

Yeah sorry I got confused there 😅 It's clear now, I'd say your solution is indeed a nice one!

[bellard]

The new heuristics should work better.