[vm] Treat Future.then(..., onError:...) as catch all handler

Commit a52f2b9 which reworked awaiter stack unwinding and its
integration with debugger introduced the following regression:
it stopped treating `Future` listeners which had both `onValue`
and `onError` callbacks as catch all exception handlers. Only
listners with `onError` callback (those created with
`Future.onError`) were treated as a catch all handler.

This meant that debugger started to treat exceptions in the
code below as uncaught:

```
Future<void> foo() {
  await 0;
  throw '';
}

await foo().then(..., onError: (e, st) {

});
```

This change fixes this regression by checking if
`FutureListener.state & stateCatchError != 0` instead of
more narrow `FutureListener.state == stateCatchError` which
only detects listeners which only catch errors but do not
handle values. The new predicate matches
`FutureListener.handlesError`.

Fixes https://github.com/dart-lang/sdk/issues/53334

TEST=service/pause_on_unhandled_async_exceptions_test

Fixed: 53334
Change-Id: I374114b7a7b2ef86b7ed6bf7d5e7cf71ba6054dc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/323280
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>

runtime/observatory/tests/service/pause_on_unhandled_async_exceptions_test.dart

@@ -12,31 +12,49 @@ import 'service_test_common.dart';
 //
 // Update these constants by running:
 //
-//     dart runtime/observatory/tests/service/test_helper.dart update-lines
+// dart runtime/observatory/tests/service/update_line_numbers.dart <test.dart>
 //
-const int LINE_A = 41;
+const int LINE_A = 59;
 // AUTOGENERATED END
 
 class Foo {}
 
-doThrow() {
+String doThrow() {
   throw "TheException";
   return "end of doThrow";
 }
 
-asyncThrower() async {
+Future<void> asyncThrower() async {
   await 0; // force async gap
   doThrow();
 }
 
 testeeMain() async {
   try {
-    // caught.
+    // This is a regression case for https://dartbug.com/53334:
+    // we should recognize `then(..., onError: ...)` as a catch
+    // all exception handler.
+    await asyncThrower().then((v) => v, onError: (e, st) {
+      // Caught and ignored.
+    });
+
+    await asyncThrower().onError((error, stackTrace) {
+      // Caught and ignored.
+    });
+
     try {
       await asyncThrower();
-    } catch (e) {}
+    } on String catch (e) {
+      // Caught and ignored.
+    }
 
-    // uncaught.
+    try {
+      await asyncThrower();
+    } catch (e) {
+      // Caught and ignored.
+    }
+
+    // This does not catch the exception.
     try {
       await asyncThrower(); // LINE_A.
     } on double catch (e) {}
@@ -50,11 +68,15 @@ var tests = <IsolateTest>[
     var stack = await isolate.getStack();
     expect(stack['asyncCausalFrames'], isNotNull);
     var asyncStack = stack['asyncCausalFrames'];
-    expect(asyncStack.length, greaterThanOrEqualTo(2));
-    expect(asyncStack[0].toString(), contains('doThrow'));
-    expect(asyncStack[1].toString(), contains('asyncThrower'));
-    // There was no await'er for "doThrow()".
-  }
+    expect(asyncStack.length, greaterThanOrEqualTo(4));
+    await expectFrame(asyncStack[0], functionName: 'doThrow');
+    await expectFrame(asyncStack[1], functionName: 'asyncThrower');
+    await expectFrame(asyncStack[2], kind: M.FrameKind.asyncSuspensionMarker);
+    await expectFrame(asyncStack[3],
+        kind: M.FrameKind.asyncCausal,
+        functionName: 'testeeMain',
+        line: LINE_A);
+  },
 ];
 
 main(args) => runIsolateTests(args, tests,

runtime/observatory/tests/service/service_test_common.dart

@@ -270,6 +270,21 @@ IsolateTest setBreakpointAtUriAndLine(String uri, int line) {
   };
 }
 
+Future<void> expectFrame(Frame frame,
+    {M.FrameKind kind = M.FrameKind.regular,
+    String? functionName,
+    int? line}) async {
+  expect(frame.kind, equals(kind));
+  if (functionName != null) {
+    expect(frame.function?.name, equals(functionName));
+  }
+  if (line != null) {
+    final script = await frame.location!.script.load() as Script;
+    expect(frame.location, isNotNull);
+    expect(script.tokenToLine(frame.location!.tokenPos), equals(line));
+  }
+}
+
 IsolateTest stoppedAtLine(int line) {
   return (Isolate isolate) async {
     print("Checking we are at line $line");

runtime/vm/stack_trace.cc

@@ -393,7 +393,17 @@ void AsyncAwareStackUnwinder::InitializeAwaiterFrameFromFutureListener(
   }
   awaiter_frame_.closure =
       Closure::RawCast(Get_FutureListener_callback(listener));
-  if (state == k_FutureListener_stateCatchError) {
+
+  // If the Future has catchError callback attached through either
+  // `Future.onError` or `Future.then(..., onError: ...)` then we should
+  // treat this listener as a catch all exception handler. However we should
+  // ignore the case when these callbacks are forwarding errors into a
+  // suspended async function, because it will be represented by its own async
+  // frame.
+  if ((state & k_FutureListener_stateCatchError) != 0 &&
+      (awaiter_frame_.closure.IsNull() ||
+       !StackTraceUtils::GetSuspendState(awaiter_frame_.closure,
+                                         &suspend_state_))) {
     awaiter_frame_.has_catch_error = true;
   }
 }