Skip to content

Catch unable to start app exception #154970

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.

Sign up for GitHub

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

Jump to bottom
Merged
auto-submit merged 6 commits into from
Sep 16, 2024
Merged

Catch unable to start app exception #154970

auto-submit merged 6 commits into from
Sep 16, 2024

Conversation

@christopherfujino
Copy link
Contributor

@christopherfujino christopherfujino commented Sep 11, 2024 •
edited
Loading

Fixes #153433

@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Sep 11, 2024
@christopherfujino
Copy link
Contributor Author

christopherfujino commented Sep 11, 2024

This worked in my local testing. I'm putting out this PR to see if it passes all integration tests. If so, I will add a specific test for this.

@christopherfujino
Copy link
Contributor Author

christopherfujino commented Sep 11, 2024

This worked in my local testing. I'm putting out this PR to see if it passes all integration tests. If so, I will add a specific test for this.

It appears this change is causing a set of integration tests to deadlock. I suspect it's because I'm now await-ing two futures together.

@christopherfujino christopherfujino marked this pull request as ready for review September 11, 2024 23:52
@christopherfujino christopherfujino marked this pull request as draft September 11, 2024 23:52
shihaohong
shihaohong reviewed Sep 12, 2024
View reviewed changes
packages/flutter_tools/lib/src/test/flutter_platform.dart Outdated
globals.printTrace('test $ourTestCount: VM Service uri is not available');
}
watcher?.handleStartedDevice(processVmServiceUri);
// These must be await-ed together, and if the first errors the second will never complete
Copy link
Contributor

@shihaohong shihaohong Sep 12, 2024

Choose a reason for hiding this comment

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

could you explain why if the first errors the second will never complete? setting eagerError: true implies this behavior, but it's not clear why this could happen

Copy link
Contributor Author

@christopherfujino christopherfujino Sep 12, 2024

Choose a reason for hiding this comment

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

Well, in real life, the most likely reason that the remoteChannelCompleter.future would error is because the app failed to start (for whatever reason). If the app fails to start, we're never gonna get a VM service URI, which is what testDevice.vmServiceUri is waiting for.

Copy link
Contributor Author

@christopherfujino christopherfujino Sep 12, 2024

Choose a reason for hiding this comment

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

at a high level, the trickiness here is because testDevice.vmServiceUri and testDevice.start() have dependencies between each other before they will both complete successfully, so if we fail someplace earlier we want to make sure we don't deadlock but handle it the error immediately (in particular, that the error bubble up from the right zone so the test runner knows there was a failure and it ends the test).

@christopherfujino christopherfujino force-pushed the catch-unable-to-start-app-exception branch from 910567b to c68aedc Compare September 13, 2024 00:24
@christopherfujino christopherfujino marked this pull request as ready for review September 13, 2024 00:37
@christopherfujino christopherfujino added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 16, 2024
@auto-submit auto-submit bot merged commit b565379 into flutter:master Sep 16, 2024
@christopherfujino christopherfujino mentioned this pull request Sep 16, 2024
Closed
@christopherfujino christopherfujino deleted the catch-unable-to-start-app-exception branch September 16, 2024 18:06
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 16, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 16, 2024
@engine-flutter-autoroll
b565379 Catch unable to start app exception (flutter/flutter#154970)
7ad0d36
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 17, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 17, 2024
@engine-flutter-autoroll
b565379 Catch unable to start app exception (flutter/flutter#154970)
ce6d9f8
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 17, 2024
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 17, 2024
@engine-flutter-autoroll
b565379 Catch unable to start app exception (flutter/flutter#154970)
79c5cae
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Sep 17, 2024
@engine-flutter-autoroll
Roll Flutter from 5d83a98 to c4c9f47 (15 revisions) (#7656)
4f2b9cd 
flutter/flutter@5d83a98...c4c9f47

2024-09-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from a328f2362414 to 1376288f5c2a (1 revision) (flutter/flutter#155282)
2024-09-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from 04bc90bff86c to a328f2362414 (2 revisions) (flutter/flutter#155280)
2024-09-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 6b698138c600 to 04bc90bff86c (3 revisions) (flutter/flutter#155277)
2024-09-16 30870216+gaaclarke@users.noreply.github.com Added .keys to ColorSwatch (flutter/flutter#155262)
2024-09-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 362b9bcdedce to 6b698138c600 (4 revisions) (flutter/flutter#155267)
2024-09-16 engine-flutter-autoroll@skia.org Roll Packages from 330581f to df88c81 (7 revisions) (flutter/flutter#155261)
2024-09-16 sstrickl@google.com Extend 'flutter symbolize' to handle deferred loading units. (flutter/flutter#149315)
2024-09-16 christopherfujino@gmail.com Catch unable to start app exception (flutter/flutter#154970)
2024-09-16 magder@google.com Assert iOS framework artifact contains xcprivacy manifest (flutter/flutter#155187)
2024-09-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 52dcc8a16d8b to 362b9bcdedce (1 revision) (flutter/flutter#155241)
2024-09-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from af4fe97fa13a to 52dcc8a16d8b (1 revision) (flutter/flutter#155238)
2024-09-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 9aaea5a4bd50 to af4fe97fa13a (1 revision) (flutter/flutter#155235)
2024-09-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from fe22a199aca3 to 9aaea5a4bd50 (1 revision) (flutter/flutter#155232)
2024-09-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from f5ccef12c15c to fe22a199aca3 (1 revision) (flutter/flutter#155229)
2024-09-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from 622d4aec2e04 to f5ccef12c15c (1 revision) (flutter/flutter#155215)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC rmistry@google.com,stuartmorgan@google.com,tarrinneal@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
This was referenced Sep 18, 2024
Open
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 11, 2024
@engine-flutter-autoroll
b565379 Catch unable to start app exception (flutter/flutter#154970)
7a85fc6
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 12, 2024
@engine-flutter-autoroll
b565379 Catch unable to start app exception (flutter/flutter#154970)
17b7807
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bkonyi bkonyi bkonyi approved these changes

@andrewkolos andrewkolos Awaiting requested review from andrewkolos

+1 more reviewer

@shihaohong shihaohong shihaohong left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Sometimes integration test fails to start on iOS simulator and flutter test takes 12 minutes to time out

3 participants

@christopherfujino @bkonyi @shihaohong