Debugger doesn't catch error thrown inside for...of loop immediately

[fatcerberus]

I have JsDiagBreakOnExceptionAttributeUncaught enabled. Normally, errors are caught as soon as they're thrown. I've noticed, however, that if an error is thrown inside a for...of loop, entire stack frames can be unwound before the error is intercepted, losing valuable debugging information in the process. Here's an example, illustrated by my SSj debugger:

D:\src\spectacles-i>ssj dist
SSj X.X.X Sphere JavaScript debugger (x64)
the powerful symbolic JS debugger for Sphere
(c) 2015-2017 Fat Cerberus

starting 'D:/src/spectacles-i/dist/'... OK.
connecting to 127.0.0.1:1208... OK.
establishing communication... OK.
querying target... OK.
    game: Spectacles: Bruce's Story
    author: Fat Cerberus

=> # 0: [anon] at src/main.js:6
6 RequireSystemScript('persist.js');

src/main.js:6 [anon]
(ssj) c
UNCAUGHT: Error: The pig ate everything at 8:12
   at Anonymous function (src/main.js:45:12)
   at game (src/main.js:45:4)
=> # 0: [anon] at src/main.js:49
49      Console.initialize({ hotKey: Key.Tilde });

src/main.js:49 [anon]
(ssj) l 20
     39
     40         persist.init();
     41
     42         var evens = from([ 1, 2, 3, 4, 5, 6 ])
     43                 .where(it => it % 2 == 0);
     44         for (let even of evens) {
     45                 (() => { throw new Error("The pig ate everything at 8:12"); })();
     46                 SSj.log(even);
     47         }
     48
=>   49         Console.initialize({ hotKey: Key.Tilde });
     50         Console.defineObject('yap', null, {
     51                 'on': function() {
     52                         Sphere.Game.disableTalking = false;
     53                         Console.log("oh, yappy times are here again...");
     54                 },
     55                 'off': function() {
     56                         Sphere.Game.disableTalking = true;
     57                         Console.log("the yappy times are OVER!");
     58                 },

src/main.js:49 [anon]
(ssj) bt
=> # 0: [anon] at src/main.js:49

Notice the stacktrace of the Error object includes the arrow function, but the backtrace output does not, and the breakpoint actually triggered later, completely outside the for...of loop.

If I move the throw outside the loop, then it works as expected:

src/main.js:6 [anon]
(ssj) c
UNCAUGHT: Error: The pig ate everything at 8:12
   at Anonymous function (src/main.js:42:11)
   at game (src/main.js:42:3)
=> # 0: [anon] at src/main.js:42
42      (() => { throw new Error("The pig ate everything at 8:12"); })();

src/main.js:42 [anon]
(ssj) bt
=> # 0: [anon] at src/main.js:42
   # 1: [anon] at src/main.js:42

[fatcerberus]

There's also this oddity:

src/main.js:6 [anonymous function]
(ssj) c
UNCAUGHT: Error: The pig ate everything at 8:12
   at Anonymous function (src/main.js:43:12)
   at game (src/main.js:43:4)
=> # 0: [anonymous function] at src/main.js:44
44              SSj.log(even);

src/main.js:44 [anonymous function]
(ssj) l
     39
     40         var evens = from([ 1, 2, 3, 4, 5, 6 ])
     41                 .where(it => it % 2 == 0);
     42         for (let even of evens) {
     43                 (() => { throw new Error("The pig ate everything at 8:12"); })();
=>   44                 SSj.log(even);
     45         }
     46
     47         persist.init();
     48

src/main.js:44 [anonymous function]
(ssj) bt
=> # 0: [anonymous function] at src/main.js:44

It paused execution on a statement that shouldn't even be reached. 😕

[agarwal-sandeep]

Same behavior as Edge browser for arrow functions so this is not specific to JsRT debugger but a runtime issue.
@akroshg to triage.

[fatcerberus]

The arrow function is a bit of a red herring. I put it in to illustrate but the actual problem is that an exception thrown from inside a for...of loop often causes a partial unwind before the debugger intercepts it. I've seen this even with regular functions, so it's not specific to arrows.

[agarwal-sandeep]

yes this is issue with for..of nothing related to arrow function. Also doesn't repro with for..in

[akroshg]

we convert

for (var i of item) {
  ...
}

to similar (not exactly same)

for (var i of item) {
  try {
  ...
  } catch (e) {
    throw e;
  }
}

for iterator close support. In the debug mode we actually re-throw in the catch. (this code is hidden). But in the debugger as the code is hidden we take the last statement in the for..of.

[fatcerberus]

@akroshg Yeah, but in my case the breakpoint doesn’t trigger until after the entire stack frame of the function containing the for...of has been unwound.

[zenparsing]

Test case (with -debuglaunch):

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

for (let x of [1, 2, 3, 4]) {
  (() => { throw new Error('Exception thrown from within for-of')})();
  print('what the what');
}

It appears that since we desugar for-of using try-catch, the exception handling code thinks that the exception thrown inside of the for-of loop body above is "handled" (see AutoCatchHandlerExists). It seems that we need to give the runtime a way to know that a given try-catch should be considered "internal", perhaps by introducing a new opcode (e.g. TryCatchInternal).

[fatcerberus]

@zenparsing Just for the record, I had made a pull request to remove the AutoCatchHandlerExists for async functions a while back for a similar reason (errors were not being intercepted by the debugger), but end up closing it as it got too much pushback and there were also unwanted false positives: #4691

[rhuanjl]

@fatcerberus AutoCatchHandlerExists is the method used for preventing a debugger break inside a try/catch block and inside an async function - the commonality of method doesn't mean the fix is the same here. This one is to do with the internal sugaring of for-of (see the explanation I wrote out at #6314) and could be fixed fairly simply - though the fix I can think of won't fix #6314 a better/more complex fix will be needed to do both.

The Async function issue is rather different - there isn't currently a way of determining if the async function's promise is going to be handled or not so if the debugger breaks on an exception in an async function it may be one that had a handler or it may not - removing the AutoCatchHandlerExists from async functions therefore meant the debugger breaking when it shouldn't (as well as when it should).