Skip to content
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

Use adb logcat to discover if startApp might have failed. #162185

Closed
wants to merge 2 commits into from
Closed

Use adb logcat to discover if startApp might have failed. #162185

wants to merge 2 commits into from
+243 −9

Conversation

matanlurey
Copy link
Contributor

@matanlurey matanlurey commented Jan 24, 2025 •
edited
Loading

Changes how AndroidDevice.startApp decides if the launch succeeded or failed by sniffing adb logcat instead of just relying on the stdout of adb shell am start, which from what I can tell is extremely inconsistent.

This hypothetically could have false positives if there are startup logs that print the text Error: (this false positive existed before too, but there was just a good chance we'd miss the log), so I'm open to other suggestions as well.

Fixes #162087.

After this change, flutter drive lib/will_never_start_successfully.dart does fail:

✓ Built build/app/outputs/flutter-apk/app-debug.apk
Installing build/app/outputs/flutter-apk/app-debug.apk...          588ms
E/flutter ( 7055): [ERROR:flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc(285)] Break on 'ImpellerValidationBreak' to inspect point of failure: Device does not support required Android extension: VK_KHR_external_semaphore_fd
E/flutter ( 7055): [ERROR:flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc(310)] Break on 'ImpellerValidationBreak' to inspect point of failure: Device not suitable since required extensions are not supported.
E/flutter ( 7055): [ERROR:flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc(393)] Break on 'ImpellerValidationBreak' to inspect point of failure: Device doesn't support the required queues.
E/flutter ( 7055): [ERROR:flutter/impeller/renderer/backend/vulkan/context_vk.cc(278)] Break on 'ImpellerValidationBreak' to inspect point of failure: No valid Vulkan device found.
I/flutter ( 7055): [IMPORTANT:flutter/shell/platform/android/android_context_vk_impeller.cc(60)] Using the Impeller rendering backend (Vulkan).
F/flutter ( 7055): [FATAL:flutter/shell/platform/android/platform_view_android.cc(127)] Check failed: android_context_->IsValid(). Could not create surface from invalid Android context.
I/flutter ( 7055): The Dart VM service is listening on http://127.0.0.1:45457/DBy2tSfAFDY=/
Application failed to start on attempt: 1
Installing build/app/outputs/flutter-apk/app-debug.apk...          524ms
E/flutter ( 7166): [ERROR:flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc(285)] Break on 'ImpellerValidationBreak' to inspect point of failure: Device does not support required Android extension: VK_KHR_external_semaphore_fd
E/flutter ( 7166): [ERROR:flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc(310)] Break on 'ImpellerValidationBreak' to inspect point of failure: Device not suitable since required extensions are not supported.
E/flutter ( 7166): [ERROR:flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc(393)] Break on 'ImpellerValidationBreak' to inspect point of failure: Device doesn't support the required queues.
E/flutter ( 7166): [ERROR:flutter/impeller/renderer/backend/vulkan/context_vk.cc(278)] Break on 'ImpellerValidationBreak' to inspect point of failure: No valid Vulkan device found.
I/flutter ( 7166): [IMPORTANT:flutter/shell/platform/android/android_context_vk_impeller.cc(60)] Using the Impeller rendering backend (Vulkan).
F/flutter ( 7166): [FATAL:flutter/shell/platform/android/platform_view_android.cc(127)] Check failed: android_context_->IsValid(). Could not create surface from invalid Android context.
I/flutter ( 7166): The Dart VM service is listening on http://127.0.0.1:38091/B3KmzM-kJeY=/
Application failed to start on attempt: 2
Installing build/app/outputs/flutter-apk/app-debug.apk...          573ms
E/flutter ( 7243): [ERROR:flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc(285)] Break on 'ImpellerValidationBreak' to inspect point of failure: Device does not support required Android extension: VK_KHR_external_semaphore_fd
E/flutter ( 7243): [ERROR:flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc(310)] Break on 'ImpellerValidationBreak' to inspect point of failure: Device not suitable since required extensions are not supported.
E/flutter ( 7243): [ERROR:flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc(393)] Break on 'ImpellerValidationBreak' to inspect point of failure: Device doesn't support the required queues.
E/flutter ( 7243): [ERROR:flutter/impeller/renderer/backend/vulkan/context_vk.cc(278)] Break on 'ImpellerValidationBreak' to inspect point of failure: No valid Vulkan device found.
I/flutter ( 7243): [IMPORTANT:flutter/shell/platform/android/android_context_vk_impeller.cc(60)] Using the Impeller rendering backend (Vulkan).
F/flutter ( 7243): [FATAL:flutter/shell/platform/android/platform_view_android.cc(127)] Check failed: android_context_->IsValid(). Could not create surface from invalid Android context.
I/flutter ( 7243): The Dart VM service is listening on http://127.0.0.1:35637/2EA1B4yoH2U=/
Application failed to start on attempt: 3
Application failed to start. Will not run test. Quitting.

It would be nice not to restart twice, but we have no way of distinguishing "this is a failure that cannot be recovered" and I'm not sure we have a good way (F/flutter?). I'd be open to another PR that improves this, feedback welcome.

@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jan 24, 2025
jonahwilliams
jonahwilliams approved these changes Jan 24, 2025
View reviewed changes
Copy link
Member

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

RSLGTM

@matanlurey
Copy link
Contributor Author

matanlurey commented Jan 24, 2025 •
edited
Loading

RSLGTM

Any thoughts about the false positives?

I imagine with this setup, if I wrote:

import 'package:flutter/material.dart';

void main() {
  print('Error: Jonah is a bad');
}

... then we'd report it as a launch failure, which probably is not right? Maybe it's OK?

jonahwilliams
jonahwilliams reviewed Jan 24, 2025
View reviewed changes
packages/flutter_tools/lib/src/android/android_device.dart
// For apps that use the VM service protocol, we *do* have a signal that the app
// appears to have started, and can wait until the VM service is discovered, and
// then check "startupLogs" to see if an error was detected so far.
if (checkErrorFromStartupLogs() case final String error) {
Copy link
Member

Choose a reason for hiding this comment

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

We could concievably get a successfull connection but still see an error log right?

Could we ignore this error if we have a vm service URI?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reverse is also true though, we get a VM service URI when we otherwise don't have a successful connection :-/ (fatal Impeller)

Copy link
Member

Choose a reason for hiding this comment

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

Ah, so we connect but then get the connection is closed error. Yeah, I dunno. Maybe you could do something like set an error bit and if an http exception is encountered then assume the app crashed. But if that is happening way later its not going to be possible to track.

I don't think there is going to be a foolproof method here. Unless we want to add some magic string to these known errors that would be less likely to colide than "Error"

@matanlurey
Copy link
Contributor Author

I've been convinced to mark this blocked/paused until we can filter log messages and generally have more stable signals.

@matanlurey matanlurey added the blocked Issue is blocked by another issue label Jan 24, 2025
@matanlurey matanlurey marked this pull request as draft January 24, 2025 21:20
@flutter-dashboard
Copy link

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@matanlurey matanlurey mentioned this pull request Feb 13, 2025
Open
@matanlurey matanlurey closed this Mar 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonahwilliams jonahwilliams jonahwilliams approved these changes

@gmackall gmackall Awaiting requested review from gmackall

Assignees
No one assigned
Labels
blocked Issue is blocked by another issue 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.

flutter drive Impeller startup crashes appear as VM HTTP exceptions
2 participants
@matanlurey @jonahwilliams