[flutter_tools/dap] Improve rendering of structured errors via DAP by DanTup · Pull Request #131251 · flutter/flutter

DanTup · 2023-07-25T10:41:43Z

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:

(fyi @jacob314 @christopherfujino)

Fixes Dart-Code/Dart-Code#4571.

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

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 attaching Source metadata so links are clickable. 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. Fixes Dart-Code/Dart-Code#4571.

Abjithwyn

everything look fine

Abjithwyn

ok

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

christopherfujino · 2023-07-27T21:12:06Z

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

+}
+
+enum _DiagnosticsNodeStyle {
+  flat,


are there going to be more values later?

There are other values in the source enum, but this is the only value we currently use for this formatting, so I trimmed the enums just to the used values (otherwise we'd be needlessly keeping them in-sync with the source).

In future it would be nice to extract these classes/enums from package:flutter into a shared package that flutter_tools can use, but I failed to do that cleanly previously because the source types have lots of references to other Flutter framework types.

christopherfujino

LGTM module a few nits

…tter.dart Co-authored-by: Christopher Fujino <[email protected]>

…a DAP (flutter/flutter#131251)

…lutter#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)

…a DAP (flutter/flutter#131251)

Manual roll requested by [email protected] flutter/flutter@1d44fbd...1d59196 2023-07-31 [email protected] Appended period remove & Uri parsing fix. (flutter/flutter#131293) 2023-07-31 [email protected] Fixed regex to show missing assets file error (flutter/flutter#131160) 2023-07-31 [email protected] Update `CheckboxListTile` and `CalendarDatePicker` tests for M2/M3 (flutter/flutter#131363) 2023-07-31 [email protected] Reland --omit-type-checks for benchmarks. (flutter/flutter#131493) 2023-07-31 [email protected] Update the cirrus key jul-31-2023 (flutter/flutter#131624) 2023-07-31 [email protected] Add Expanded/Collapsed State for Semantics (flutter/flutter#131233) 2023-07-31 [email protected] Reland - "Update Unit Tests for M2/M3" (flutter/flutter#131504) 2023-07-31 [email protected] Roll Flutter Engine from ae6d1d60df95 to b83172a4e995 (4 revisions) (flutter/flutter#131614) 2023-07-31 [email protected] Upgrade compile and target sdk versions in tests and benchmarks (flutter/flutter#131428) 2023-07-31 [email protected] Roll Flutter Engine from b11a832ea7d4 to ae6d1d60df95 (1 revision) (flutter/flutter#131611) 2023-07-31 [email protected] Roll Packages from 10aab44 to 60e9a54 (6 revisions) (flutter/flutter#131607) 2023-07-31 [email protected] Fix dartdoc for `ButtonSegment` constructor (flutter/flutter#131400) 2023-07-31 [email protected] [flutter_tools/dap] Improve rendering of structured errors via DAP (flutter/flutter#131251) 2023-07-31 [email protected] [doc] Fix module_test_ios comments (flutter/flutter#131470) 2023-07-31 [email protected] Use Flutter app project's NDK version from FFI plugin (flutter/flutter#131141) 2023-07-31 [email protected] Roll Flutter Engine from 22f9aad5aba5 to b11a832ea7d4 (2 revisions) (flutter/flutter#131597) 2023-07-31 [email protected] Roll Flutter Engine from b84c93601684 to 22f9aad5aba5 (3 revisions) (flutter/flutter#131592) 2023-07-31 [email protected] Roll Flutter Engine from d95adb9c8bc6 to b84c93601684 (1 revision) (flutter/flutter#131585) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…lutter#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)

github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jul 25, 2023

Abjithwyn reviewed Jul 25, 2023

View reviewed changes

Avoid hard-coded line/cols in DAP integration tests

2058a7c

Abjithwyn reviewed Jul 25, 2023

View reviewed changes

Avoid test failures caused by path wrapping in noDebug mode

58942db

christopherfujino reviewed Jul 27, 2023

View reviewed changes

packages/flutter_tools/lib/src/debug_adapters/error_formatter.dart Outdated Show resolved Hide resolved

packages/flutter_tools/lib/src/debug_adapters/error_formatter.dart Outdated Show resolved Hide resolved

christopherfujino reviewed Jul 27, 2023

View reviewed changes

christopherfujino approved these changes Jul 27, 2023

View reviewed changes

DanTup and others added 3 commits July 31, 2023 12:01

Fix typo in packages/flutter_tools/lib/src/debug_adapters/error_forma…

d791fa9

…tter.dart Co-authored-by: Christopher Fujino <[email protected]>

Fix typo in packages/flutter_tools/lib/src/debug_adapters/error_forma…

fd55a9a

…tter.dart Co-authored-by: Christopher Fujino <[email protected]>

Merge branch 'master' into dap-improve-structured-errors

e26de2f

DanTup merged commit 0386f91 into flutter:master Jul 31, 2023

engine-flutter-autoroll mentioned this pull request Jul 31, 2023

Roll Flutter from 1d44fbd8b906 to 28ee5581784e (7 revisions) flutter/packages#4610

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 31, 2023

0386f91 [flutter_tools/dap] Improve rendering of structured errors vi…

57d308e

…a DAP (flutter/flutter#131251)

engine-flutter-autoroll mentioned this pull request Jul 31, 2023

Roll Flutter from 1d44fbd8b906 to 0881d07e135d (8 revisions) flutter/packages#4611

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 31, 2023

0386f91 [flutter_tools/dap] Improve rendering of structured errors vi…

fc2f7d4

…a DAP (flutter/flutter#131251)

engine-flutter-autoroll mentioned this pull request Jul 31, 2023

Roll Flutter from 1d44fbd8b906 to 972d36c4efee (9 revisions) flutter/packages#4612

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 31, 2023

0386f91 [flutter_tools/dap] Improve rendering of structured errors vi…

2a670ca

…a DAP (flutter/flutter#131251)

engine-flutter-autoroll mentioned this pull request Jul 31, 2023

Roll Flutter from 1d44fbd8b906 to ae750e57f5e9 (10 revisions) flutter/packages#4615

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 31, 2023

0386f91 [flutter_tools/dap] Improve rendering of structured errors vi…

74fe5c4

…a DAP (flutter/flutter#131251)

engine-flutter-autoroll mentioned this pull request Jul 31, 2023

Roll Flutter from 1d44fbd8b906 to 544c04089686 (11 revisions) flutter/packages#4617

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 31, 2023

0386f91 [flutter_tools/dap] Improve rendering of structured errors vi…

8c2ebfd

…a DAP (flutter/flutter#131251)

engine-flutter-autoroll mentioned this pull request Jul 31, 2023

Manual roll Flutter from 1d44fbd8b906 to 1d59196bafdb (18 revisions) flutter/packages#4621

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 31, 2023

0386f91 [flutter_tools/dap] Improve rendering of structured errors vi…

72dae70

…a DAP (flutter/flutter#131251)

DanTup mentioned this pull request Aug 31, 2023

Debug Console doesn't add clickable links for stdout output when using new Flutter SDK debug adapters Dart-Code/Dart-Code#4711

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

[flutter_tools/dap] Improve rendering of structured errors via DAP#131251
DanTup merged 6 commits intoflutter:masterfrom
DanTup:dap-improve-structured-errors

DanTup commented Jul 25, 2023

Uh oh!

Abjithwyn left a comment

Uh oh!

Abjithwyn left a comment

Uh oh!

Uh oh!

Uh oh!

christopherfujino Jul 27, 2023

Uh oh!

DanTup Jul 31, 2023

Uh oh!

christopherfujino left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

DanTup commented Jul 25, 2023

Pre-launch Checklist

Uh oh!

Abjithwyn left a comment

Choose a reason for hiding this comment

Uh oh!

Abjithwyn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

christopherfujino Jul 27, 2023

Choose a reason for hiding this comment

Uh oh!

DanTup Jul 31, 2023

Choose a reason for hiding this comment

Uh oh!

christopherfujino left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants