From 1f1efa35fa2b02b3b7825d3d994584cbf8eceed0 Mon Sep 17 00:00:00 2001
From: Elliott Brooks <21270878+elliette@users.noreply.github.com>
Date: Thu, 10 Aug 2023 09:59:29 -0700
Subject: [PATCH] Fix Flutter crash by catching WipError on resume and mapping
 to RPC error (#2188)

---
 dwds/CHANGELOG.md                             |  1 +
 dwds/lib/src/debugging/debugger.dart          | 61 +++++++++++--------
 .../src/services/chrome_proxy_service.dart    | 41 +++++--------
 dwds/test/chrome_proxy_service_test.dart      | 11 +++-
 dwds/test/fixtures/context.dart               |  4 ++
 5 files changed, 64 insertions(+), 54 deletions(-)

diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md
index 338e0a89e..be666deaf 100644
--- a/dwds/CHANGELOG.md
+++ b/dwds/CHANGELOG.md
@@ -4,6 +4,7 @@
 - Update SDK constraint to `>=3.1.0-254.0.dev <4.0.0`. - [#2169](https://github.com/dart-lang/webdev/pull/2169)
 - Require min `build_web_compilers` version `4.0.4` - [#2171](https://github.com/dart-lang/webdev/pull/2171)
 - Switch to using new debugging API from DDC to support new type system. - [#2159](https://github.com/dart-lang/webdev/pull/2159)
+- Fix Flutter crash when calling `resume` when app is not paused. - [#2188](https://github.com/dart-lang/webdev/pull/2188)
 
 ## 19.0.2
 
diff --git a/dwds/lib/src/debugging/debugger.dart b/dwds/lib/src/debugging/debugger.dart
index 27e4c1b54..e506205ba 100644
--- a/dwds/lib/src/debugging/debugger.dart
+++ b/dwds/lib/src/debugging/debugger.dart
@@ -121,32 +121,45 @@ class Debugger extends Domain {
   /// Note that stepping will automatically continue until Chrome is paused at
   /// a location for which we have source information.
   Future<Success> resume({String? step, int? frameIndex}) async {
-    if (frameIndex != null) {
-      throw ArgumentError('FrameIndex is currently unsupported.');
-    }
-    WipResponse? result;
-    if (step != null) {
-      _isStepping = true;
-      switch (step) {
-        case 'Over':
-          result = await _remoteDebugger.stepOver();
-          break;
-        case 'Out':
-          result = await _remoteDebugger.stepOut();
-          break;
-        case 'Into':
-          result = await _remoteDebugger.stepInto();
-          break;
-        default:
-          throwInvalidParam('resume', 'Unexpected value for step: $step');
+    try {
+      if (frameIndex != null) {
+        throw ArgumentError('FrameIndex is currently unsupported.');
       }
-    } else {
-      _isStepping = false;
-      _previousSteppingLocation = null;
-      result = await _remoteDebugger.resume();
+      WipResponse? result;
+      if (step != null) {
+        _isStepping = true;
+        switch (step) {
+          case 'Over':
+            result = await _remoteDebugger.stepOver();
+            break;
+          case 'Out':
+            result = await _remoteDebugger.stepOut();
+            break;
+          case 'Into':
+            result = await _remoteDebugger.stepInto();
+            break;
+          default:
+            throwInvalidParam('resume', 'Unexpected value for step: $step');
+        }
+      } else {
+        _isStepping = false;
+        _previousSteppingLocation = null;
+        result = await _remoteDebugger.resume();
+      }
+      handleErrorIfPresent(result);
+      return Success();
+    } on WipError catch (e) {
+      final errorMessage = e.message;
+      if (errorMessage != null &&
+          errorMessage.contains('Can only perform operation while paused')) {
+        throw RPCError(
+          'resume',
+          RPCErrorKind.kIsolateMustBePaused.code,
+          errorMessage,
+        );
+      }
+      rethrow;
     }
-    handleErrorIfPresent(result);
-    return Success();
   }
 
   /// Returns the current Dart stack for the paused debugger.
diff --git a/dwds/lib/src/services/chrome_proxy_service.dart b/dwds/lib/src/services/chrome_proxy_service.dart
index 9a239838b..968d15d85 100644
--- a/dwds/lib/src/services/chrome_proxy_service.dart
+++ b/dwds/lib/src/services/chrome_proxy_service.dart
@@ -1088,33 +1088,20 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension(
     String? step,
     int? frameIndex,
   }) async {
-    try {
-      if (inspector.appConnection.isStarted) {
-        return captureElapsedTime(
-          () async {
-            await isInitialized;
-            await isStarted;
-            _checkIsolate('resume', isolateId);
-            return await (await debuggerFuture)
-                .resume(step: step, frameIndex: frameIndex);
-          },
-          (result) => DwdsEvent.resume(step),
-        );
-      } else {
-        inspector.appConnection.runMain();
-        return Success();
-      }
-    } on WipError catch (e) {
-      final errorMessage = e.message;
-      if (errorMessage != null &&
-          errorMessage.contains('Can only perform operation while paused')) {
-        throw RPCError(
-          'resume',
-          RPCErrorKind.kIsolateMustBePaused.code,
-          errorMessage,
-        );
-      }
-      rethrow;
+    if (inspector.appConnection.isStarted) {
+      return captureElapsedTime(
+        () async {
+          await isInitialized;
+          await isStarted;
+          _checkIsolate('resume', isolateId);
+          return await (await debuggerFuture)
+              .resume(step: step, frameIndex: frameIndex);
+        },
+        (result) => DwdsEvent.resume(step),
+      );
+    } else {
+      inspector.appConnection.runMain();
+      return Success();
     }
   }
 
diff --git a/dwds/test/chrome_proxy_service_test.dart b/dwds/test/chrome_proxy_service_test.dart
index 8fa93fa1c..ae4ae41c6 100644
--- a/dwds/test/chrome_proxy_service_test.dart
+++ b/dwds/test/chrome_proxy_service_test.dart
@@ -1339,12 +1339,17 @@ void main() {
         expect(pauseBreakpoints, hasLength(1));
         expect(pauseBreakpoints.first.id, bp.id);
         await service.removeBreakpoint(isolateId!, bp.id!);
-      });
-
-      tearDown(() async {
         // Resume execution to not impact other tests.
         await service.resume(isolateId!);
       });
+
+      test('resuming throws kIsolateMustBePaused error if not paused',
+          () async {
+        await expectLater(
+          service.resume(isolateId!),
+          throwsRPCErrorWithCode(RPCErrorKind.kIsolateMustBePaused.code),
+        );
+      });
     });
 
     group('Step', () {
diff --git a/dwds/test/fixtures/context.dart b/dwds/test/fixtures/context.dart
index f2f27a6dc..c926ca082 100644
--- a/dwds/test/fixtures/context.dart
+++ b/dwds/test/fixtures/context.dart
@@ -55,6 +55,10 @@ Matcher isRPCErrorWithMessage(String message) =>
 Matcher throwsRPCErrorWithMessage(String message) =>
     throwsA(isRPCErrorWithMessage(message));
 
+Matcher isRPCErrorWithCode(int code) =>
+    isA<RPCError>().having((e) => e.code, 'code', equals(code));
+Matcher throwsRPCErrorWithCode(int code) => throwsA(isRPCErrorWithCode(code));
+
 enum CompilationMode { buildDaemon, frontendServer }
 
 class TestContext {