Throw in async function after await is not caught by debugger

[fatcerberus]

While debugging is enabled, if one throws an error inside of an async function:

async function f()
{
    throw new Error("*munch*");
}
f();

Normally, such an error would be intercepted and trigger a breakpoint (JsDiagDebugEventRuntimeException):

D:\temp>ssj -r error.js
SSj X.X.X Sphere JavaScript debugger (x64)
the powerful symbolic JS debugger for Sphere
(c) 2015-2018 Fat Cerberus

starting 'D:/temp/error.js'... OK.
connecting to 127.0.0.1:1208... OK.
establishing communication... OK.
downloading game information... OK.
    engine: miniSphere X.X.X
    title:  error.js
    author: Author Unknown

uncaught: Error: *munch*
   at f (@/error.js:4:2)
   at Generator.prototype.next (native code)
   at Global code (@/error.js:6:1)
-> # 0: f(), at @/error.js:4
4       throw new Error("*munch*");

@/error.js:4 f()
(ssj) l
      1 async function f()
      2 {
      3         //await null;
->    4         throw new Error("*munch*");
      5 }
      6 f();

However, if one were to uncomment the await in the code above, no breakpoint will be triggered and the promise is just silently rejected:

D:\temp>ssj -r error.js
SSj X.X.X Sphere JavaScript debugger (x64)
the powerful symbolic JS debugger for Sphere
(c) 2015-2018 Fat Cerberus

starting 'D:/temp/error.js'... OK.
connecting to 127.0.0.1:1208... OK.
establishing communication... OK.
downloading game information... OK.
    engine: miniSphere X.X.X
    title:  error.js
    author: Author Unknown

SSj/Ki debug session disconnected normally.
the SSj debugger has been detached.

Update (6/26/2018):
There are multiple work items here & I wanted to break them out independently. Will leave this open to track getting this to work w/ async/await. The other issues are:

• PR: fixing issue where "uncaught" exceptions in promises wouldn't notify debugger fixing issue where "uncaught" exceptions in promises wouldn't notify debugger  #5328
• Issue: Debugger: Support detecting pending promise reactions for "break on uhandled exceptions" Debugger: Support detecting pending promise reactions for "break on uhandled exceptions" #5371
• Issue: Debugger: Add support for breaking in debugger on unhandled promise rejection Debugger: Add support for breaking in debugger on unhandled promise rejection  #5370

RE async/await, it looks like we'll need to do some work to update byte-code-generation & possibly jit code so that we understand syntactic constructs that result in a "handled" async functions.

[fatcerberus]

I've found that if I enable first-chance exceptions, I will get called back for a throw after an await but in that case uncaught is set to false, i.e. the Chakra debugger erroneously considers the error to be handled. I'm trying to look into the cause of this but I haven't turned up any leads yet.

[fatcerberus]

Nevermind, I was wrong - the error after the await doesn't generate a first-chance exception either.

[akroshg]

@fatcerberus the throw new Error("*munch*"); will not be executed unless you call the .then(...
on ch.exe you see the first chance exception will be generated when you do

/**exception(all):stack();**/

async function f()
{
    await null;
    throw new Error("*munch*");
}
f().then( function (result) {
    console.log('here 1 ' + result);
},
function (error) {
    console.log('here 2 ' + error);
});

same is true if you debug in the Edge using F12.

[fatcerberus]

@akroshg That doesn’t seem right: #4608 implements uncaught rejection notifications and if what you say is true that means we won’t get a rejected promise notification for this code. But my own experience has proven the opposite.

[akroshg]

I am not sure about #4608, but what I meant was it generates the first chance exception. Is that not what you see?

[fatcerberus]

You say the throw won’t be executed until someone adds a .then() to the promise, which doesn’t make sense since in that case #4608 wouldn’t work (it does).

[akroshg]

hmm, I must be mistaken then.

[rhuanjl]

Comparing this with HostPromiseRejectionTracker (#4608) is a bit of a misnomer. The detection mechanisms are totally different.

With HostPromiseRejectionTracker the trigger is whenever a promise is rejected to check if it's got a handler or not - to be honest I have no idea what the trigger the debugger uses is.

The above case:

async function f()
{
  await null;
  throw new Error("*munch*");
}
f();

Roughly de-sugars to:

function f()
{
  var p = Promise.Resolve(null);
  var prom;
  p.then(()=>{
    try{
      throw new Error("*munch*");//
    }
    catch(e){
      prom = Promise.Reject(e);//-> rejected promise with no handler
      // -> triggers HostPromiseRejectionTracker
    }
  }
  return prom;
}
f();

(I note that there should be 1 more try catch block and at least one more throwaway promise to write that fully but it just gets unnecessarily messy)

The throw definitely fires - but the debugger does not flag for it - you can catch and log the error if wanted of course.

[fatcerberus]

@rhuanjl Right, I know they’re different mechanisms. I was just referring specifically to akroshg’s assertion that the throw is not executed at all, which clearly isn’t the case or else rejection tracking would be pointless.

[fatcerberus]

This seems to be the culprit:
https://github.com/Microsoft/ChakraCore/blob/002e9be2aa8c8ea29ce37a605c7f6c6baa2890bd/lib/Runtime/Debug/ProbeContainer.cpp#L1026-L1029

In the case of the following code:

async function f() {
    await null;
    throw new Error();
}
f();

When deciding whether to pass this exception onto the debugger, ChakraCore considers the thrown exception to both:

1. Have a catch handler, --AND--
2. Be both thrown and caught in user code

While 1) is true, 2) most definitely isn't as there is no catch in the async function. The exception is caught directly by the runtime in order to reject a promise.

[rhuanjl]

I've taken a quick look at this, and I believe that the issue is not Async functions at all but rather the way Promise reactions are implemented

An alternative repro of this issue would be:

Promise.resolve().then(()=>{throw new Error("hunger increases");});

No break point is seen in the debugger for this code.

In JavascriptPromise.cpp the following function call occurs in 2 different methods that handle promise reactions:

Js::JavascriptExceptionOperators::AutoCatchHandlerExists autoCatchHandlerExists(scriptContext);

I believe from looking over the code that this turns off debugger break points until it's disabled again I assume there is a purpose for this - but I'll be honest I don't really understand some parts of this code at the moment, could the fix perhaps be to only set that if the promise IsHandled?

[mike-kaufman]

See PR #5328 for a fix here a fix for exceptions in promises w/out handlers hooked up.