Add better handling of JSON-RPC exception (#36082)

packages/flutter_tools/lib/src/resident_runner.dart

@@ -844,7 +844,7 @@ abstract class ResidentRunner {
 }
 
 class OperationResult {
-  OperationResult(this.code, this.message);
+  OperationResult(this.code, this.message, { this.fatal = false });
 
   /// The result of the operation; a non-zero code indicates a failure.
   final int code;
@@ -852,6 +852,9 @@ class OperationResult {
   /// A user facing message about the results of the operation.
   final String message;
 
+  /// Whether this error should cause the runner to exit.
+  final bool fatal;
+
   bool get isOk => code == 0;
 
   static final OperationResult ok = OperationResult(0, '');
@@ -906,8 +909,14 @@ class TerminalHandler {
 
   void registerSignalHandlers() {
     assert(residentRunner.stayResident);
-    io.ProcessSignal.SIGINT.watch().listen(_cleanUpAndExit);
-    io.ProcessSignal.SIGTERM.watch().listen(_cleanUpAndExit);
+    io.ProcessSignal.SIGINT.watch().listen((io.ProcessSignal signal) {
+      _cleanUp(signal);
+      io.exit(0);
+    });
+    io.ProcessSignal.SIGTERM.watch().listen((io.ProcessSignal signal) {
+      _cleanUp(signal);
+      io.exit(0);
+    });
     if (!residentRunner.supportsServiceProtocol || !residentRunner.supportsRestart)
       return;
     io.ProcessSignal.SIGUSR1.watch().listen(_handleSignal);
@@ -990,6 +999,9 @@ class TerminalHandler {
           return false;
         }
         final OperationResult result = await residentRunner.restart(fullRestart: false);
+        if (result.fatal) {
+          throwToolExit(result.message);
+        }
         if (!result.isOk) {
           printStatus('Try again after fixing the above error(s).', emphasis: true);
         }
@@ -1000,6 +1012,9 @@ class TerminalHandler {
           return false;
         }
         final OperationResult result = await residentRunner.restart(fullRestart: true);
+        if (result.fatal) {
+          throwToolExit(result.message);
+        }
         if (!result.isOk) {
           printStatus('Try again after fixing the above error(s).', emphasis: true);
         }
@@ -1051,7 +1066,8 @@ class TerminalHandler {
       await _commonTerminalInputHandler(command);
     } catch (error, st) {
       printError('$error\n$st');
-      await _cleanUpAndExit(null);
+      await _cleanUp(null);
+      rethrow;
     } finally {
       _processingUserRequest = false;
     }
@@ -1073,11 +1089,10 @@ class TerminalHandler {
     }
   }
 
-  Future<void> _cleanUpAndExit(io.ProcessSignal signal) async {
+  Future<void> _cleanUp(io.ProcessSignal signal) async {
     terminal.singleCharMode = false;
-    await subscription.cancel();
+    await subscription?.cancel();
     await residentRunner.cleanupAfterSignal();
-    io.exit(0);
   }
 }
 

packages/flutter_tools/lib/src/run_hot.dart

@@ -516,6 +516,9 @@ class HotRunner extends ResidentRunner {
         final OperationResult result = await _restartFromSources(reason: reason, benchmarkMode: benchmarkMode,);
         if (!result.isOk)
           return result;
+      } on rpc.RpcException {
+        await _measureJsonRpcException(flutterDevices, fullRestart);
+        return OperationResult(1, 'hot restart failed to complete', fatal: true);
       } finally {
         status.cancel();
       }
@@ -545,6 +548,9 @@ class HotRunner extends ResidentRunner {
             showTime = false;
           },
         );
+      } on rpc.RpcException {
+        await _measureJsonRpcException(flutterDevices, fullRestart);
+        return OperationResult(1, 'hot reload failed to complete', fatal: true);
       } finally {
         status.cancel();
       }
@@ -946,3 +952,32 @@ class ProjectFileInvalidator {
     return invalidatedFiles;
   }
 }
+
+// This is an error case we would like to know more about.
+Future<void> _measureJsonRpcException(List<FlutterDevice> flutterDevices, bool fullRestart) async {
+    String targetPlatform;
+    String deviceSdk;
+    bool emulator;
+    if (flutterDevices.length == 1) {
+      final Device device = flutterDevices.first.device;
+      targetPlatform = getNameForTargetPlatform(await device.targetPlatform);
+      deviceSdk = await device.sdkNameAndVersion;
+      emulator = await device.isLocalEmulator;
+    } else if (flutterDevices.length > 1) {
+      targetPlatform = 'multiple';
+      deviceSdk = 'multiple';
+      emulator = false;
+    } else {
+      targetPlatform = 'unknown';
+      deviceSdk = 'unknown';
+      emulator = false;
+    }
+    flutterUsage.sendEvent('hot', 'exception',
+      parameters: <String, String>{
+        reloadExceptionTargetPlatform: targetPlatform,
+        reloadExceptionSdkName: deviceSdk,
+        reloadExceptionEmulator: emulator.toString(),
+        reloadExceptionFullRestart: fullRestart.toString(),
+      },
+    );
+}

packages/flutter_tools/lib/src/usage.dart

@@ -49,7 +49,12 @@ const String kCommandBuildBundleTargetPlatform = 'cd24';
 const String kCommandBuildBundleIsModule = 'cd25';
 
 const String kCommandResult = 'cd26';
-// Next ID: cd27
+
+const String reloadExceptionTargetPlatform = 'cd27';
+const String reloadExceptionSdkName = 'cd28';
+const String reloadExceptionEmulator = 'cd29';
+const String reloadExceptionFullRestart = 'cd30';
+// Next ID: cd31
 
 Usage get flutterUsage => Usage.instance;
 

packages/flutter_tools/test/general.shard/resident_runner_test.dart

@@ -4,12 +4,15 @@
 
 import 'dart:async';
 
+import 'package:flutter_tools/src/base/common.dart';
 import 'package:flutter_tools/src/build_info.dart';
 import 'package:flutter_tools/src/devfs.dart';
 import 'package:flutter_tools/src/device.dart';
 import 'package:flutter_tools/src/resident_runner.dart';
 import 'package:flutter_tools/src/run_hot.dart';
+import 'package:flutter_tools/src/usage.dart';
 import 'package:flutter_tools/src/vmservice.dart';
+import 'package:json_rpc_2/json_rpc_2.dart';
 import 'package:mockito/mockito.dart';
 
 import '../src/common.dart';
@@ -19,30 +22,34 @@ void main() {
   group('ResidentRunner', () {
     final Uri testUri = Uri.parse('foo://bar');
     Testbed testbed;
-    MockDevice mockDevice;
+    MockFlutterDevice mockFlutterDevice;
     MockVMService mockVMService;
     MockDevFS mockDevFS;
+    MockFlutterView mockFlutterView;
     ResidentRunner residentRunner;
+    MockDevice mockDevice;
 
     setUp(() {
       testbed = Testbed(setup: () {
         residentRunner = HotRunner(
           <FlutterDevice>[
-            mockDevice,
+            mockFlutterDevice,
           ],
           stayResident: false,
           debuggingOptions: DebuggingOptions.enabled(BuildInfo.debug),
         );
       });
+      mockFlutterDevice = MockFlutterDevice();
       mockDevice = MockDevice();
       mockVMService = MockVMService();
       mockDevFS = MockDevFS();
+      mockFlutterView = MockFlutterView();
       // DevFS Mocks
       when(mockDevFS.lastCompiled).thenReturn(DateTime(2000));
       when(mockDevFS.sources).thenReturn(<Uri>[]);
       when(mockDevFS.destroy()).thenAnswer((Invocation invocation) async { });
       // FlutterDevice Mocks.
-      when(mockDevice.updateDevFS(
+      when(mockFlutterDevice.updateDevFS(
         // Intentionally provide empty list to match above mock.
         invalidatedFiles: <Uri>[],
         mainPath: anyNamed('mainPath'),
@@ -61,27 +68,29 @@ void main() {
           invalidatedSourcesCount: 0,
         );
       });
-      when(mockDevice.devFS).thenReturn(mockDevFS);
-      when(mockDevice.views).thenReturn(<FlutterView>[
-        MockFlutterView(),
+      when(mockFlutterDevice.devFS).thenReturn(mockDevFS);
+      when(mockFlutterDevice.views).thenReturn(<FlutterView>[
+        mockFlutterView
       ]);
-      when(mockDevice.stopEchoingDeviceLog()).thenAnswer((Invocation invocation) async { });
-      when(mockDevice.observatoryUris).thenReturn(<Uri>[
+      when(mockFlutterDevice.device).thenReturn(mockDevice);
+      when(mockFlutterView.uiIsolate).thenReturn(MockIsolate());
+      when(mockFlutterDevice.stopEchoingDeviceLog()).thenAnswer((Invocation invocation) async { });
+      when(mockFlutterDevice.observatoryUris).thenReturn(<Uri>[
         testUri,
       ]);
-      when(mockDevice.connect(
+      when(mockFlutterDevice.connect(
         reloadSources: anyNamed('reloadSources'),
         restart: anyNamed('restart'),
         compileExpression: anyNamed('compileExpression')
       )).thenAnswer((Invocation invocation) async { });
-      when(mockDevice.setupDevFS(any, any, packagesFilePath: anyNamed('packagesFilePath')))
+      when(mockFlutterDevice.setupDevFS(any, any, packagesFilePath: anyNamed('packagesFilePath')))
         .thenAnswer((Invocation invocation) async {
           return testUri;
         });
-      when(mockDevice.vmServices).thenReturn(<VMService>[
+      when(mockFlutterDevice.vmServices).thenReturn(<VMService>[
         mockVMService,
       ]);
-      when(mockDevice.refreshViews()).thenAnswer((Invocation invocation) async { });
+      when(mockFlutterDevice.refreshViews()).thenAnswer((Invocation invocation) async { });
       // VMService mocks.
       when(mockVMService.wsAddress).thenReturn(testUri);
       when(mockVMService.done).thenAnswer((Invocation invocation) {
@@ -101,16 +110,106 @@ void main() {
 
       expect(await result, 0);
 
-      verify(mockDevice.initLogReader()).called(1);
+      verify(mockFlutterDevice.initLogReader()).called(1);
 
       expect(onConnectionInfo.isCompleted, true);
       expect((await connectionInfo).baseUri, 'foo://bar');
       expect(onAppStart.isCompleted, true);
     }));
+
+    test('Can handle an RPC exception from hot reload', () => testbed.run(() async {
+      when(mockDevice.sdkNameAndVersion).thenAnswer((Invocation invocation) async {
+        return 'Example';
+      });
+      when(mockDevice.targetPlatform).thenAnswer((Invocation invocation) async {
+        return TargetPlatform.android_arm;
+      });
+      when(mockDevice.isLocalEmulator).thenAnswer((Invocation invocation) async {
+        return false;
+      });
+      final Completer<DebugConnectionInfo> onConnectionInfo = Completer<DebugConnectionInfo>.sync();
+      final Completer<void> onAppStart = Completer<void>.sync();
+      unawaited(residentRunner.attach(
+        appStartedCompleter: onAppStart,
+        connectionInfoCompleter: onConnectionInfo,
+      ));
+      await onAppStart.future;
+      when(mockFlutterDevice.updateDevFS(
+        mainPath: anyNamed('mainPath'),
+        target: anyNamed('target'),
+        bundle: anyNamed('bundle'),
+        firstBuildTime: anyNamed('firstBuildTime'),
+        bundleFirstUpload: anyNamed('bundleFirstUpload'),
+        bundleDirty: anyNamed('bundleDirty'),
+        fullRestart: anyNamed('fullRestart'),
+        projectRootPath: anyNamed('projectRootPath'),
+        pathToReload: anyNamed('pathToReload'),
+        invalidatedFiles: anyNamed('invalidatedFiles'),
+      )).thenThrow(RpcException(666, 'something bad happened'));
+
+      final OperationResult result = await residentRunner.restart(fullRestart: false);
+      expect(result.fatal, true);
+      expect(result.code, 1);
+      verify(flutterUsage.sendEvent('hot', 'exception', parameters: <String, String>{
+        reloadExceptionTargetPlatform: getNameForTargetPlatform(TargetPlatform.android_arm),
+        reloadExceptionSdkName: 'Example',
+        reloadExceptionEmulator: 'false',
+        reloadExceptionFullRestart: 'false',
+      })).called(1);
+    }, overrides: <Type, Generator>{
+      Usage: () => MockUsage(),
+    }));
+
+    test('Can handle an RPC exception from hot restart', () => testbed.run(() async {
+      when(mockDevice.sdkNameAndVersion).thenAnswer((Invocation invocation) async {
+        return 'Example';
+      });
+      when(mockDevice.targetPlatform).thenAnswer((Invocation invocation) async {
+        return TargetPlatform.android_arm;
+      });
+      when(mockDevice.isLocalEmulator).thenAnswer((Invocation invocation) async {
+        return false;
+      });
+      when(mockDevice.supportsHotRestart).thenReturn(true);
+      final Completer<DebugConnectionInfo> onConnectionInfo = Completer<DebugConnectionInfo>.sync();
+      final Completer<void> onAppStart = Completer<void>.sync();
+      unawaited(residentRunner.attach(
+        appStartedCompleter: onAppStart,
+        connectionInfoCompleter: onConnectionInfo,
+      ));
+      await onAppStart.future;
+      when(mockFlutterDevice.updateDevFS(
+        mainPath: anyNamed('mainPath'),
+        target: anyNamed('target'),
+        bundle: anyNamed('bundle'),
+        firstBuildTime: anyNamed('firstBuildTime'),
+        bundleFirstUpload: anyNamed('bundleFirstUpload'),
+        bundleDirty: anyNamed('bundleDirty'),
+        fullRestart: anyNamed('fullRestart'),
+        projectRootPath: anyNamed('projectRootPath'),
+        pathToReload: anyNamed('pathToReload'),
+        invalidatedFiles: anyNamed('invalidatedFiles'),
+      )).thenThrow(RpcException(666, 'something bad happened'));
+
+      final OperationResult result = await residentRunner.restart(fullRestart: true);
+      expect(result.fatal, true);
+      expect(result.code, 1);
+      verify(flutterUsage.sendEvent('hot', 'exception', parameters: <String, String>{
+        reloadExceptionTargetPlatform: getNameForTargetPlatform(TargetPlatform.android_arm),
+        reloadExceptionSdkName: 'Example',
+        reloadExceptionEmulator: 'false',
+        reloadExceptionFullRestart: 'true',
+      })).called(1);
+    }, overrides: <Type, Generator>{
+      Usage: () => MockUsage(),
+    }));
   });
 }
 
-class MockDevice extends Mock implements FlutterDevice {}
+class MockFlutterDevice extends Mock implements FlutterDevice {}
 class MockFlutterView extends Mock implements FlutterView {}
 class MockVMService extends Mock implements VMService {}
 class MockDevFS extends Mock implements DevFS {}
+class MockIsolate extends Mock implements Isolate {}
+class MockDevice extends Mock implements Device {}
+class MockUsage extends Mock implements Usage {}

packages/flutter_tools/test/general.shard/terminal_handler_test.dart

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 import 'dart:async';
+import 'package:flutter_tools/src/base/common.dart';
 import 'package:flutter_tools/src/base/logger.dart';
 import 'package:flutter_tools/src/build_info.dart';
 import 'package:flutter_tools/src/device.dart';
@@ -246,6 +247,16 @@ void main() {
       expect(bufferLogger.statusText, contains('Try again after fixing the above error(s).'));
     });
 
+    testUsingContext('r - hotReload supported and fails fatally', () async {
+      when(mockResidentRunner.canHotReload).thenReturn(true);
+      when(mockResidentRunner.hotMode).thenReturn(true);
+      when(mockResidentRunner.restart(fullRestart: false))
+        .thenAnswer((Invocation invocation) async {
+          return OperationResult(1, 'fail', fatal: true);
+        });
+      expect(terminalHandler.processTerminalInput('r'), throwsA(isInstanceOf<ToolExit>()));
+    });
+
     testUsingContext('r - hotReload unsupported', () async {
       when(mockResidentRunner.canHotReload).thenReturn(false);
       await terminalHandler.processTerminalInput('r');
@@ -281,6 +292,16 @@ void main() {
       expect(bufferLogger.statusText, contains('Try again after fixing the above error(s).'));
     });
 
+    testUsingContext('R - hotRestart supported and fails fatally', () async {
+      when(mockResidentRunner.canHotRestart).thenReturn(true);
+      when(mockResidentRunner.hotMode).thenReturn(true);
+      when(mockResidentRunner.restart(fullRestart: true))
+        .thenAnswer((Invocation invocation) async {
+          return OperationResult(1, 'fail', fatal: true);
+        });
+      expect(() => terminalHandler.processTerminalInput('R'), throwsA(isInstanceOf<ToolExit>()));
+    });
+
     testUsingContext('R - hot restart unsupported', () async {
       when(mockResidentRunner.canHotRestart).thenReturn(false);
       await terminalHandler.processTerminalInput('R');