-
Notifications
You must be signed in to change notification settings - Fork 29.8k
[flutter_tools] catch ProcessException and throw ToolExit during upgrade #55759
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[flutter_tools] catch ProcessException and throw ToolExit during upgrade #55759
Conversation
| ); | ||
| if (result.exitCode != 0) { | ||
| throwToolExit(null, exitCode: result.exitCode); | ||
| Future<void> attemptReset(String newRevision) async { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this first arg wasn't used
|
|
||
| @override | ||
| Future<bool> attemptReset(FlutterVersion flutterVersion, String newRevision) async => alreadyUpToDate; | ||
| Future<void> attemptReset(String newRevision) async {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the return value was a mistake that should have been cleaned up in my previous PR.
jonahwilliams
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…est.dart Co-Authored-By: Jonah Williams <jonahwilliams@google.com>
* [flutter_tools] fix type error in symbolize (#55212) * [flutter_tools] check first for stable tag, then dev tag (#55342) * [flutter_tools] enable `flutter upgrade` to support force pushed branches (#55594) * [flutter_tools] catch ProcessException and throw ToolExit during upgrade (#55759) * Update engine version to 376ad6a64b08aa26005e3f82aed26de2e290b572 Co-authored-by: Jonah Williams <campfish91@gmail.com> Co-authored-by: Christopher Fujino <fujino@google.com> Co-authored-by: Christopher Fujino <christopherfujino@gmail.com>
Description
Previously the
attemptResetmethod (of theflutter upgradeflow) was invoking git withthrowOnErrorwhile also checking if the exit code was non-zero. Thus we were never actually catching the errors. This wraps the process call in a try, catchingProcessExceptions and throwing tool exits on them.Related Issues
Based on this comment, related to #55576
Tests
Added a new unit test to verify this behavior.
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read [Handling breaking changes].