From 53b87635b05e22cc11efc336fcbf6e0d0eb0d8c4 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Thu, 27 Mar 2025 08:51:38 +0000 Subject: [PATCH] [flutter_tool] Handle RPCErrorKind.kConnectionDisposed (#164299) There's currently a lot of code that handles RPC Errors that contain the text "Service connection disposed" because the error originally did not have a unique error code. A new error code was added in https://dart-review.googlesource.com/c/sdk/+/381501 but it's not currently used because it won't be caught by existing code. This change updates all places that check for this text, and now also handle the new error code in preperation for the code changing in future. See https://github.com/flutter/flutter/issues/153471 cc @bkonyi ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. Issue listed, but this change does not directly fix it, it just prepares for a related future change that will simplify handling these errors without string checks - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. --- .../lib/src/commands/attach.dart | 1 + .../flutter_tools/lib/src/commands/run.dart | 1 + packages/flutter_tools/lib/src/devfs.dart | 1 + packages/flutter_tools/lib/src/vmservice.dart | 3 ++ .../commands.shard/hermetic/attach_test.dart | 50 ++++++++++++++++++- .../commands.shard/hermetic/run_test.dart | 37 +++++++++++++- .../android/android_device_test.dart | 41 +++++++++++++-- 7 files changed, 129 insertions(+), 5 deletions(-) diff --git a/packages/flutter_tools/lib/src/commands/attach.dart b/packages/flutter_tools/lib/src/commands/attach.dart index e2c26ea285..f4c87cf417 100644 --- a/packages/flutter_tools/lib/src/commands/attach.dart +++ b/packages/flutter_tools/lib/src/commands/attach.dart @@ -430,6 +430,7 @@ known, it can be explicitly provided to attach via the command-line, e.g. } } on RPCError catch (err) { if (err.code == RPCErrorKind.kServiceDisappeared.code || + err.code == RPCErrorKind.kConnectionDisposed.code || err.message.contains('Service connection disposed')) { throwToolExit('Lost connection to device.'); } diff --git a/packages/flutter_tools/lib/src/commands/run.dart b/packages/flutter_tools/lib/src/commands/run.dart index 489fa46e4d..8f6e89eae7 100644 --- a/packages/flutter_tools/lib/src/commands/run.dart +++ b/packages/flutter_tools/lib/src/commands/run.dart @@ -868,6 +868,7 @@ class RunCommand extends RunCommandBase { } } on RPCError catch (error) { if (error.code == RPCErrorKind.kServiceDisappeared.code || + error.code == RPCErrorKind.kConnectionDisposed.code || error.message.contains('Service connection disposed')) { throwToolExit('Lost connection to device.'); } diff --git a/packages/flutter_tools/lib/src/devfs.dart b/packages/flutter_tools/lib/src/devfs.dart index b779d18877..c2779119e2 100644 --- a/packages/flutter_tools/lib/src/devfs.dart +++ b/packages/flutter_tools/lib/src/devfs.dart @@ -519,6 +519,7 @@ class DevFS { _baseUri = Uri.parse(response.json!['uri'] as String); } on vm_service.RPCError catch (rpcException) { if (rpcException.code == vm_service.RPCErrorKind.kServiceDisappeared.code || + rpcException.code == vm_service.RPCErrorKind.kConnectionDisposed.code || rpcException.message.contains('Service connection disposed')) { // This can happen if the device has been disconnected, so translate to // a DevFSException, which the caller will handle. diff --git a/packages/flutter_tools/lib/src/vmservice.dart b/packages/flutter_tools/lib/src/vmservice.dart index 9d23256a43..89d2ad1dfc 100644 --- a/packages/flutter_tools/lib/src/vmservice.dart +++ b/packages/flutter_tools/lib/src/vmservice.dart @@ -486,6 +486,7 @@ class FlutterVmService { return await service.getVM(); } on vm_service.RPCError catch (err) { if (err.code == vm_service.RPCErrorKind.kServiceDisappeared.code || + err.code == vm_service.RPCErrorKind.kConnectionDisposed.code || err.message.contains('Service connection disposed')) { globals.printTrace('VmService.getVm call failed: $err'); return null; @@ -507,6 +508,7 @@ class FlutterVmService { // Swallow the exception here and let the shutdown logic elsewhere deal // with cleaning up. if (e.code == vm_service.RPCErrorKind.kServiceDisappeared.code || + e.code == vm_service.RPCErrorKind.kConnectionDisposed.code || e.message.contains('Service connection disposed')) { return null; } @@ -792,6 +794,7 @@ class FlutterVmService { // disappears while handling a request, return null. if ((err.code == vm_service.RPCErrorKind.kMethodNotFound.code) || (err.code == vm_service.RPCErrorKind.kServiceDisappeared.code) || + (err.code == vm_service.RPCErrorKind.kConnectionDisposed.code) || (err.message.contains('Service connection disposed'))) { return null; } diff --git a/packages/flutter_tools/test/commands.shard/hermetic/attach_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/attach_test.dart index de6b1544fc..1e3e9b7322 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/attach_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/attach_test.dart @@ -1385,7 +1385,55 @@ void main() { ); testUsingContext( - 'Catches "Service connection disposed" error', + 'Catches "Service connection disposed" error by code', + () async { + final FakeAndroidDevice device = + FakeAndroidDevice(id: '1') + ..portForwarder = const NoOpDevicePortForwarder() + ..onGetLogReader = () => NoOpDeviceLogReader('test'); + final FakeHotRunner hotRunner = FakeHotRunner(); + final FakeHotRunnerFactory hotRunnerFactory = FakeHotRunnerFactory()..hotRunner = hotRunner; + hotRunner.onAttach = ( + Completer? connectionInfoCompleter, + Completer? appStartedCompleter, + bool allowExistingDdsInstance, + bool enableDevTools, + ) async { + await null; + throw vm_service.RPCError( + 'flutter._listViews', + vm_service.RPCErrorKind.kConnectionDisposed.code, + 'dummy text not matched', + ); + }; + + testDeviceManager.devices = [device]; + testFileSystem.file('lib/main.dart').createSync(); + + final AttachCommand command = AttachCommand( + hotRunnerFactory: hotRunnerFactory, + stdio: stdio, + logger: logger, + terminal: terminal, + signals: signals, + platform: platform, + processInfo: processInfo, + fileSystem: testFileSystem, + ); + await expectLater( + createTestCommandRunner(command).run(['attach']), + throwsToolExit(message: 'Lost connection to device.'), + ); + }, + overrides: { + FileSystem: () => testFileSystem, + ProcessManager: () => FakeProcessManager.any(), + DeviceManager: () => testDeviceManager, + }, + ); + + testUsingContext( + 'Catches "Service connection disposed" error by text', () async { final FakeAndroidDevice device = FakeAndroidDevice(id: '1') diff --git a/packages/flutter_tools/test/commands.shard/hermetic/run_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/run_test.dart index 7841c03abb..5367d4311e 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/run_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/run_test.dart @@ -1156,7 +1156,7 @@ void main() { }); testUsingContext( - 'Flutter run catches catches errors due to vm service disconnection and throws a tool exit', + 'Flutter run catches catches errors due to vm service disconnection by text and throws a tool exit', () async { final FakeResidentRunner residentRunner = FakeResidentRunner(); residentRunner.rpcError = RPCError( @@ -1190,6 +1190,41 @@ void main() { }, ); + testUsingContext( + 'Flutter run catches catches errors due to vm service disconnection by code and throws a tool exit', + () async { + final FakeResidentRunner residentRunner = FakeResidentRunner(); + residentRunner.rpcError = RPCError( + 'flutter._listViews', + RPCErrorKind.kServiceDisappeared.code, + '', + ); + final TestRunCommandWithFakeResidentRunner command = TestRunCommandWithFakeResidentRunner(); + command.fakeResidentRunner = residentRunner; + + await expectToolExitLater( + createTestCommandRunner(command).run(['run', '--no-pub']), + contains('Lost connection to device.'), + ); + + residentRunner.rpcError = RPCError( + 'flutter._listViews', + RPCErrorKind.kConnectionDisposed.code, + 'dummy text not matched.', + ); + + await expectToolExitLater( + createTestCommandRunner(command).run(['run', '--no-pub']), + contains('Lost connection to device.'), + ); + }, + overrides: { + Cache: () => Cache.test(processManager: FakeProcessManager.any()), + FileSystem: () => MemoryFileSystem.test(), + ProcessManager: () => FakeProcessManager.any(), + }, + ); + testUsingContext( 'Flutter run does not catch other RPC errors', () async { diff --git a/packages/flutter_tools/test/general.shard/android/android_device_test.dart b/packages/flutter_tools/test/general.shard/android/android_device_test.dart index bd3815c9c2..3a2da8053a 100644 --- a/packages/flutter_tools/test/general.shard/android/android_device_test.dart +++ b/packages/flutter_tools/test/general.shard/android/android_device_test.dart @@ -499,10 +499,10 @@ Uptime: 441088659 Realtime: 521464097 }); testUsingContext( - 'AdbLogReader.provideVmService catches any RPCError due to VM service disconnection', + 'AdbLogReader.provideVmService catches any RPCError due to VM service disconnection by text', () async { final BufferLogger logger = globals.logger as BufferLogger; - final FlutterVmService vmService = FlutterVmService(_MyFakeVmService()); + final FlutterVmService vmService = FlutterVmService(_MyFakeVmServiceConnectionDisposedText()); final AdbLogReader logReader = AdbLogReader.test(FakeProcess(), 'foo', logger); await logReader.provideVmService(vmService); expect( @@ -518,15 +518,50 @@ Uptime: 441088659 Realtime: 521464097 }, overrides: {Logger: () => BufferLogger.test()}, ); + + testUsingContext( + 'AdbLogReader.provideVmService catches any RPCError due to VM service disconnection by code', + () async { + final BufferLogger logger = globals.logger as BufferLogger; + final FlutterVmService vmService = FlutterVmService(_MyFakeVmServiceConnectionDisposedCode()); + final AdbLogReader logReader = AdbLogReader.test(FakeProcess(), 'foo', logger); + await logReader.provideVmService(vmService); + expect( + logger.traceText, + 'VmService.getVm call failed: null: (-32010) ' + 'Dummy text not matched\n', + ); + expect( + logger.errorText, + 'An error occurred when setting up filtering for adb logs. ' + 'Unable to communicate with the VM service.\n', + ); + }, + overrides: {Logger: () => BufferLogger.test()}, + ); } -class _MyFakeVmService extends Fake implements VmService { +/// A mock VM Service that throws a generic [RPCErrorKind.kServerError] error +/// with the text "Service connection disposed". +/// +/// This is the way these errors are currently sent (as of Feb 2025) but are +/// planned to be migrated to their own error code (see +/// [_MyFakeVmServiceConnectionDisposedCode]) soon. +class _MyFakeVmServiceConnectionDisposedText extends Fake implements VmService { @override Future getVM() async { throw RPCError(null, RPCErrorKind.kServerError.code, 'Service connection disposed'); } } +/// A mock VM Service that throws a [RPCErrorKind.kConnectionDisposed] error. +class _MyFakeVmServiceConnectionDisposedCode extends Fake implements VmService { + @override + Future getVM() async { + throw RPCError(null, RPCErrorKind.kConnectionDisposed.code, 'Dummy text not matched'); + } +} + AndroidDevice setUpAndroidDevice({ String? id, AndroidSdk? androidSdk,