Skip to content

[Property Editor] Better error handling#9101

Merged
elliette merged 4 commits intoflutter:masterfrom
elliette:issue-9086
Apr 4, 2025
Merged

[Property Editor] Better error handling#9101
elliette merged 4 commits intoflutter:masterfrom
elliette:issue-9086

Conversation

@elliette
Copy link
Copy Markdown
Member

@elliette elliette commented Apr 3, 2025

Fixes #9086

This PR:

  1. Reports errors to GA instead of just logging them in the console
  2. Ignores contentModifiedError returned from getEditableArguments. This is expected if a user edits their code before the request completes. Because another request is sent afterwards, this is expected and does affect functionality.

@elliette elliette requested a review from a team as a code owner April 3, 2025 23:07
@elliette elliette requested review from kenzieschmoll and removed request for a team April 3, 2025 23:07
Comment on lines +290 to +294
reportError(
errorMessage,
errorType: PropertyEditorSidebar.getEditableArgumentsIdentifier,
stack: st,
);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we could reduce duplication by putting this in a finally clause. If we had a variable String? errorMessage defined before the try, the on RpcException and catch blocks could just set the errorMessage variable that is used in the finally block that calls reportError

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

Comment on lines +338 to +342
reportError(
errorMessage,
errorType: PropertyEditorSidebar.editArgumentIdentifier,
stack: st,
);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

put in finally instead

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

Copy link
Copy Markdown
Member

@kenzieschmoll kenzieschmoll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a couple comments but lgtm

@elliette elliette merged commit 3045a56 into flutter:master Apr 4, 2025
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Property Editor] Error logs in console

2 participants