From 3f038aea0bbcee98cd741930436545c013b06b3f Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Tue, 24 Sep 2024 12:30:06 +0100 Subject: [PATCH] [flutter_tools] Fix encoded stderr in "dart.log" from debug adapter to client (#155249) To aid debugging, debug adapter (DAP) clients can ask the debug adapters to send verbose logs back to the client (so they can capture them in a client-side log along with other things happening on the client). Included in this log is the `stderr` output of the `flutter run` process spawned by the debug adapter. This output was not decoded correctly for these logs, so showed up like: ``` [Flutter] [stderr] [91, 32, 32, 43, 49, 52, 32, 109, 115, 93, 32, 67, 111, 117, 108, 100, 32, 110, 111, 116, 32, 102, 105, 110, 100, 32, 97, 110, 32, 111, 112, 116, 105, 111, 110, 32, 110, 97, 109, 101, 100, 32, 34, 105, 110, 118, 97, 108, 105, 100, 34, 46, 10, 10, 91, 32, 32, 32, 32, 32, 32, 32, 32, 93, 32, 82, 117, 110, 32, 39, 102, 108, 117, 116, 116, 101, 114, 32, 45, 104, 39, 32, 40, 111, 114, 32, 39, 102, 108, 117, 116, 116, 101, 114, 32, 60, 99, 111, 109, 109, 97, 110, 100, 62, 32, 45, 104, 39, 41, 32, 102, 111, 114, 32, 97, 118, 97, 105, 108, 97, 98, 108, 101, 32, 102, 108, 117, 116, 116, 101, 114, 32, 99, 111, 109, 109, 97, 110, 100, 115, 32, 97, 110, 100, 32, 111, 112, 116, 105, 111, 110, 115, 46, 10] ``` This change decodes the output before processing. Fixes https://github.com/Dart-Code/Dart-Code/issues/5268 ## 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]. - [x] I listed at least one issue that this PR fixes in the description above. - [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]. [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --- .../src/debug_adapters/flutter_adapter.dart | 4 +-- .../debug_adapters/flutter_base_adapter.dart | 5 +-- .../debug_adapters/flutter_test_adapter.dart | 4 +-- .../debug_adapter/flutter_adapter_test.dart | 31 ++++++++++++++++++- 4 files changed, 37 insertions(+), 7 deletions(-) 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 b066f5f78f..aa494a5932 100644 --- a/packages/flutter_tools/lib/src/debug_adapters/flutter_adapter.dart +++ b/packages/flutter_tools/lib/src/debug_adapters/flutter_adapter.dart @@ -603,9 +603,9 @@ class FlutterDebugAdapter extends FlutterBaseDebugAdapter with VmServiceInfoFile } @override - void handleStderr(List data) { + void handleStderr(String data) { _logTraffic('<== [Flutter] [stderr] $data'); - sendOutput('stderr', utf8.decode(data)); + sendOutput('stderr', data); } /// Handles stdout from the `flutter run --machine` process, decoding the JSON and calling the appropriate handlers. diff --git a/packages/flutter_tools/lib/src/debug_adapters/flutter_base_adapter.dart b/packages/flutter_tools/lib/src/debug_adapters/flutter_base_adapter.dart index fd8d848c6d..2032c65c7d 100644 --- a/packages/flutter_tools/lib/src/debug_adapters/flutter_base_adapter.dart +++ b/packages/flutter_tools/lib/src/debug_adapters/flutter_base_adapter.dart @@ -11,6 +11,7 @@ import '../base/file_system.dart'; import '../base/io.dart'; import '../base/platform.dart'; import '../cache.dart'; +import '../convert.dart'; import 'flutter_adapter_args.dart'; import 'mixins.dart'; @@ -150,11 +151,11 @@ abstract class FlutterBaseDebugAdapter extends DartDebugAdapter data); + void handleStderr(String data); void handleStdout(String data); } diff --git a/packages/flutter_tools/lib/src/debug_adapters/flutter_test_adapter.dart b/packages/flutter_tools/lib/src/debug_adapters/flutter_test_adapter.dart index 22f83d7ec8..7e1b41b31e 100644 --- a/packages/flutter_tools/lib/src/debug_adapters/flutter_test_adapter.dart +++ b/packages/flutter_tools/lib/src/debug_adapters/flutter_test_adapter.dart @@ -105,9 +105,9 @@ class FlutterTestDebugAdapter extends FlutterBaseDebugAdapter with TestAdapter { } @override - void handleStderr(List data) { + void handleStderr(String data) { logger?.call('stderr: $data'); - sendOutput('stderr', utf8.decode(data)); + sendOutput('stderr', data); } /// Handles stdout from the `flutter test --machine` process, decoding the JSON and calling the appropriate handlers. diff --git a/packages/flutter_tools/test/integration.shard/debug_adapter/flutter_adapter_test.dart b/packages/flutter_tools/test/integration.shard/debug_adapter/flutter_adapter_test.dart index f659fcf7ad..15b77f90b3 100644 --- a/packages/flutter_tools/test/integration.shard/debug_adapter/flutter_adapter_test.dart +++ b/packages/flutter_tools/test/integration.shard/debug_adapter/flutter_adapter_test.dart @@ -74,7 +74,7 @@ void main() { ); }); - testWithoutContext('logs to client when sendLogsToClient=true', () async { + testWithoutContext('logs stdout to client when sendLogsToClient=true', () async { final BasicProject project = BasicProject(); await project.setUpIn(tempDir); @@ -107,6 +107,35 @@ void main() { ); }); + testWithoutContext('logs stderr to client when sendLogsToClient=true', () async { + final BasicProject project = BasicProject(); + await project.setUpIn(tempDir); + + // Capture all log events. + final Future> logEventsFuture = dap.client.events('dart.log').toList(); + + // Launch the app and wait for it to terminate (because of the error). + await Future.wait(>[ + dap.client.event('terminated'), + dap.client.start( + launch: () => dap.client.launch( + cwd: project.dir.path, + noDebug: true, + toolArgs: ['--not-a-valid-flag'], + sendLogsToClient: true, + ), + ), + ], eagerError: true); + + // Ensure logs contain the expected error message. + final List logEvents = await logEventsFuture; + final List logMessages = logEvents.map((Event l) => (l.body! as Map)['message']! as String).toList(); + expect( + logMessages, + contains(startsWith('<== [Flutter] [stderr] Could not find an option named "not-a-valid-flag"')), + ); + }); + testWithoutContext('can run and terminate a Flutter app in noDebug mode', () async { final BasicProject project = BasicProject(); await project.setUpIn(tempDir);