Move Inspector error tracking out of ErrorBadgeManager - #9923
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors error badge management by moving inspector-specific error tracking logic from ErrorBadgeManager to InspectorScreenController, introducing the InspectableWidgetError class, and updating the UI and tests accordingly. Feedback on these changes highlights opportunities to prevent potential runtime crashes by adding defensive null checks on error.extensionData, properly manage and cancel the extension event stream subscription to avoid memory leaks on reconnects, and optimize the markErrorAsRead method to use an O(1) lookup instead of iterating over the entire map.
d1af6ac to
3cc614c
Compare
3cc614c to
ad6f03f
Compare
|
Hii @kenzieschmoll, @srawlins can you please add the |
srawlins
left a comment
There was a problem hiding this comment.
I think this looks great. Nice new tests.
|
CC @johnpryan |
|
Thanks @srawlins! |
Fixes #9105
ErrorBadgeManager was doing a lot of Inspector-specific stuff (storing the actual error objects, append/mark-as-read/clear, extracting inspectable errors from Flutter events). Most of that only ever applied to Inspector anyway.
This PR moves that logic into InspectorScreenController and leaves ErrorBadgeManager as a plain unread badge counter. Inspector still owns its unread badge so switching tabs doesn’t clear it (same behavior as before).
Pre-launch Checklist
General checklist
///).Issues checklist
contributions-welcomeorgood-first-issuelabel.contributions-welcomeorgood-first-issuelabel. I understand this means my PR might take longer to be reviewed.Tests checklist
AI-tooling checklist
Feature-change checklist
release-notes-not-requiredlabel or left a comment requesting the label be added.packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md.If you need help, consider asking for help on Discord.