[quick_actions_android] Return null from getLaunchAction when no activity is attached - #12331
[quick_actions_android] Return null from getLaunchAction when no activity is attached#12331Sembauke wants to merge 4 commits into
Conversation
…vity is attached getLaunchAction threw quick_action_getlaunchaction_no_activity whenever the plugin had no attached activity, which made QuickActionsAndroid.initialize throw, since it calls getLaunchAction unconditionally. An engine can legitimately run without an activity, for example a cached engine warmed up by a background service, so callers had no way to avoid this other than catching the error code around initialize. The throw also provided no recovery value: initialize registers the Dart handler before calling getLaunchAction, and any action that arrives later is reported through the plugin's onNewIntent listener once an activity attaches. A warning is logged so a misconfigured launcher activity stays diagnosable. Fixes flutter/flutter#190348
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request modifies the Android quick_actions plugin to return null and log a message instead of throwing a FlutterError when getLaunchAction is called without an attached activity. It also adds a unit test to verify this behavior. The review feedback suggests downgrading the log level from warning to debug to prevent misleading logs during normal execution, and recommends using doReturn().when() instead of when().thenReturn() for stubbing the Mockito spy in the test.
|
@googlebot I signed it! |
Logs at debug level rather than warning, since running without an attached activity is an expected state and would otherwise be noisy on every headless engine start. Uses doReturn().when() for the spy stub so the real method is not invoked while stubbing.
Reverts the version bump and CHANGELOG entry, leaving the release decision to the package maintainers.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request modifies the Android QuickActions plugin to return null instead of throwing a FlutterError when the activity is null during getLaunchAction(), logging a debug message instead. A corresponding unit test was added to verify this behavior. The review feedback suggests changing the log level from debug (Log.d) to warning (Log.w) to ensure that a missing activity remains diagnosable in logcat.
| // up by a background service. Nothing launched the app in that case, so there is no launch | ||
| // action to report. Any action that arrives later is reported through the plugin's | ||
| // onNewIntent listener once an activity attaches. | ||
| Log.d(TAG, "There is no activity available when getting the launch action."); |
There was a problem hiding this comment.
The PR description mentions that this change logs a warning so a genuinely misconfigured launcher activity stays diagnosable in logcat. However, the implementation uses Log.d (debug level) instead of Log.w (warning level). Using Log.w ensures that this potential misconfiguration is logged as a warning, making it easier to diagnose in production/logcat.
| Log.d(TAG, "There is no activity available when getting the launch action."); | |
| Log.w(TAG, "There is no activity available when getting the launch action."); |
There was a problem hiding this comment.
Keeping Log.d here deliberately, and I have updated the PR description instead, which is where the inconsistency actually was.
The description previously said this logs a warning, which no longer matched the code after the earlier suggestion to move to Log.d. It now says debug level, so the two agree.
On the level itself: having no attached activity is an expected state for a cached engine, so it occurs on every headless start rather than only on misconfiguration. Warning level would be persistent noise during normal operation. A genuinely misconfigured launcher activity is a development time problem, where debug logging is visible.
|
Thank you for your contribution! Because of the volume of PRs we receive, we require that new contributors use our checklist to guide them through critical steps in creating a Flutter PR. This PR's description is using an alternate (possibly AI-generated) checklist that does not follow our process. Please edit the PR description to add the correct checklist, then ensure that you have completed all of the steps. Once you've done that, please mark the PR as ready for review. If you need help, consider asking for advice on the #hackers-new channel on Discord. |
There was a problem hiding this comment.
Code Review
This pull request modifies QuickActions.java to return null and log a debug message instead of throwing a FlutterError when there is no activity available during getLaunchAction(). Additionally, a unit test getLaunchAction_noActivity_returnsNull has been added to QuickActionsTest.java to verify this behavior. I have no feedback to provide.
This item is checked, but has not been completed. Please mark the PR as ready for review only when all the items are actually complete. |
Generated with update-release-info --version=minimal.
There was a problem hiding this comment.
Code Review
This pull request updates the quick_actions_android package to version 1.0.33. The changes modify getLaunchAction in QuickActions.java to log a debug message and return null instead of throwing a FlutterError when no activity is attached. Additionally, a unit test has been added to verify this behavior. There are no review comments, and I have no feedback to provide.
getLaunchActionthrewquick_action_getlaunchaction_no_activitywhen the plugin had no attached activity.QuickActionsAndroid.initializecallsgetLaunchActionunconditionally, soinitializethrew aPlatformExceptionthat callers could not prevent.An engine can legitimately run without an attached activity. A cached engine warmed up by a background service is the common case:
audio_servicesupplies a cached engine to its activity and also warms up that same engine fromAudioService.onCreate, somain()runs with no activity when a media button or Android Auto starts the service.This returns
nullin that case instead, and logs at debug level. The throw provided no recovery value:initializeregisters the Dart handler before it callsgetLaunchAction, so the handler is already installed when the exception is thrown.QuickActionsPlugin.onAttachedToActivitycallsonNewIntent(activity.getIntent()), which reports the shortcut throughAndroidQuickActionsFlutterApi.launchAction.nullis also already the return value for the other "cannot answer" case in this method, on the line directly above (if (!isVersionAllowed())).Verified on an Android 12 emulator by pointing an app at this branch and starting its audio service with no UI: no exception, and a shortcut tap afterwards still routed correctly. A cold start from a shortcut still returns the action, so the activity-present path is unchanged. The new unit test fails with
io.flutter.plugins.quickactions.FlutterErroragainst the unmodified file.Fixes flutter/flutter#190348
Pre-Review Checklist
[shared_preferences]///).Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2