Skip to content

Reland: [flutter_tool] Where possible, catch only subtypes of Exception - #52021

Merged
zanderso merged 2 commits into
flutter:masterfrom
zanderso:retry-catch-exception
Mar 6, 2020
Merged

zanderso merged 2 commits into
flutter:masterfrom
zanderso:retry-catch-exception

Conversation

@zanderso

@zanderso zanderso commented Mar 5, 2020

Copy link
Copy Markdown
Member

Reland of #51774 with zip installed on more devicelab machines.

@zanderso zanderso added tool Affects the "flutter" command-line tool. See also t: labels. work in progress; do not review labels Mar 5, 2020
@zanderso

zanderso commented Mar 5, 2020

Copy link
Copy Markdown
Member Author

With asserts enabled, this catch was previously catching an assertion failure, and defaulting to arm64:

With asserts not enabled, nothing was caught and the architecture would be propagated through the build as null, which just happened to work, I guess by accident.

When this PR changed getIOSArchForName() to throw an Exception instead of assert, and the catch was changed to catch an Exception, we correctly(?) fell back on the default of arm64 as the comment under the catch seems to intend. However if the device was actually an armv7f device, then we'd propagate the wrong architecture, leading to a failure from ios_deploy in the device lab.

To try to fix this, I've added a case to getIOSArchForName() to handle armv7f.

@zanderso

zanderso commented Mar 6, 2020

Copy link
Copy Markdown
Member Author

ptal

@jonahwilliams jonahwilliams left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants