[flutter_tools/dap] Improve rendering of structured errors via DAP (#131251)

In the legacy VS Code DAP, we would deserialise the Flutter.Error event
and provide some basic colouring (eg. stack frames are faded if not from
user code and the text is split between stdout/stderr to allow the
client to colour it).

In the new DAPs we originally used `renderedErrorText` which didn't
support either of these. This change adds changes to use the structured
data (with some basic parsing because the source classes are in
package:flutter and not accessible here) to provide a similar
experience.

It would be nicer if we could use the real underlying Flutter classes
for this deserialisation, but extracting them from `package:flutter` and
removing all dependencies on Flutter is a much larger job and I don't
think should hold up providing improved error formatting for the new
DAPs.

Some comparisons:


![1_comparison](https://github.com/flutter/flutter/assets/1078012/74e7e6d6-c8d0-471f-b584-37ae148b0ce7)


![2_comparison](https://github.com/flutter/flutter/assets/1078012/21888934-6f2f-4048-86d7-bdf92d5c7301)

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

+// Copyright 2014 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'dart:math' as math;
+
+typedef _OutputSender = void Function(String category, String message, {bool? parseStackFrames, int? variablesReference});
+
+/// A formatter for improving the display of Flutter structured errors over DAP.
+///
+/// The formatter deserializes a `Flutter.Error` event and produces output
+/// similar to the `renderedErrorText` field, but may include ansi color codes
+/// to provide improved formatting (such as making stack frames from non-user
+/// code faint) if the client indicated support.
+///
+/// Lines that look like stack frames will be marked so they can be parsed by
+/// the base adapter and attached as [Source]s to allow them to be clickable
+/// in the client.
+class FlutterErrorFormatter {
+  final List<_BatchedOutput> batchedOutput = <_BatchedOutput>[];
+
+  /// Formats a Flutter error.
+  ///
+  /// If this is not the first error since the reload, only a summary will be
+  /// included.
+  void formatError(Map<String, Object?> errorData) {
+    final _ErrorData data = _ErrorData(errorData);
+
+    const int assumedTerminalSize = 80;
+    const String barChar = '═';
+    final String headerPrefix = barChar * 8;
+    final String headerSuffix = barChar * math.max( assumedTerminalSize - (data.description?.length ?? 0) - 2 - headerPrefix.length, 0);
+    final String header = '$headerPrefix ${data.description} $headerSuffix';
+    _write('');
+    _write(header, isError: true);
+
+    if (data.errorsSinceReload == 0) {
+      data.properties.forEach(_writeNode);
+      data.children.forEach(_writeNode);
+    } else {
+      data.properties.forEach(_writeSummary);
+    }
+
+    _write(barChar * header.length, isError: true);
+  }
+
+  /// Sends all collected output through [sendOutput].
+  void sendOutput(_OutputSender sendOutput) {
+    for (final _BatchedOutput output in batchedOutput) {
+      sendOutput(
+        output.isError ? 'stderr' : 'stdout',
+        output.output,
+        parseStackFrames: output.parseStackFrames,
+      );
+    }
+  }
+
+  /// Writes [text] to the output.
+  ///
+  /// If the last item in the batch has the same settings as this item, it will
+  /// be appended to the same item, otherwise a new item will be added to the
+  /// batch.
+  void _write(
+    String? text, {
+    int indent = 0,
+    bool isError = false,
+    bool parseStackFrames = false,
+  }) {
+    if (text != null) {
+      final String indentString = '    ' * indent;
+      final String message = '$indentString${text.trim()}';
+
+      _BatchedOutput? output = batchedOutput.lastOrNull;
+      if (output == null || output.isError != isError || output.parseStackFrames != parseStackFrames) {
+        batchedOutput.add(output = _BatchedOutput(isError, parseStackFrames: parseStackFrames));
+      }
+      output.writeln(message);
+    }
+  }
+
+  /// Writes [node] to the output using [indent], recursing unless [recursive]
+  /// is `false`.
+  void _writeNode(_ErrorNode node, {int indent = 0, bool recursive = true}) {
+    // Errors, summaries and lines starting "Exception:" are marked as errors so
+    // they go to stderr instead of stdout (this may cause the client to colour
+    // them like errors).
+    final bool showAsError = node.level == _DiagnosticsNodeLevel.error ||
+        node.level == _DiagnosticsNodeLevel.summary ||
+        (node.description?.startsWith('Exception: ') ?? false);
+
+    if (node.showName && node.name != null) {
+      _write('${node.name}: ${node.description}', indent: indent, isError: showAsError);
+    } else if (node.description?.startsWith('#') ?? false) {
+      // Possible stack frame.
+      _write(node.description, indent: indent, isError: showAsError, parseStackFrames: true);
+    } else {
+      _write(node.description, indent: indent, isError: showAsError);
+    }
+
+    if (recursive) {
+      if (node.style != _DiagnosticsNodeStyle.flat) {
+        indent++;
+      }
+      _writeNodes(node.properties, indent: indent);
+      _writeNodes(node.children, indent: indent);
+    }
+  }
+
+  /// Writes [nodes] to the output.
+  void _writeNodes(List<_ErrorNode> nodes, {int indent = 0, bool recursive = true}) {
+    for (final _ErrorNode child in nodes) {
+      _writeNode(child, indent: indent, recursive: recursive);
+    }
+  }
+
+  /// Writes a simple summary of [node] to the output.
+  void _writeSummary(_ErrorNode node) {
+    final bool allChildrenAreLeaf = node.children.isNotEmpty &&
+        !node.children.any((_ErrorNode child) => child.children.isNotEmpty);
+    if (node.level == _DiagnosticsNodeLevel.summary || allChildrenAreLeaf) {
+      _writeNode(node, recursive: false);
+    }
+  }
+}
+
+/// A container for output to be sent to the client.
+///
+/// When multiple lines are being sent, they may be written to the same batch
+/// if the output options (error/stackFrame) are the same.
+class _BatchedOutput {
+  _BatchedOutput(this.isError, {this.parseStackFrames = false});
+
+  final bool isError;
+  final bool parseStackFrames;
+  final StringBuffer _buffer = StringBuffer();
+
+  String get output => _buffer.toString();
+
+  void writeln(String output) => _buffer.writeln(output);
+}
+
+enum _DiagnosticsNodeLevel {
+  error,
+  summary,
+}
+
+enum _DiagnosticsNodeStyle {
+  flat,
+}
+
+class _ErrorData extends _ErrorNode {
+  _ErrorData(super.data);
+
+  int get errorsSinceReload => data['errorsSinceReload'] as int? ?? 0;
+  String get renderedErrorText => data['renderedErrorText'] as String? ?? '';
+}
+
+class _ErrorNode {
+  _ErrorNode(this.data);
+
+  final Map<Object, Object?> data;
+
+  List<_ErrorNode> get children => asList('children', _ErrorNode.new);
+  String? get description => asString('description');
+  _DiagnosticsNodeLevel? get level => asEnum('level', _DiagnosticsNodeLevel.values);
+  String? get name => asString('name');
+  List<_ErrorNode> get properties => asList('properties', _ErrorNode.new);
+  bool get showName => data['showName'] != false;
+  _DiagnosticsNodeStyle? get style => asEnum('style', _DiagnosticsNodeStyle.values);
+
+  String? asString(String field) {
+    final Object? value = data[field];
+    return value is String ? value : null;
+  }
+
+  T? asEnum<T extends Enum>(String field, Iterable<T> enumValues) {
+    final String? value = asString(field);
+    return value != null ? enumValues.asNameMap()[value] : null;
+  }
+
+  List<T> asList<T>(String field, T Function(Map<Object, Object?>) constructor) {
+    final Object? objects = data[field];
+    return objects is List && objects.every((Object? element) => element is Map<String, Object?>)
+      ? objects.cast<Map<Object, Object?>>().map(constructor).toList()
+      : <T>[];
+  }
+}

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

@@ -12,6 +12,7 @@ import '../base/io.dart';
 import '../cache.dart';
 import '../convert.dart';
 import '../globals.dart' as globals show fs;
+import 'error_formatter.dart';
 import 'flutter_adapter_args.dart';
 import 'flutter_base_adapter.dart';
 
@@ -219,17 +220,14 @@ class FlutterDebugAdapter extends FlutterBaseDebugAdapter with VmServiceInfoFile
 
   /// Sends OutputEvents to the client for a Flutter.Error event.
   void _handleFlutterErrorEvent(vm.ExtensionData? data) {
-    final Map<String, dynamic>? errorData = data?.data;
+    final Map<String, Object?>? errorData = data?.data;
     if (errorData == null) {
       return;
     }
 
-    final String errorText = (errorData['renderedErrorText'] as String?)
-        ?? (errorData['description'] as String?)
-        // We should never not error text, but if we do at least send something
-        // so it's not just completely silent.
-        ?? 'Unknown error in Flutter.Error event';
-    sendOutput('stderr', '$errorText\n');
+    FlutterErrorFormatter()
+      ..formatError(errorData)
+      ..sendOutput(sendOutput);
   }
 
   /// Called by [launchRequest] to request that we actually start the app to be run/debugged.

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

@@ -24,6 +24,7 @@ class FlutterAttachRequestArguments
     super.cwd,
     super.env,
     super.additionalProjectPaths,
+    super.allowAnsiColorOutput,
     super.debugSdkLibraries,
     super.debugExternalPackageLibraries,
     super.evaluateGettersInDebugViews,
@@ -109,6 +110,7 @@ class FlutterLaunchRequestArguments
     super.cwd,
     super.env,
     super.additionalProjectPaths,
+    super.allowAnsiColorOutput,
     super.debugSdkLibraries,
     super.debugExternalPackageLibraries,
     super.evaluateGettersInDebugViews,

packages/flutter_tools/test/integration.shard/debug_adapter/flutter_adapter_test.dart

@@ -13,6 +13,7 @@ import 'package:flutter_tools/src/globals.dart' as globals;
 import '../../src/common.dart';
 import '../test_data/basic_project.dart';
 import '../test_data/compile_error_project.dart';
+import '../test_data/project.dart';
 import '../test_utils.dart';
 import 'test_client.dart';
 import 'test_server.dart';
@@ -187,13 +188,18 @@ void main() {
       expect(output, contains('Exited (1)'));
     });
 
-    /// Helper that tests exception output in either debug or noDebug mode.
-    Future<void> testExceptionOutput({required bool noDebug}) async {
-        final BasicProjectThatThrows project = BasicProjectThatThrows();
+    group('structured errors', () {
+      /// Helper that runs [project] and collects the output.
+      ///
+      /// Line and column numbers are replaced with "1" to avoid fragile tests.
+      Future<String> getExceptionOutput(
+        Project project, {
+        required bool noDebug,
+        required bool ansiColors,
+      }) async {
         await project.setUpIn(tempDir);
 
-        final List<OutputEventBody> outputEvents =
-            await dap.client.collectAllOutput(launch: () {
+        final List<OutputEventBody> outputEvents = await dap.client.collectAllOutput(launch: () {
           // Terminate the app after we see the exception because otherwise
           // it will keep running and `collectAllOutput` won't end.
           dap.client.output
@@ -203,23 +209,85 @@ void main() {
             noDebug: noDebug,
             cwd: project.dir.path,
             toolArgs: <String>['-d', 'flutter-tester'],
+            allowAnsiColorOutput: ansiColors,
           );
         });
 
-        final String output = _uniqueOutputLines(outputEvents);
-        final List<String> outputLines = output.split('\n');
-        expect( outputLines, containsAllInOrder(<String>[
-            '══╡ EXCEPTION CAUGHT BY WIDGETS LIBRARY ╞═══════════════════════════════════════════════════════════',
-            'The following _Exception was thrown building App(dirty):',
-            'Exception: c',
-            'The relevant error-causing widget was:',
-        ]));
-        expect(output, contains('App:${Uri.file(project.dir.path)}/lib/main.dart:24:12'));
-    }
+        String output = _uniqueOutputLines(outputEvents);
+
+        // Replace out any line/columns to make tests less fragile.
+        output = output.replaceAll(RegExp(r'\.dart:\d+:\d+'), '.dart:1:1');
+
+        return output;
+      }
+
+      testWithoutContext('correctly outputs exceptions in debug mode', () async {
+        final BasicProjectThatThrows project = BasicProjectThatThrows();
+        final String output = await getExceptionOutput(project, noDebug: false, ansiColors: false);
 
-    testWithoutContext('correctly outputs exceptions in debug mode', () => testExceptionOutput(noDebug: false));
+        expect(
+          output,
+          contains('''
+════════ Exception caught by widgets library ═══════════════════════════════════
+The following _Exception was thrown building App(dirty):
+Exception: c
 
-    testWithoutContext('correctly outputs exceptions in noDebug mode', () => testExceptionOutput(noDebug: true));
+The relevant error-causing widget was:
+    App App:${Uri.file(project.dir.path)}/lib/main.dart:1:1'''),
+        );
+      });
+
+      testWithoutContext('correctly outputs colored exceptions when supported', () async {
+        final BasicProjectThatThrows project = BasicProjectThatThrows();
+        final String output = await getExceptionOutput(project, noDebug: false, ansiColors: true);
+
+        // Frames in the stack trace that are the users own code will be unformatted, but
+        // frames from the framework are faint (starting with `\x1B[2m`).
+
+        expect(
+          output,
+          contains('''
+════════ Exception caught by widgets library ═══════════════════════════════════
+The following _Exception was thrown building App(dirty):
+Exception: c
+
+The relevant error-causing widget was:
+    App App:${Uri.file(project.dir.path)}/lib/main.dart:1:1
+
+When the exception was thrown, this was the stack:
+#0      c (package:test/main.dart:1:1)
+          ^ source: package:test/main.dart
+#1      App.build (package:test/main.dart:1:1)
+          ^ source: package:test/main.dart
+\x1B[2m#2      StatelessElement.build (package:flutter/src/widgets/framework.dart:1:1)\x1B[0m
+          ^ source: package:flutter/src/widgets/framework.dart
+\x1B[2m#3      ComponentElement.performRebuild (package:flutter/src/widgets/framework.dart:1:1)\x1B[0m
+          ^ source: package:flutter/src/widgets/framework.dart'''),
+        );
+      });
+
+      testWithoutContext('correctly outputs exceptions in noDebug mode', () async {
+        final BasicProjectThatThrows project = BasicProjectThatThrows();
+        final String output = await getExceptionOutput(project, noDebug: true, ansiColors: false);
+
+        // When running in noDebug mode, we don't get the Flutter.Error event so
+        // we get the basic Flutter-formatted version of the error.
+        expect(
+          output,
+          contains('''
+══╡ EXCEPTION CAUGHT BY WIDGETS LIBRARY ╞═══════════════════════════════════════════════════════════
+The following _Exception was thrown building App(dirty):
+Exception: c
+
+The relevant error-causing widget was:
+  App'''),
+        );
+        expect(
+          output,
+          contains('App:${Uri.file(project.dir.path)}/lib/main.dart:1:1'),
+        );
+      });
+    });
 
     testWithoutContext('can hot reload', () async {
       final BasicProject project = BasicProject();
@@ -630,10 +698,19 @@ void main() {
 
 /// Extracts the output from a set of [OutputEventBody], removing any
 /// adjacent duplicates and combining into a single string.
+///
+/// If the output event contains a [Source], the name will be shown on the
+/// following line indented and prefixed with `^ source:`.
 String _uniqueOutputLines(List<OutputEventBody> outputEvents) {
   String? lastItem;
   return outputEvents
-      .map((OutputEventBody e) => e.output)
+      .map((OutputEventBody e) {
+        final String output = e.output;
+        final Source? source = e.source;
+        return source != null
+            ? '$output          ^ source: ${source.name}\n'
+            : output;
+      })
       .where((String output) {
         // Skip the item if it's the same as the previous one.
         final bool isDupe = output == lastItem;

packages/flutter_tools/test/integration.shard/debug_adapter/test_client.dart

@@ -155,6 +155,7 @@ class DapTestClient {
     String? cwd,
     bool? noDebug,
     List<String>? additionalProjectPaths,
+    bool? allowAnsiColorOutput,
     bool? debugSdkLibraries,
     bool? debugExternalPackageLibraries,
     bool? evaluateGettersInDebugViews,
@@ -169,6 +170,7 @@ class DapTestClient {
         args: args,
         toolArgs: toolArgs,
         additionalProjectPaths: additionalProjectPaths,
+        allowAnsiColorOutput: allowAnsiColorOutput,
         debugSdkLibraries: debugSdkLibraries,
         debugExternalPackageLibraries: debugExternalPackageLibraries,
         evaluateGettersInDebugViews: evaluateGettersInDebugViews,

packages/flutter_tools/test/integration.shard/debug_adapter/test_support.dart

@@ -31,7 +31,7 @@ final bool useInProcessDap = Platform.environment['DAP_TEST_INTERNAL'] == 'true'
 /// Service traffic (wrapped in a custom 'dart.log' event).
 final bool verboseLogging = Platform.environment['DAP_TEST_VERBOSE'] == 'true';
 
-const String endOfErrorOutputMarker = '════════════════════════════════════════════════════════════════════════════════════════════════════';
+const String endOfErrorOutputMarker = '════════════════════════════════════════════════════════════════════════════════';
 
 /// Expects the lines in [actual] to match the relevant matcher in [expected],
 /// ignoring differences in line endings and trailing whitespace.