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.

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

[Fix] FCM register retrying and correct types of errors handling #1599

Merged
merged 2 commits into from
Jun 8, 2022

Conversation

jkasten2
Copy link
Member

@jkasten2 jkasten2 commented Jun 3, 2022

Description

One Line Summary

Fix not retrying to register for push when there is an FCM error. Also added additional handling for AUTHENTICATION_FAILED.

Details

Motivation

Some errors from FCM are temporary and we should retry them instead of waiting until the app is restarted. This retry logic is already in place but was broken. The fix and the additional AUTHENTICATION_FAILED will give better insight into the issue when we report them on the dashboard.

Scope

Only affects handling errors from FCM when registering for push.

Testing

Unit testing

No existing unit tests, adding tests would be fragile when FCM's API changes.

Manual testing

Tested on an Android 12 emulators having issues always throwing AUTHENTICATION_FAILED.

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
    • FCM - Registering for push token
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible.
  • I have reviewed this PR myself, ensuring it meets each checklist item

This change is Reviewable

When there is an FCM error, it always throws an ExecutionException type
instead of the root cause error due to the Task.await used. To fix this
we need to catch this and use task.getException() to get the root cause.

https://developers.google.com/android/guides/tasks
Since we moved to FCM's newer Task API we did not correctly unwrap the
nested root causes to parse the root error message. Correct this along
with some refactoring to make this easier to read and to support another
error type.

Also accounted for AUTHENTICATION_FAILED which we didn't handle before.
This can happen if you boot an Android Studio emulator for the first
time right after creating it and run your app. We should retry as it
only takes a short amount of time for the emulator to initialize with
Google's servers.
@jkasten2 jkasten2 changed the title WIP - [Fix] FCM register retrying and correct types of errors handling [Fix] FCM register retrying and correct types of errors handling Jun 3, 2022
@jkasten2 jkasten2 requested review from nan-li and emawby June 3, 2022 04:40
@jkasten2 jkasten2 merged commit da8c8d8 into main Jun 8, 2022
@jkasten2 jkasten2 deleted the fix/fcm_correct_types_of_errors_handling branch June 8, 2022 18:46
@jkasten2 jkasten2 mentioned this pull request Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants