diff --git a/packages/flutter_tools/lib/src/commands/screenshot.dart b/packages/flutter_tools/lib/src/commands/screenshot.dart index d6370fd3555..937dfe25a69 100644 --- a/packages/flutter_tools/lib/src/commands/screenshot.dart +++ b/packages/flutter_tools/lib/src/commands/screenshot.dart @@ -97,19 +97,21 @@ class ScreenshotCommand extends FlutterCommand { outputFile = globals.fs.file(stringArg(_kOut)); } + bool success = true; switch (stringArg(_kType)) { case _kDeviceType: await runScreenshot(outputFile); - return FlutterCommandResult.success(); + break; case _kSkiaType: - await runSkia(outputFile); - return FlutterCommandResult.success(); + success = await runSkia(outputFile); + break; case _kRasterizerType: - await runRasterizer(outputFile); - return FlutterCommandResult.success(); + success = await runRasterizer(outputFile); + break; } - return FlutterCommandResult.success(); + return success ? FlutterCommandResult.success() + : FlutterCommandResult.fail(); } Future runScreenshot(File outputFile) async { @@ -126,10 +128,17 @@ class ScreenshotCommand extends FlutterCommand { _showOutputFileInfo(outputFile); } - Future runSkia(File outputFile) async { + Future runSkia(File outputFile) async { final Uri observatoryUri = Uri.parse(stringArg(_kObservatoryUri)); final vm_service.VmService vmService = await connectToVmService(observatoryUri); final vm_service.Response skp = await vmService.screenshotSkp(); + if (skp == null) { + globals.printError( + 'The Skia picture request failed, probably because the device was ' + 'disconnected', + ); + return false; + } outputFile ??= globals.fsUtils.getUniqueFile( globals.fs.currentDirectory, 'flutter', @@ -140,12 +149,20 @@ class ScreenshotCommand extends FlutterCommand { await sink.close(); _showOutputFileInfo(outputFile); _ensureOutputIsNotJsonRpcError(outputFile); + return true; } - Future runRasterizer(File outputFile) async { + Future runRasterizer(File outputFile) async { final Uri observatoryUri = Uri.parse(stringArg(_kObservatoryUri)); final vm_service.VmService vmService = await connectToVmService(observatoryUri); final vm_service.Response response = await vmService.screenshot(); + if (response == null) { + globals.printError( + 'The screenshot request failed, probably because the device was ' + 'disconnected', + ); + return false; + } outputFile ??= globals.fsUtils.getUniqueFile( globals.fs.currentDirectory, 'flutter', @@ -156,6 +173,7 @@ class ScreenshotCommand extends FlutterCommand { await sink.close(); _showOutputFileInfo(outputFile); _ensureOutputIsNotJsonRpcError(outputFile); + return true; } void _ensureOutputIsNotJsonRpcError(File outputFile) { diff --git a/packages/flutter_tools/lib/src/devfs.dart b/packages/flutter_tools/lib/src/devfs.dart index 9a64b2eb453..b463dce3174 100644 --- a/packages/flutter_tools/lib/src/devfs.dart +++ b/packages/flutter_tools/lib/src/devfs.dart @@ -419,8 +419,15 @@ class DevFS { final vm_service.Response response = await _vmService.createDevFS(fsName); _baseUri = Uri.parse(response.json['uri'] as String); } on vm_service.RPCError catch (rpcException) { + if (rpcException.code == RPCErrorCodes.kServiceDisappeared) { + // This can happen if the device has been disconnected, so translate to + // a DevFSException, which the caller will handle. + throw DevFSException('Service disconnected', rpcException); + } // 1001 is kFileSystemAlreadyExists in //dart/runtime/vm/json_stream.h if (rpcException.code != 1001) { + // Other RPCErrors are unexpected. Rethrow so it will hit crash + // logging. rethrow; } _logger.printTrace('DevFS: Creating failed. Destroying and trying again'); diff --git a/packages/flutter_tools/lib/src/tracing.dart b/packages/flutter_tools/lib/src/tracing.dart index 7c55d3584b9..9c6a64ae1ae 100644 --- a/packages/flutter_tools/lib/src/tracing.dart +++ b/packages/flutter_tools/lib/src/tracing.dart @@ -32,7 +32,7 @@ class Tracing { final Logger _logger; Future startTracing() async { - await vmService.setVMTimelineFlags(['Compiler', 'Dart', 'Embedder', 'GC']); + await vmService.setTimelineFlags(['Compiler', 'Dart', 'Embedder', 'GC']); await vmService.clearVMTimeline(); } @@ -78,8 +78,13 @@ class Tracing { } status.stop(); } - final vm_service.Timeline timeline = await vmService.getVMTimeline(); - await vmService.setVMTimelineFlags([]); + final vm_service.Response timeline = await vmService.getTimeline(); + await vmService.setTimelineFlags([]); + if (timeline == null) { + throwToolExit( + 'The device disconnected before the timeline could be retrieved.', + ); + } return timeline.json; } } diff --git a/packages/flutter_tools/lib/src/vmservice.dart b/packages/flutter_tools/lib/src/vmservice.dart index c0aee7ee40f..9e45e553d20 100644 --- a/packages/flutter_tools/lib/src/vmservice.dart +++ b/packages/flutter_tools/lib/src/vmservice.dart @@ -728,23 +728,12 @@ extension FlutterVmService on vm_service.VmService { return null; } - /// Invoke a flutter extension method, if the flutter extension is not - /// available, returns null. - Future> invokeFlutterExtensionRpcRaw( + Future _checkedCallServiceExtension( String method, { - @required String isolateId, Map args, }) async { try { - - final vm_service.Response response = await callServiceExtension( - method, - args: { - 'isolateId': isolateId, - ...?args, - }, - ); - return response.json; + return await callServiceExtension(method, args: args); } on vm_service.RPCError catch (err) { // If an application is not using the framework or the VM service // disappears while handling a request, return null. @@ -756,6 +745,23 @@ extension FlutterVmService on vm_service.VmService { } } + /// Invoke a flutter extension method, if the flutter extension is not + /// available, returns null. + Future> invokeFlutterExtensionRpcRaw( + String method, { + @required String isolateId, + Map args, + }) async { + final vm_service.Response response = await _checkedCallServiceExtension( + method, + args: { + 'isolateId': isolateId, + ...?args, + }, + ); + return response?.json; + } + /// List all [FlutterView]s attached to the current VM. /// /// If this returns an empty list, it will poll forever unless [returnEarly] @@ -799,26 +805,34 @@ extension FlutterVmService on vm_service.VmService { /// Create a new development file system on the device. Future createDevFS(String fsName) { - return callServiceExtension('_createDevFS', args: {'fsName': fsName}); + // Call the unchecked version of `callServiceExtension` because the caller + // has custom handling of certain RPCErrors. + return callServiceExtension( + '_createDevFS', + args: {'fsName': fsName}, + ); } /// Delete an existing file system. - Future deleteDevFS(String fsName) { - return callServiceExtension('_deleteDevFS', args: {'fsName': fsName}); + Future deleteDevFS(String fsName) async { + await _checkedCallServiceExtension( + '_deleteDevFS', + args: {'fsName': fsName}, + ); } Future screenshot() { - return callServiceExtension(kScreenshotMethod); + return _checkedCallServiceExtension(kScreenshotMethod); } Future screenshotSkp() { - return callServiceExtension(kScreenshotSkpMethod); + return _checkedCallServiceExtension(kScreenshotSkpMethod); } /// Set the VM timeline flags. - Future setVMTimelineFlags(List recordedStreams) { + Future setTimelineFlags(List recordedStreams) async { assert(recordedStreams != null); - return callServiceExtension( + await _checkedCallServiceExtension( 'setVMTimelineFlags', args: { 'recordedStreams': recordedStreams, @@ -826,8 +840,8 @@ extension FlutterVmService on vm_service.VmService { ); } - Future getVMTimeline() { - return callServiceExtension('getVMTimeline'); + Future getTimeline() { + return _checkedCallServiceExtension('getVMTimeline'); } } diff --git a/packages/flutter_tools/test/general.shard/devfs_test.dart b/packages/flutter_tools/test/general.shard/devfs_test.dart index 5a7667d241f..0fd20b04a5f 100644 --- a/packages/flutter_tools/test/general.shard/devfs_test.dart +++ b/packages/flutter_tools/test/general.shard/devfs_test.dart @@ -31,6 +31,20 @@ final FakeVmServiceRequest createDevFSRequest = FakeVmServiceRequest( } ); +const FakeVmServiceRequest failingCreateDevFSRequest = FakeVmServiceRequest( + method: '_createDevFS', + args: { + 'fsName': 'test', + }, + errorCode: RPCErrorCodes.kServiceDisappeared, +); + +const FakeVmServiceRequest failingDeleteDevFSRequest = FakeVmServiceRequest( + method: '_deleteDevFS', + args: {'fsName': 'test'}, + errorCode: RPCErrorCodes.kServiceDisappeared, +); + void main() { testWithoutContext('DevFSByteContent', () { final DevFSByteContent content = DevFSByteContent([4, 5, 6]); @@ -93,6 +107,73 @@ void main() { expect(content.isModified, isFalse); }); + testWithoutContext('DevFS create throws a DevFSException when vmservice disconnects unexpectedly', () async { + final HttpClient httpClient = MockHttpClient(); + final FileSystem fileSystem = MemoryFileSystem.test(); + final OperatingSystemUtils osUtils = MockOperatingSystemUtils(); + final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost( + requests: [failingCreateDevFSRequest], + ); + setHttpAddress(Uri.parse('http://localhost'), fakeVmServiceHost.vmService); + + final MockHttpClientRequest httpRequest = MockHttpClientRequest(); + when(httpRequest.headers).thenReturn(MockHttpHeaders()); + when(httpClient.putUrl(any)).thenAnswer((Invocation invocation) { + return Future.value(httpRequest); + }); + final MockHttpClientResponse httpClientResponse = MockHttpClientResponse(); + when(httpRequest.close()).thenAnswer((Invocation invocation) { + return Future.value(httpClientResponse); + }); + + final DevFS devFS = DevFS( + fakeVmServiceHost.vmService, + 'test', + fileSystem.currentDirectory, + osUtils: osUtils, + fileSystem: fileSystem, + logger: BufferLogger.test(), + httpClient: httpClient, + ); + expect(() async => await devFS.create(), throwsA(isA())); + }); + + testWithoutContext('DevFS destroy is resiliant to vmservice disconnection', () async { + final HttpClient httpClient = MockHttpClient(); + final FileSystem fileSystem = MemoryFileSystem.test(); + final OperatingSystemUtils osUtils = MockOperatingSystemUtils(); + final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost( + requests: [ + createDevFSRequest, + failingDeleteDevFSRequest, + ], + ); + setHttpAddress(Uri.parse('http://localhost'), fakeVmServiceHost.vmService); + + final MockHttpClientRequest httpRequest = MockHttpClientRequest(); + when(httpRequest.headers).thenReturn(MockHttpHeaders()); + when(httpClient.putUrl(any)).thenAnswer((Invocation invocation) { + return Future.value(httpRequest); + }); + final MockHttpClientResponse httpClientResponse = MockHttpClientResponse(); + when(httpRequest.close()).thenAnswer((Invocation invocation) { + return Future.value(httpClientResponse); + }); + + final DevFS devFS = DevFS( + fakeVmServiceHost.vmService, + 'test', + fileSystem.currentDirectory, + osUtils: osUtils, + fileSystem: fileSystem, + logger: BufferLogger.test(), + httpClient: httpClient, + ); + + expect(await devFS.create(), isNotNull); + await devFS.destroy(); // Testing that this does not throw. + }); + testWithoutContext('DevFS retries uploads when connection reset by peer', () async { final HttpClient httpClient = MockHttpClient(); final FileSystem fileSystem = MemoryFileSystem.test(); diff --git a/packages/flutter_tools/test/general.shard/tracing_test.dart b/packages/flutter_tools/test/general.shard/tracing_test.dart index 95004317467..d17cc85bf2c 100644 --- a/packages/flutter_tools/test/general.shard/tracing_test.dart +++ b/packages/flutter_tools/test/general.shard/tracing_test.dart @@ -130,6 +130,29 @@ void main() { }); }); + testWithoutContext('throws tool exit if the vmservice disconnects', () async { + final BufferLogger logger = BufferLogger.test(); + final FileSystem fileSystem = MemoryFileSystem.test(); + final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost(requests: [ + ...vmServiceSetup, + const FakeVmServiceRequest( + method: 'getVMTimeline', + errorCode: RPCErrorCodes.kServiceDisappeared, + ), + const FakeVmServiceRequest( + method: 'setVMTimelineFlags', + args: { + 'recordedStreams': [], + }, + ), + ]); + + await expectLater(() async => await downloadStartupTrace(fakeVmServiceHost.vmService, + output: fileSystem.currentDirectory, + logger: logger, + ), throwsToolExit(message: 'The device disconnected before the timeline could be retrieved.')); + }); + testWithoutContext('throws tool exit if timeline is missing the engine start event', () async { final BufferLogger logger = BufferLogger.test(); final FileSystem fileSystem = MemoryFileSystem.test(); diff --git a/packages/flutter_tools/test/general.shard/vmservice_test.dart b/packages/flutter_tools/test/general.shard/vmservice_test.dart index 8aef59ed7a4..2edce097401 100644 --- a/packages/flutter_tools/test/general.shard/vmservice_test.dart +++ b/packages/flutter_tools/test/general.shard/vmservice_test.dart @@ -315,20 +315,58 @@ void main() { testWithoutContext('Framework service extension invocations return null if service disappears ', () async { final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost( requests: [ - const FakeVmServiceRequest(method: kGetSkSLsMethod, args: { - 'viewId': '1234', - }, errorCode: RPCErrorCodes.kServiceDisappeared), - const FakeVmServiceRequest(method: kListViewsMethod, errorCode: RPCErrorCodes.kServiceDisappeared), + const FakeVmServiceRequest( + method: kGetSkSLsMethod, + args: { + 'viewId': '1234', + }, + errorCode: RPCErrorCodes.kServiceDisappeared, + ), + const FakeVmServiceRequest( + method: kListViewsMethod, + errorCode: RPCErrorCodes.kServiceDisappeared, + ), + const FakeVmServiceRequest( + method: kScreenshotMethod, + errorCode: RPCErrorCodes.kServiceDisappeared, + ), + const FakeVmServiceRequest( + method: kScreenshotSkpMethod, + errorCode: RPCErrorCodes.kServiceDisappeared, + ), + const FakeVmServiceRequest( + method: 'setVMTimelineFlags', + args: { + 'recordedStreams': ['test'], + }, + errorCode: RPCErrorCodes.kServiceDisappeared, + ), + const FakeVmServiceRequest( + method: 'getVMTimeline', + errorCode: RPCErrorCodes.kServiceDisappeared, + ), ] ); final Map skSLs = await fakeVmServiceHost.vmService.getSkSLs( viewId: '1234', ); - expect(skSLs, null); + expect(skSLs, isNull); final List views = await fakeVmServiceHost.vmService.getFlutterViews(); - expect(views, null); + expect(views, isNull); + + final vm_service.Response screenshot = await fakeVmServiceHost.vmService.screenshot(); + expect(screenshot, isNull); + + final vm_service.Response screenshotSkp = await fakeVmServiceHost.vmService.screenshotSkp(); + expect(screenshotSkp, isNull); + + // Checking that this doesn't throw. + await fakeVmServiceHost.vmService.setTimelineFlags(['test']); + + final vm_service.Response timeline = await fakeVmServiceHost.vmService.getTimeline(); + expect(timeline, isNull); expect(fakeVmServiceHost.hasRemainingExpectations, false); });