Skip to content

Commit e250c65

Browse files
authored
[flutter_tools/dap] Handle app.stop errors when launching/attaching (flutter#149734)
Two issues I noticed when I hit the issue at 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
1 parent c9f72d5 commit e250c65

File tree

3 files changed

+88
-0
lines changed

3 files changed

+88
-0
lines changed

packages/flutter_tools/lib/src/debug_adapters/flutter_adapter.dart

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,22 @@ class FlutterDebugAdapter extends FlutterBaseDebugAdapter with VmServiceInfoFile
445445
);
446446
}
447447

448+
/// Handles the app.stop event from Flutter.
449+
Future<void> _handleAppStop(Map<String, Object?> params) async {
450+
// It's possible to get an app.stop without ever having an app.start in the
451+
// case of an error, so we may need to clean up the launch progress.
452+
// https://github.com/Dart-Code/Dart-Code/issues/5124
453+
// https://github.com/flutter/flutter/issues/149258
454+
launchProgress?.end();
455+
launchProgress = null;
456+
457+
// If the stop had an error attached, be sure to pass it to the client.
458+
final Object? error = params['error'];
459+
if (error is String) {
460+
sendConsoleOutput(error);
461+
}
462+
}
463+
448464
/// Handles the daemon.connected event, recording the pid of the flutter_tools process.
449465
void _handleDaemonConnected(Map<String, Object?> params) {
450466
// On Windows, the pid from the process we spawn is the shell running
@@ -491,6 +507,8 @@ class FlutterDebugAdapter extends FlutterBaseDebugAdapter with VmServiceInfoFile
491507
_handleAppProgress(params);
492508
case 'app.started':
493509
_handleAppStarted();
510+
case 'app.stop':
511+
_handleAppStop(params);
494512
}
495513

496514
if (_eventsToForwardToClient.contains(event)) {

packages/flutter_tools/test/general.shard/dap/flutter_adapter_test.dart

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,63 @@ void main() {
289289
// progressEnd isn't included because we used takeWhile to stop when it arrived above.
290290
]));
291291
});
292+
293+
test('handles app.stop errors during launch', () async {
294+
final MockFlutterDebugAdapter adapter = MockFlutterDebugAdapter(
295+
fileSystem: MemoryFileSystem.test(style: fsStyle),
296+
platform: platform,
297+
simulateAppStarted: false,
298+
simulateAppStopError: true,
299+
300+
);
301+
final Completer<void> responseCompleter = Completer<void>();
302+
303+
final FlutterLaunchRequestArguments args = FlutterLaunchRequestArguments(
304+
cwd: '.',
305+
program: 'foo.dart',
306+
);
307+
308+
// Capture any progress events.
309+
final List<List<Object?>> progressEvents = <List<Object?>>[];
310+
final StreamSubscription<Map<String, Object?>> progressEventsSubscription =
311+
adapter.dapToClientProgressEvents
312+
.listen((Map<String, Object?> message) {
313+
progressEvents.add(<Object?>[message['event'], (message['body']! as Map<String, Object?>)['message']]);
314+
});
315+
316+
// Capture any console output messages.
317+
final List<String> consoleOutputMessages = <String>[];
318+
final StreamSubscription<String> consoleOutputMessagesSubscription =
319+
adapter.dapToClientMessages
320+
.where((Map<String, Object?> message) => message['event'] == 'output')
321+
.map((Map<String, Object?> message) => message['body']! as Map<String, Object?>)
322+
.where((Map<String, Object?> body) => body['category'] == 'console' || body['category'] == null)
323+
.map((Map<String, Object?> body) => body['output']! as String)
324+
.listen(consoleOutputMessages.add);
325+
326+
// Initialize with progress support.
327+
await adapter.initializeRequest(
328+
MockRequest(),
329+
DartInitializeRequestArguments(adapterID: 'test', supportsProgressReporting: true, ),
330+
(_) {},
331+
);
332+
await adapter.configurationDoneRequest(MockRequest(), null, () {});
333+
await adapter.launchRequest(MockRequest(), args, responseCompleter.complete);
334+
await responseCompleter.future;
335+
await pumpEventQueue(); // Allow async events to be processed.
336+
await progressEventsSubscription.cancel();
337+
await consoleOutputMessagesSubscription.cancel();
338+
339+
// Ensure we got both the start and end progress events.
340+
expect(progressEvents, containsAllInOrder(<List<Object?>>[
341+
<Object?>['progressStart', 'Launching…'],
342+
<Object?>['progressEnd', null],
343+
// progressEnd isn't included because we used takeWhile to stop when it arrived above.
344+
]));
345+
346+
// Also ensure we got console output with the error.
347+
expect(consoleOutputMessages, contains('App stopped due to an error\n'));
348+
});
292349
});
293350

294351
group('attachRequest', () {

packages/flutter_tools/test/general.shard/dap/mocks.dart

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ class MockFlutterDebugAdapter extends FlutterDebugAdapter {
1818
required FileSystem fileSystem,
1919
required Platform platform,
2020
bool simulateAppStarted = true,
21+
bool simulateAppStopError = false,
2122
bool supportsRestart = true,
2223
FutureOr<void> Function(MockFlutterDebugAdapter adapter)? preAppStart,
2324
}) {
@@ -32,6 +33,7 @@ class MockFlutterDebugAdapter extends FlutterDebugAdapter {
3233
fileSystem: fileSystem,
3334
platform: platform,
3435
simulateAppStarted: simulateAppStarted,
36+
simulateAppStopError: simulateAppStopError,
3537
supportsRestart: supportsRestart,
3638
preAppStart: preAppStart,
3739
);
@@ -43,6 +45,7 @@ class MockFlutterDebugAdapter extends FlutterDebugAdapter {
4345
required super.fileSystem,
4446
required super.platform,
4547
this.simulateAppStarted = true,
48+
this.simulateAppStopError = false,
4649
this.supportsRestart = true,
4750
this.preAppStart,
4851
}) {
@@ -54,6 +57,7 @@ class MockFlutterDebugAdapter extends FlutterDebugAdapter {
5457
int _seq = 1;
5558
final ByteStreamServerChannel clientChannel;
5659
final bool simulateAppStarted;
60+
final bool simulateAppStopError;
5761
final bool supportsRestart;
5862
final FutureOr<void> Function(MockFlutterDebugAdapter adapter)? preAppStart;
5963

@@ -135,6 +139,15 @@ class MockFlutterDebugAdapter extends FlutterDebugAdapter {
135139
'event': 'app.started',
136140
});
137141
}
142+
if (simulateAppStopError) {
143+
simulateStdoutMessage(<String, Object?>{
144+
'event': 'app.stop',
145+
'params': <String, Object?>{
146+
'appId': 'TEST',
147+
'error': 'App stopped due to an error',
148+
}
149+
});
150+
}
138151
}
139152

140153
/// Handles messages sent from the debug adapter back to the client.

0 commit comments

Comments
 (0)