Skip to content

If one method of finding Dart VM fails for CoreDevice, wait for the other method - #139754

Merged
auto-submit[bot] merged 1 commit into
flutter:masterfrom
vashworth:core_device_dart_vm_discovery_fix
Dec 8, 2023
Merged

auto-submit[bot] merged 1 commit into
flutter:masterfrom
vashworth:core_device_dart_vm_discovery_fix

Conversation

@vashworth

@vashworth vashworth commented Dec 7, 2023 •

Copy link
Copy Markdown
Contributor

For CoreDevices we use a combination of mDNS and device logs to find the Dart VM url. If mDNS fails first, it will cause the launch to fail even though the device logs may be able to find the url. So if one of the methods fails, wait for the other method before failing the launch.

Fixes #139685.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@github-actions github-actions Bot added platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels. labels Dec 7, 2023
@vashworth
vashworth marked this pull request as ready for review December 7, 2023 21:54
<Future<Uri?>>[vmUrlFromMDns, vmUrlFromLogs]
);

// If the first future to return is null, wait for the other to complete.

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.

this is ugly, but I can't think of a less ugly way to handle this, so SGTM.

@christopherfujino christopherfujino 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

@vashworth vashworth added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 8, 2023
@auto-submit
auto-submit Bot merged commit 82bc9ca into flutter:master Dec 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App platform-ios iOS applications 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.

Mac_ios microbenchmarks_ios_xcode_debug flakes on failing to find Dart VM url with "mDNS query failed"

2 participants