[flutter_tools/dap] Handle app.stop errors when launching/attaching (#149734)

DanTup · web-flow · commit e250c655d156 · 2024-06-25T10:56:29.000Z
Two issues I noticed when I hit the issue at flutter/flutter#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 Dart-Code/Dart-Code#5124
diff --git 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<void> _handleAppStop(Map<String, Object?> 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<String, Object?> 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
@@ -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<void> responseCompleter = Completer<void>();
+
+        final FlutterLaunchRequestArguments args = FlutterLaunchRequestArguments(
+          cwd: '.',
+          program: 'foo.dart',
+        );
+
+        // Capture any progress events.
+        final List<List<Object?>> progressEvents = <List<Object?>>[];
+        final StreamSubscription<Map<String, Object?>> progressEventsSubscription =
+            adapter.dapToClientProgressEvents
+                .listen((Map<String, Object?> message) {
+                    progressEvents.add(<Object?>[message['event'], (message['body']! as Map<String, Object?>)['message']]);
+                });
+
+        // Capture any console output messages.
+        final List<String> consoleOutputMessages = <String>[];
+        final StreamSubscription<String> consoleOutputMessagesSubscription =
+            adapter.dapToClientMessages
+                .where((Map<String, Object?> message) => message['event'] == 'output')
+                .map((Map<String, Object?> message) => message['body']! as Map<String, Object?>)
+                .where((Map<String, Object?> body) => body['category'] == 'console' || body['category'] == null)
+                .map((Map<String, Object?> 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(<List<Object?>>[
+          <Object?>['progressStart', 'Launching…'],
+          <Object?>['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
@@ -18,6 +18,7 @@ class MockFlutterDebugAdapter extends FlutterDebugAdapter {
     required FileSystem fileSystem,
     required Platform platform,
     bool simulateAppStarted = true,
+    bool simulateAppStopError = false,
     bool supportsRestart = true,
     FutureOr<void> 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<void> Function(MockFlutterDebugAdapter adapter)? preAppStart;
 
@@ -135,6 +139,15 @@ class MockFlutterDebugAdapter extends FlutterDebugAdapter {
         'event': 'app.started',
       });
     }
+    if (simulateAppStopError) {
+      simulateStdoutMessage(<String, Object?>{
+        'event': 'app.stop',
+        'params': <String, Object?>{
+          'appId': 'TEST',
+          'error': 'App stopped due to an error',
+        }
+      });
+    }
   }
 
   /// Handles messages sent from the debug adapter back to the client.
