Fix Flutter crash by catching WipError on resume and mapping to RPC e…

…rror (#2188)
dart-lang · Aug 10, 2023 · 1f1efa3 · 1f1efa3
1 parent f8f752c
commit 1f1efa3
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 54 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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 {