Add better handling of JSON-RPC exception (#36082)
diff --git a/packages/flutter_tools/lib/src/resident_runner.dart b/packages/flutter_tools/lib/src/resident_runner.dart
index 77c9e94..fc6b2d4 100644
--- a/packages/flutter_tools/lib/src/resident_runner.dart
+++ b/packages/flutter_tools/lib/src/resident_runner.dart
@@ -844,7 +844,7 @@
}
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 @@
/// 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 @@
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 @@
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 @@
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 @@
await _commonTerminalInputHandler(command);
} catch (error, st) {
printError('$error\n$st');
- await _cleanUpAndExit(null);
+ await _cleanUp(null);
+ rethrow;
} finally {
_processingUserRequest = false;
}
@@ -1073,11 +1089,10 @@
}
}
- 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);
}
}
diff --git a/packages/flutter_tools/lib/src/run_hot.dart b/packages/flutter_tools/lib/src/run_hot.dart
index c19057e..36d1f61 100644
--- a/packages/flutter_tools/lib/src/run_hot.dart
+++ b/packages/flutter_tools/lib/src/run_hot.dart
@@ -516,6 +516,9 @@
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 @@
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 @@
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(),
+ },
+ );
+}
diff --git a/packages/flutter_tools/lib/src/usage.dart b/packages/flutter_tools/lib/src/usage.dart
index 63d8d7f..6ad484b 100644
--- a/packages/flutter_tools/lib/src/usage.dart
+++ b/packages/flutter_tools/lib/src/usage.dart
@@ -49,7 +49,12 @@
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 26b8112..48a07a5 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 @@
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 @@
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 @@
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 {}
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 7f4a2f4..d4077f4 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 @@
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 @@
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');
