From e250c655d156d71c35c92ab6d7637e2851013a42 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Tue, 25 Jun 2024 11:56:29 +0100 Subject: [PATCH] [flutter_tools/dap] Handle app.stop errors when launching/attaching (#149734) Two issues I noticed when I hit the issue at https://github.com/flutter/flutter/issues/149258 1. When the an app.stop event arrives from Flutter with an error, DAP does not pass that error back to the client so there's no visibility of the error. 2. If app.stop occurs but no app.start ever did, we leave the progress notification hanging around This fixes both by handling app.stop and closing any open launch progress as well as passing any error to the client. Fixes https://github.com/Dart-Code/Dart-Code/issues/5124 --- .../src/debug_adapters/flutter_adapter.dart | 18 ++++++ .../dap/flutter_adapter_test.dart | 57 +++++++++++++++++++ .../test/general.shard/dap/mocks.dart | 13 +++++ 3 files changed, 88 insertions(+) diff --git a/packages/flutter_tools/lib/src/debug_adapters/flutter_adapter.dart b/packages/flutter_tools/lib/src/debug_adapters/flutter_adapter.dart index aaca81ad7c..53c6fa53dc 100644 --- a/packages/flutter_tools/lib/src/debug_adapters/flutter_adapter.dart +++ b/packages/flutter_tools/lib/src/debug_adapters/flutter_adapter.dart @@ -445,6 +445,22 @@ class FlutterDebugAdapter extends FlutterBaseDebugAdapter with VmServiceInfoFile ); } + /// Handles the app.stop event from Flutter. + Future _handleAppStop(Map params) async { + // It's possible to get an app.stop without ever having an app.start in the + // case of an error, so we may need to clean up the launch progress. + // https://github.com/Dart-Code/Dart-Code/issues/5124 + // https://github.com/flutter/flutter/issues/149258 + launchProgress?.end(); + launchProgress = null; + + // If the stop had an error attached, be sure to pass it to the client. + final Object? error = params['error']; + if (error is String) { + sendConsoleOutput(error); + } + } + /// Handles the daemon.connected event, recording the pid of the flutter_tools process. void _handleDaemonConnected(Map params) { // On Windows, the pid from the process we spawn is the shell running @@ -491,6 +507,8 @@ class FlutterDebugAdapter extends FlutterBaseDebugAdapter with VmServiceInfoFile _handleAppProgress(params); case 'app.started': _handleAppStarted(); + case 'app.stop': + _handleAppStop(params); } if (_eventsToForwardToClient.contains(event)) { diff --git a/packages/flutter_tools/test/general.shard/dap/flutter_adapter_test.dart b/packages/flutter_tools/test/general.shard/dap/flutter_adapter_test.dart index 678b298417..fbb225f4ea 100644 --- a/packages/flutter_tools/test/general.shard/dap/flutter_adapter_test.dart +++ b/packages/flutter_tools/test/general.shard/dap/flutter_adapter_test.dart @@ -289,6 +289,63 @@ void main() { // progressEnd isn't included because we used takeWhile to stop when it arrived above. ])); }); + + test('handles app.stop errors during launch', () async { + final MockFlutterDebugAdapter adapter = MockFlutterDebugAdapter( + fileSystem: MemoryFileSystem.test(style: fsStyle), + platform: platform, + simulateAppStarted: false, + simulateAppStopError: true, + + ); + final Completer responseCompleter = Completer(); + + final FlutterLaunchRequestArguments args = FlutterLaunchRequestArguments( + cwd: '.', + program: 'foo.dart', + ); + + // Capture any progress events. + final List> progressEvents = >[]; + final StreamSubscription> progressEventsSubscription = + adapter.dapToClientProgressEvents + .listen((Map message) { + progressEvents.add([message['event'], (message['body']! as Map)['message']]); + }); + + // Capture any console output messages. + final List consoleOutputMessages = []; + final StreamSubscription consoleOutputMessagesSubscription = + adapter.dapToClientMessages + .where((Map message) => message['event'] == 'output') + .map((Map message) => message['body']! as Map) + .where((Map body) => body['category'] == 'console' || body['category'] == null) + .map((Map body) => body['output']! as String) + .listen(consoleOutputMessages.add); + + // Initialize with progress support. + await adapter.initializeRequest( + MockRequest(), + DartInitializeRequestArguments(adapterID: 'test', supportsProgressReporting: true, ), + (_) {}, + ); + await adapter.configurationDoneRequest(MockRequest(), null, () {}); + await adapter.launchRequest(MockRequest(), args, responseCompleter.complete); + await responseCompleter.future; + await pumpEventQueue(); // Allow async events to be processed. + await progressEventsSubscription.cancel(); + await consoleOutputMessagesSubscription.cancel(); + + // Ensure we got both the start and end progress events. + expect(progressEvents, containsAllInOrder(>[ + ['progressStart', 'Launching…'], + ['progressEnd', null], + // progressEnd isn't included because we used takeWhile to stop when it arrived above. + ])); + + // Also ensure we got console output with the error. + expect(consoleOutputMessages, contains('App stopped due to an error\n')); + }); }); group('attachRequest', () { diff --git a/packages/flutter_tools/test/general.shard/dap/mocks.dart b/packages/flutter_tools/test/general.shard/dap/mocks.dart index fbe9a015d8..62bc892ff6 100644 --- a/packages/flutter_tools/test/general.shard/dap/mocks.dart +++ b/packages/flutter_tools/test/general.shard/dap/mocks.dart @@ -18,6 +18,7 @@ class MockFlutterDebugAdapter extends FlutterDebugAdapter { required FileSystem fileSystem, required Platform platform, bool simulateAppStarted = true, + bool simulateAppStopError = false, bool supportsRestart = true, FutureOr Function(MockFlutterDebugAdapter adapter)? preAppStart, }) { @@ -32,6 +33,7 @@ class MockFlutterDebugAdapter extends FlutterDebugAdapter { fileSystem: fileSystem, platform: platform, simulateAppStarted: simulateAppStarted, + simulateAppStopError: simulateAppStopError, supportsRestart: supportsRestart, preAppStart: preAppStart, ); @@ -43,6 +45,7 @@ class MockFlutterDebugAdapter extends FlutterDebugAdapter { required super.fileSystem, required super.platform, this.simulateAppStarted = true, + this.simulateAppStopError = false, this.supportsRestart = true, this.preAppStart, }) { @@ -54,6 +57,7 @@ class MockFlutterDebugAdapter extends FlutterDebugAdapter { int _seq = 1; final ByteStreamServerChannel clientChannel; final bool simulateAppStarted; + final bool simulateAppStopError; final bool supportsRestart; final FutureOr Function(MockFlutterDebugAdapter adapter)? preAppStart; @@ -135,6 +139,15 @@ class MockFlutterDebugAdapter extends FlutterDebugAdapter { 'event': 'app.started', }); } + if (simulateAppStopError) { + simulateStdoutMessage({ + 'event': 'app.stop', + 'params': { + 'appId': 'TEST', + 'error': 'App stopped due to an error', + } + }); + } } /// Handles messages sent from the debug adapter back to the client.