diff --git a/packages/flutter_tools/lib/src/resident_runner.dart b/packages/flutter_tools/lib/src/resident_runner.dart index 77c9e94e52b..fc6b2d4a941 100644 --- a/packages/flutter_tools/lib/src/resident_runner.dart +++ b/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 _cleanUpAndExit(io.ProcessSignal signal) async { + Future _cleanUp(io.ProcessSignal signal) async { terminal.singleCharMode = false; - await subscription.cancel(); + await subscription?.cancel(); await residentRunner.cleanupAfterSignal(); - io.exit(0); } } diff --git a/packages/flutter_tools/lib/src/run_hot.dart b/packages/flutter_tools/lib/src/run_hot.dart index c19057ecf9a..36d1f616de6 100644 --- a/packages/flutter_tools/lib/src/run_hot.dart +++ b/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 _measureJsonRpcException(List 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: { + reloadExceptionTargetPlatform: targetPlatform, + reloadExceptionSdkName: deviceSdk, + reloadExceptionEmulator: emulator.toString(), + reloadExceptionFullRestart: fullRestart.toString(), + }, + ); +} diff --git a/packages/flutter_tools/lib/src/usage.dart b/packages/flutter_tools/lib/src/usage.dart index 63d8d7f7eb5..6ad484b032a 100644 --- a/packages/flutter_tools/lib/src/usage.dart +++ b/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; diff --git a/packages/flutter_tools/test/general.shard/resident_runner_test.dart b/packages/flutter_tools/test/general.shard/resident_runner_test.dart index 26b81128bb8..48a07a5eb74 100644 --- a/packages/flutter_tools/test/general.shard/resident_runner_test.dart +++ b/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( [ - 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([]); when(mockDevFS.destroy()).thenAnswer((Invocation invocation) async { }); // FlutterDevice Mocks. - when(mockDevice.updateDevFS( + when(mockFlutterDevice.updateDevFS( // Intentionally provide empty list to match above mock. invalidatedFiles: [], mainPath: anyNamed('mainPath'), @@ -61,27 +68,29 @@ void main() { invalidatedSourcesCount: 0, ); }); - when(mockDevice.devFS).thenReturn(mockDevFS); - when(mockDevice.views).thenReturn([ - MockFlutterView(), + when(mockFlutterDevice.devFS).thenReturn(mockDevFS); + when(mockFlutterDevice.views).thenReturn([ + mockFlutterView ]); - when(mockDevice.stopEchoingDeviceLog()).thenAnswer((Invocation invocation) async { }); - when(mockDevice.observatoryUris).thenReturn([ + when(mockFlutterDevice.device).thenReturn(mockDevice); + when(mockFlutterView.uiIsolate).thenReturn(MockIsolate()); + when(mockFlutterDevice.stopEchoingDeviceLog()).thenAnswer((Invocation invocation) async { }); + when(mockFlutterDevice.observatoryUris).thenReturn([ 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([ + when(mockFlutterDevice.vmServices).thenReturn([ 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 onConnectionInfo = Completer.sync(); + final Completer onAppStart = Completer.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: { + reloadExceptionTargetPlatform: getNameForTargetPlatform(TargetPlatform.android_arm), + reloadExceptionSdkName: 'Example', + reloadExceptionEmulator: 'false', + reloadExceptionFullRestart: 'false', + })).called(1); + }, overrides: { + 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 onConnectionInfo = Completer.sync(); + final Completer onAppStart = Completer.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: { + reloadExceptionTargetPlatform: getNameForTargetPlatform(TargetPlatform.android_arm), + reloadExceptionSdkName: 'Example', + reloadExceptionEmulator: 'false', + reloadExceptionFullRestart: 'true', + })).called(1); + }, overrides: { + 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 {} diff --git a/packages/flutter_tools/test/general.shard/terminal_handler_test.dart b/packages/flutter_tools/test/general.shard/terminal_handler_test.dart index 7f4a2f4a662..d4077f498f6 100644 --- a/packages/flutter_tools/test/general.shard/terminal_handler_test.dart +++ b/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())); + }); + 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())); + }); + testUsingContext('R - hot restart unsupported', () async { when(mockResidentRunner.canHotRestart).thenReturn(false); await terminalHandler.processTerminalInput('R');