[L] Change in dart/sdk[main]: DAS plugins: Add print-debugging support in Insights pages.
0 views
Skip to first unread message
Samuel Rawlins (Gerrit)
Mar 11, 2026, 11:17:20 AMMar 11
to Brian Wilkerson, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Brian Wilkerson
Samuel Rawlins added 2 comments![]( "Open in Gerrit")
File pkg/analysis_server/lib/src/plugin/plugin_isolate.dart
Line 57, Patchset 3 (Latest):
List<protocol.PluginPrint> pluginPrints = [];Samuel Rawlins . unresolved
There is potential for this to get very long. Especially if some prints have been accidentally left in production plugin code, and a user has a long-running IDE session.
Should we implement this with a Queue from the get-go, with a max size, dropping the oldest messages as new ones are received?
File pkg/analysis_server_plugin/test/src/plugin_server_error_test.dart
Line 103, Patchset 3 (Parent):
response.error,Samuel Rawlins . resolved
This is maybe interesting. When we wrap the AnalysisRule calls in an additional zone, synchronous errors become treated like async ones, and we no longer receive a RequestError. Instead, as in the test above, we get a PluginError notification. Shouldn't be a difference for the user; it's just a different underlying reporting mechanism is in play.
Related details
Attention is currently required from:
- Brian Wilkerson
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Brian Wilkerson (Gerrit)
Mar 11, 2026, 12:26:52 PMMar 11
to Samuel Rawlins, Brian Wilkerson, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Samuel Rawlins
Brian Wilkerson voted and added 1 comment![]( "Open in Gerrit")
Votes added by Brian Wilkerson
| Code-Review | +1 |
1 comment
File pkg/analysis_server/lib/src/plugin/plugin_isolate.dart
Line 57, Patchset 3 (Latest):
List<protocol.PluginPrint> pluginPrints = [];Samuel Rawlins . unresolved
There is potential for this to get very long. Especially if some prints have been accidentally left in production plugin code, and a user has a long-running IDE session.
Should we implement this with a Queue from the get-go, with a max size, dropping the oldest messages as new ones are received?
Brian Wilkerson
I think we should. As it is this is effectively a memory leak.
Related details
Attention is currently required from:
- Samuel Rawlins
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Samuel Rawlins (Gerrit)
Mar 12, 2026, 3:11:04 PMMar 12
to Brian Wilkerson, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Samuel Rawlins added 1 comment![]( "Open in Gerrit")
File pkg/analysis_server/lib/src/plugin/plugin_isolate.dart
Brian WilkersonThere is potential for this to get very long. Especially if some prints have been accidentally left in production plugin code, and a user has a long-running IDE session.
Should we implement this with a Queue from the get-go, with a max size, dropping the oldest messages as new ones are received?
I think we should. As it is this is effectively a memory leak.
Samuel Rawlins
Done, and I turned the list into a map, so that if there is one very verbose plugin, it won't overwhelm the list of print messages from other plugins.
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Brian Wilkerson (Gerrit)
Mar 12, 2026, 3:58:12 PMMar 12
to Samuel Rawlins, Brian Wilkerson, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Samuel Rawlins
Brian Wilkerson voted and added 2 comments![]( "Open in Gerrit")
Votes added by Brian Wilkerson
| Code-Review | +1 |
2 comments
Patchset-level comments
File pkg/analysis_server/lib/src/status/pages/plugins_page.dart
Line 86, Patchset 4 (Latest):
if (prints != null && prints.isNotEmpty) {Brian Wilkerson . unresolved
Can this ever happen? (I think that if there's a value then the list always has at least one element.)
Related details
Attention is currently required from:
- Samuel Rawlins
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Samuel Rawlins (Gerrit)
Mar 12, 2026, 6:08:00 PMMar 12
to Brian Wilkerson, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Samuel Rawlins added 1 comment![]( "Open in Gerrit")
File pkg/analysis_server/lib/src/status/pages/plugins_page.dart
Line 86, Patchset 4 (Latest):
if (prints != null && prints.isNotEmpty) {Brian Wilkerson . resolved
Can this ever happen? (I think that if there's a value then the list always has at least one element.)
Samuel Rawlins
Hmm I guess not. I can tidy that up.
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Samuel Rawlins (Gerrit)
Mar 13, 2026, 12:48:11 PMMar 13
to Brian Wilkerson, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Brian Wilkerson
Samuel Rawlins added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 6 (Latest):
Samuel Rawlins . resolved
Hi Brian, sorry I had to regenerate some more files; PTAL.
Related details
Attention is currently required from:
- Brian Wilkerson
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Brian Wilkerson (Gerrit)
Mar 13, 2026, 12:49:22 PMMar 13
to Samuel Rawlins, Brian Wilkerson, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Samuel Rawlins
Brian Wilkerson voted and added 1 comment![]( "Open in Gerrit")
Votes added by Brian Wilkerson
| Code-Review | +1 |
1 comment
Patchset-level comments
Attention is currently required from:
- Samuel Rawlins
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Samuel Rawlins (Gerrit)
Mar 13, 2026, 12:50:21 PMMar 13
to Brian Wilkerson, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Samuel Rawlins voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Samuel Rawlins (Gerrit)
Mar 16, 2026, 5:02:53 PMMar 16
to Brian Wilkerson, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Brian Wilkerson
Samuel Rawlins added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 8 (Latest):
Samuel Rawlins . resolved
Sorry Brian, I had to regenerate files and PluginPrint.java was added this time; PTAL.
Related details
Attention is currently required from:
- Brian Wilkerson
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Brian Wilkerson (Gerrit)
Mar 16, 2026, 5:35:29 PMMar 16
to Samuel Rawlins, Brian Wilkerson, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Samuel Rawlins
Brian Wilkerson voted and added 1 comment![]( "Open in Gerrit")
Votes added by Brian Wilkerson
| Code-Review | +1 |
1 comment
Patchset-level comments
Brian Wilkerson . resolved
No problem.
Attention is currently required from:
- Samuel Rawlins
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Samuel Rawlins (Gerrit)
Mar 16, 2026, 5:56:55 PMMar 16
to Brian Wilkerson, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Samuel Rawlins voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Commit Queue (Gerrit)
Mar 16, 2026, 5:57:06 PMMar 16
to Samuel Rawlins, Brian Wilkerson, dart-analys...@google.com, rev...@dartlang.org
Commit Queue submitted the change![]( "Open in Gerrit")
Change information
Commit message:
DAS plugins: Add print-debugging support in Insights pages.
Work towards https://github.com/dart-lang/sdk/issues/61868
This adds a new notification type, 'PluginPrint'. There are several
fields and variables then named 'pluginPrint' or 'print', and I am
definitely open to changing these names, but this is the best one that
I thought of.
PluginPrint has three fields: The name of the plugin that printed, the
message that was printed, and the timestamp.
We wrap each plugin's AnalysisRule invocations with a zone, so that
the `print` handler can know the name of the plugin. The prints are
caught and sent to the server isolate as Notifications. The
PluginIsolate then stores the collected prints. The Plugins Insights
page can then retrieve them and display them.
Manual testing, with 1000 libraries that get new lint reported once per
file, I did not observe a negative performance impact.
Change-Id: Id800ae41781c15ce89d06b563878c2b0edae43d3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/486827
Reviewed-by: Brian Wilkerson <brianwi...@google.com>
Commit-Queue: Samuel Rawlins <sraw...@google.com>
Files:
- M pkg/analysis_server/doc/api.html
- M pkg/analysis_server/integration_test/support/protocol_matchers.dart
- M pkg/analysis_server/lib/src/plugin/notification_manager.dart
- M pkg/analysis_server/lib/src/plugin/plugin_isolate.dart
- M pkg/analysis_server/lib/src/status/pages/plugins_page.dart
- M pkg/analysis_server/test/mocks.dart
- A pkg/analysis_server/tool/spec/generated/java/types/PluginPrint.java
- M pkg/analysis_server_client/lib/src/protocol/protocol_common.dart
- M pkg/analysis_server_plugin/lib/src/plugin_server.dart
- M pkg/analysis_server_plugin/test/src/plugin_server_error_test.dart
- M pkg/analyzer_plugin/api.txt
- M pkg/analyzer_plugin/doc/api.html
- M pkg/analyzer_plugin/lib/protocol/protocol_common.dart
- M pkg/analyzer_plugin/lib/protocol/protocol_constants.dart
- M pkg/analyzer_plugin/lib/protocol/protocol_generated.dart
- M pkg/analyzer_plugin/test/integration/support/integration_test_methods.dart
- M pkg/analyzer_plugin/test/integration/support/protocol_matchers.dart
- M pkg/analyzer_plugin/tool/spec/common_types_spec.html
- M pkg/analyzer_plugin/tool/spec/plugin_spec.html
Change size: L
Delta: 19 files changed, 854 insertions(+), 65 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Brian Wilkerson
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages