Skip to content

Do not throw when trying to discover a fuchsia device and the sshConfig is invalid#52858

Merged
dnfield merged 3 commits intoflutter:masterfrom
dnfield:fuchsia_throw
Mar 19, 2020
Merged

Do not throw when trying to discover a fuchsia device and the sshConfig is invalid#52858
dnfield merged 3 commits intoflutter:masterfrom
dnfield:fuchsia_throw

Conversation

@dnfield
Copy link
Copy Markdown
Contributor

@dnfield dnfield commented Mar 18, 2020

Description

If there is a Fuchsia device discoverable on your network (via dev_finder), but you don't have any ssh config set up (e.g. because you're not actually developing for Fuchsia on this machine), the tool currently throws if you try to flutter run or flutter devices.

This makes it so we print an error and return the same default as in other error conditions in these code paths.

Related Issues

Fixes #52849

Tests

I added the following tests:

Test that we don't throw if the ssh config is not present.

@dnfield dnfield requested a review from zanderso March 18, 2020 23:07
@fluttergithubbot fluttergithubbot added the tool Affects the "flutter" command-line tool. See also t: labels. label Mar 18, 2020
@dnfield dnfield added the platform-fuchsia Fuchsia code specifically label Mar 18, 2020
final File pm;

/// Returns true if the [sshConfig] file is not null and exists.
bool validateSshConfig() {
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.

A getter like the following would probably be more idiomatic:

bool get hasSshConfig => sshConfig != null && sshConfig.existsSync();

(or hasValidSshConfig)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. At one point this was a method that was also checking that pm and dev_finder were executable, which seemed heavy for a getter, but also turned out to be less helpful than I thought.

Future<String> get sdkNameAndVersion async {
const String defaultName = 'Fuchsia';
if (!globals.fuchsiaArtifacts.validateSshConfig()) {
globals.printError('Could not determine Fuchsia sdk name or version '
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.

These could be printTrace. If someone is really trying to run on Fuchsia, they'll get a better error message later.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done and done.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Errrr - I changed the wrong one, I'll fix that.

Future<TargetPlatform> _queryTargetPlatform() async {
const TargetPlatform defaultTargetPlatform = TargetPlatform.fuchsia_arm64;
if (!globals.fuchsiaArtifacts.validateSshConfig()) {
globals.printError('Could not determine Fuchsia target platform because '
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.

printTrace instead of printError.

@dnfield dnfield merged commit 53dc8db into flutter:master Mar 19, 2020
@dnfield dnfield deleted the fuchsia_throw branch March 19, 2020 16:29
@lock
Copy link
Copy Markdown

lock Bot commented Apr 2, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@lock lock Bot locked and limited conversation to collaborators Apr 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

platform-fuchsia Fuchsia code specifically tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

flutter run and flutter devices fails with error about FUCHSIA_SSH_CONFIG

4 participants