Chakracore: Detect unhandled promise rejection

[saschanaz]

http://www.ecma-international.org/ecma-262/7.0/#sec-host-promise-rejection-tracker

The following should log an error or warning on console:

new Promise((resolve, reject) => reject()) 

(Moved from UserVoice)

[liminzhu]

The spec reads as implementation-defined behavior. Edge & ch.exe don't give warning/error, Chrome (v8) throws, and Node 6 (also v8) gives warning. @bterlson is this supposed to be host-defined behavior or JS VM-defined behavior?

[bterlson]

There is no spec mandated work here. The promise rejection tracking is implementation defined. We could consider adding a rejection tracking mechanism to ch.exe. If this is a request for the dev tools (F12) to add rejection tracking capabilities, then this is out of scope for this repo I think.

[avonwyss]

A callback would seem appropriate (to match JsSetPromiseContinuationCallback), so that the host can decide whether and how to deal with these.

[albertogasparin]

Additionally, it would be great to trigger unhandledrejection and rejectionhandled events, like currently suggested by the html spec:
https://html.spec.whatwg.org/multipage/webappapis.html#unhandled-promise-rejections

[bterlson]

@albertogasparin assuming you're talking about precisely those events and in the browser, that's up to Edge to set up for us. If you file a uservoice request or similar, please post it here so others can pile on :)

[saschanaz]

There already is a UserVoice request which got 'Oops, wrong forum' response.

[bterlson]

@SaschaNaz gah, thank you. I will correct that presently!

[bterlson]

@SaschaNaz @albertogasparin the uservoice for the Edge side of things has been corrected. You can now spend your points there :)

[fatcerberus]

Throwing my hat in the ring to see this implemented, I've started to use async and await heavily and it gets confusing when an unforeseen error in an async function goes unnoticed. I'd have to wrap every single async function in try/catch to make sure I see the errors, which is frustrating as I don't have to do that for sync code--it will just crash the process which is plenty noisy enough to find the bug. 😄

Interestingly, when using the debugging API, uncaught rejections are intercepted, which suggests that the hooks to support this are already in place. Even if it were just something as simple as "call a host callback on unhandled rejection" that would be great.

[rhuanjl]

EDIT: below text (original comment by me) is based on a misreading of the spec - left for posterity but it's wrong.

Adding an extra voice to this discussion, per spec it sounds like the engine should call HostPromiseRejectionTracker (25.4.1.9 from the spec) when a promise is rejected, and then the host can choose what to do in that function - however searching through ChakraCore source I cannot see any exposed hook for this or any equivalent hook to allow a host that uses JSRT to define a rejection handler.

[bterlson]

@rhuanjl do note that we are conformant with 25.4.1.9 - the default implementation of that hook is to do nothing, and since it's unobservable to script whether that hook is actually used for anything, it may not actually exist in any given implementation. That API is only there to explain how hosts who choose to implement promise rejection tracking should do so.

[rhuanjl]

@bterlson I may have got this wrong - my initial read of 25.4.1.9 was that the engine i.e. ChakraCore should provide a hook for HostPromiseRejectionTracker such that a host (be that a browser or any other application using the engine) can implement a handler for it if they so wish.

My potential confusion comes down to reading "implementation" on the first line of the spec paragraph as "host" when I suppose it should be read as "engine" (in the context of my logic above).

Sorry for the mistake - comment above revised.

[fatcerberus]

@rhuanjl I think in the context of the ES spec "implementation" should be read as "JavaScript implementation" - i.e. Chakra in this case.

[bterlson]

Indeed, "implementation" ~= engine ~= chakracore, and "host" ~= whatever is hosting the engine ~= Edge, Node, your own app, etc.

Regardless, the default behavior is to do nothing, which we presently comply with :-P

[bterlson]

Oh, I would also suggest always referring to the latest spec snapshot rendered here: https://tc39.github.io/ecma262.

[rhuanjl]

I'd be interested in creating an API within JSRT to respond to unhandled promise rejections, I would be quite happy to send a Pull Request for the below idea if it would be appreciated.

I was thinking it could be useful for a host to be able to choose to:
a) ignore them (as done now) - backwards compatibility and may be best for released products
b) have ChakraCore treat the unhandled promise rejection as an error and throw it
c) call a host defined API for it

My proposal would be to implement a JSRT function that allows a host to set any of the above 3 behaviours (option (a) would be set by default if the function is not called)

I understand from the above discussion that it is not required by spec to implement this behaviour and is not a priority of the team but would you be happy to welcome a PR to do this from an external contributor?

[bterlson]

@liminzhu any thoughts on @rhuanjl's proposal? I think this could be a welcome addition... perhaps worth looking at how NAPI handles this as well.

[liminzhu]

@digitalinfinity how do we emit unhandledRejection event in NodeChakraCore with the current set of APIs? Wonder if there's an existing solution.

[MSLaguana]

We do not, currently. At least not at the engine level. Some userspace libraries (such as zone.js) try to detect this from javascript, which doesn't always work properly. See https://github.com/Microsoft/ChakraCore/issues/3827 for some discussion of one case.

[fatcerberus]

It's possible to polyfill by monkey-patching the Promise constructor. That doesn't work for async/await though, as async functions always use the native Promise.

[liminzhu]

Thanks @MSLaguana . It makes sense to me to expose a hook for unhandledRejection in the engine.

@bterlson are there any other cases/events we should consider? AFAIK there's at least rejectionhandled.

@agarwal-sandeep does the debugger need something like that as well?

[bterlson]

@liminzhu sorry I just came across this in my inbox :) The two events in HTML are unhandledrejection and rejectionhandled. Not aware of any other events that are needed.

[rhuanjl]

Just wanted apologise that I haven't followed this up with a PR yet, I spent some time looking into it and it appears to be a lot more complicated than I at first thought, I will try and give it some more time in the next week or two, and will hopefully be able to submit a PR at some point in the next month or so - I can't guarantee this though.

[rhuanjl]

OK, I think I've got a handle on this now, will aim to open the PR by the end of the weekend.

• [x] implement basic machinery for HostPromiseRejectionTracker
• [x] put in the hook for unhandledrejection
• [x] put in the hook for rejectionhandled
• [x] handle async functions correctly... sortof...
• [x] track the implementation through ch
• [x] write tests

Not yet sure how to go about tracking the change through ch - intending to add it as a command line option so will have to figure out how that works.

[rhuanjl]

@saschanaz HostPromiseRejection tracking is now in CC master, I've no idea of the release schedule for when this will make it to a release also would take the Edge team implementing the feature in Edge for it to track through to there; but I've done what I can here CC now has the hook for this.

[rhuanjl]

This was implemented some time ago, and was shipped in version 1.10 of Chakracore - unclear on if it was ever tracked through into Edge but that is now a moot point.