Discussion: Report unhandled promise rejections

[dodgerblue]

Hello,

If my understanding is correct, in the jerryscript implementation, if an error is thrown when resolving a promise, it will only be passed to the rejection handler (if there is one registered). If there is no promise rejection registered, the error is silently ignored. I am looking for a way to make that error return to the calling function, or some other way to signal it.

My approach was to change ecma_reject_promise() to take an additional out parameter telling whether the rejection was handled or not. ecma_promise_trigger_reactions() sets this parameter to false in case the reactions collection was empty from the start. Based on this, ecma_promise_reject_handler() will return an error for unhandled rejections.

This works for a snapshot that was loaded, with all the chained promises' reactions registered. In this situation, when a .then() handler executes, we can determine whether it has a subsequent .catch() rejection handler chained. If it doesn't, ecma_reject_handler() returns with an error as expected.

This doesn't work for direct function calls. For instance, a scenario like foo().then(), where foo() calls into a C implementation that returns a rejected promise immediately via jerry_resolve_or_reject_promise(). At this point, I noticed ecma_promise_do_then() wasn't called yet to register the handlers and schedule the reactions.

I would like opinions on this approach. Do you think the current jerry design allows to signal unhandled errors thrown during exceptions, or it's always up the the developer to chain a .catch() call for thrown errors?

Thank you!

[akosthekiss]

Hi @dodgerblue ! Do you have a (preferably small) test case (a js script) that shows what you'd like to achieve?

The goal of JerryScript is to be spec-conforming, in this case ES2015-conforming. I'm not sure that what you outlined is proper use of ES2015 promises. (As a sanity check: what you'd like to achieve, does that work in another engine, e.g., in v8?)

[dodgerblue]

Hey @akosthekiss! Here's an example:

new Promise(function(resolve, reject) {
  resolve("Check");
}).then(function(val) {
  print(val);
  throw new Error("Boom");
});

If I run this in jerry (built with --profile=es2015-subset), I will only get the "Check" print statement on screen, and no info on the error. If I chain a .catch() function at the end, the error is correctly passed to it.

If I run above code in node (node --version v8.10.0) - and change the print to a console.log call - I get the error reported as an unhandled promise rejection:

Check
(node:14261) UnhandledPromiseRejectionWarning: Error: Boom
    at /home/repos/create_promise.js:5:9
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)
    at Function.Module.runMain (module.js:695:11)
    at startup (bootstrap_node.js:188:16)
    at bootstrap_node.js:609:3
(node:14261) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:14261) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

I went over the ECMAScript 2015 language specification and couldn't find info on unhandled promise rejections.

[akosthekiss]

It seems that the public API of JerryScript was designed to allow jerry_run_all_enqueued_jobs to return errors and jerry-main was written to handle (print) the returned errors as unhandled exceptions, but a bit of analysis shows that the implementation never returns errors uncaught by handlers.

But it also seems that this is in line with the ES2015 spec. I couldn't find a path in the algorithms that would end PromiseReactionJob with an abrupt completion status (sect. 25.4.2.1 steps 7.a&b), which would allow implementation-specific unhandled exception processing (sect. 8.4.2 step 1).

However, ES2016(!) seems to have support for implementation-defined unhandled rejection handling: see RejectPromise and HostPromiseRejectionTracker (sect. 25.4.1.7 step 7, and 25.4.1.9, https://www.ecma-international.org/ecma-262/7.0/#sec-rejectpromise). It uses a new promise internal slot to support that.

I'm not sure whether we can implement that in JerryScript at the moment, as that would be ES2015 incompatible (wouldn't it?).

[dodgerblue]

I need a clarification: which ECMA standard does jerryscript currently support? I assumed it's ES2015 - mostly due to the test suites, the --profile=es2015-subset and the native promises implementation. I rechecked the README.md file and ECMA 5.1 is mentioned.

Based on the description of HostPromiseRejectionTracker:

An implementation of HostPromiseRejectionTracker must complete normally in all cases. The default implementation of HostPromiseRejectionTracker is to do nothing.

The default implementation seems backwards compatible, so adding the new promise internal slot for above would not necessarily make JerryScript ECMA2015 incompatible. Users can enable reporting unhandled promise rejections based on a flag.

I was curious whether there are any plans of implementing this feature in JerryScript, or what's the likelihood of integrating such a change, design-wise.

Thank you!

[akosthekiss]

The current status is full ES5.1 support and partial ES6 (a.k.a. ES2015) support. That's why the profile is still called es2015-subset, as not all ES2015 features are supported yet.

As for whether/when/how to add promise rejection handling, I summon fellow maintainers, because that's a question that needs discussion in wider circles. CC @zherczeg @rerobika @dbatyai @galpeter

[chances]

Any news from discussions with maintainers?

I'm evaluating this JS engine for use as a scripting engine in a game. Exposing the same kind of warning as V8 would be useful for debugging purposes.

[akosthekiss]

summon @rerobika

[rerobika]

Hi @chances! I've just opened a PR which adds support for it, hopefully it will land soon.