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

Android: getInitialNotification/onNotificationOpenedApp not triggered for old notifications if app restarts #4052

Closed
reeq-dev opened this issue Aug 5, 2020 · 15 comments · Fixed by #4317
Labels
platform: android plugin: messaging FCM only - ( messaging() ) - do not use for Notifications

Comments

@reeq-dev
Copy link
Contributor

reeq-dev commented Aug 5, 2020

Issue

Before i was using firebase 5+ and everything was working just fine. But few months ago i updated firebase to 7.0.1 (it was latest version) and over time users have started to report issues that they can't open specific page from push notification. After investigation i found steps to reproduce this bug, here it is:

  1. Completely close the app;
  2. Send more than 1 notification (e.g 3, can be the same data or different - does not matter);
  3. Open app from notification (everything still fine, listeners are working);
  4. Again completely close the app (we still have 2 more notifications);
  5. Open app from any of left notifications (App should open) and here is a bug happen: any of listeners wont trigger even if you try to open them from background/foreground or quit state.

It looks like they become expired or something, since you already opened app from another notification. Can that be possible ?

I'll update issue with code snippets if necessary, but seems like the problem is not in the code.

Versions:

"react-native": "0.61.2",
"@react-native-firebase/messaging": "^7.0.1",
"@react-native-firebase/app": "^7.0.1",

@hackrx
Copy link

hackrx commented Aug 11, 2020

@noolie Hii can you please tell me how you are calling your listeners in your app because, on android, I am not receiving notification when app is in quit state. Please have a look at #4071

@Wedin
Copy link

Wedin commented Aug 18, 2020

I'm also experiencing this bug on Android, on iOS it is working as expected. I've upgraded to the latest version and it's still an issue.

Does anyone have an idea on how to debug this?

@reeq-dev
Copy link
Contributor Author

@noolie Hii can you please tell me how you are calling your listeners in your app because, on android, I am not receiving notification when app is in quit state. Please have a look at #4071

Just exactly how it explained in react-native-firebase docs

@Wedin
Copy link

Wedin commented Aug 18, 2020

I did some digging in getInitialNotification : I get a messageId as expected but the ReactNativeFirebaseMessagingReceiver.notifications is empty.

ie null here:

RemoteMessage remoteMessage = ReactNativeFirebaseMessagingReceiver.notifications.get(messageId);

@mikehardy
Copy link
Collaborator

@Wedin common cause, a splash screen intercepting (and not passing along) the Intent required to ferry the data through. react-native-splash-screen is a known failing, unmaintained culprit. react-native-boot-splash is a maintained package that works. You're piggy-backing on someone else's issue, so you haven't posted the information we request for troubleshooting, meaning this is just a guess

@Wedin
Copy link

Wedin commented Aug 19, 2020

Yeah sorry, let me add some more info! It's not related to any other package as far as I can see, as this can be reproduced in a clean react-native project with only react-native-firebase as deps. I've verified this here: https://github.com/Wedin/test-react-native-firebase-messaging.

Dependencies

package.json:

"@react-native-firebase/app": "^8.3.1",
"@react-native-firebase/messaging": "^7.7.1",
"react": "16.13.1",
"react-native": "0.63.2"

firebase.json for react-native-firebase v6:

# N/A

The code is copied from the documentation on how to use getInitialNotification. With those things added, I believe the bug should be come from this package, but Android development isn't something I know a lot about.

@mikehardy mikehardy added Workflow: Needs Review Pending feedback or review from a maintainer. plugin: messaging FCM only - ( messaging() ) - do not use for Notifications Version: >= 6 labels Aug 19, 2020
@alexwasner
Copy link

I believe I am having the same issue. I haven't installed any RN splash screen modules in my project. Easiest way for me to reproduce is by receiving a notification, opening app normally, close app, tap notification and see that getInitialNotification is null. iOS seems to work as intended.

@vannt1991
Copy link
Contributor

vannt1991 commented Aug 30, 2020

Yep, i have encountered the same issue at Android app, iOS app is working as expected, any solution for that? @Wedin @alexwasner ?

Step to reproduced:
1, Open app as normal and hide app to background.
2, App received firebase notification message.
3, Kill app from recent menu app. (At the first time notification maybe will be disappear in status bar, but in the seconds time notification will be keep stay in status bar on my phone).
4, Tap to notification item from status bar and BUM... getInitialNotification from RN code will be null.

As as know, ReactNativeFirebaseMessagingReceiver.notifications array will be keep all remote messages, but when we kill application from recent menu app, this array will be free because app process died.

@mikehardy
Copy link
Collaborator

@vannt1991 there is no one I'm aware of (community or Invertase) working on this at the moment. I'm not sure what a solution would look like but if the app goes away then I assume there will need to be some sort of storage used instead of an in-memory array. Thinking about that practically, are there any patterns to follow in the current codebase where the library uses app-local storage already? (prefs, async-storage, something else?) Because if there is an existing pattern, then I would just re-use that to store information allowing for old notifications to be handled well. If there is not, then it seems a small JSON in the app cache dir that is stored with current notification information whenever they come in, with notifications removed as they are handled/cleared would do the trick

@vannt1991
Copy link
Contributor

vannt1991 commented Aug 30, 2020

@mikehardy Thanks 👍 Yep, Pls do that assap, this issue is very common case and easy to reproduced with user. What we need todo:
1, transform Remote message to json and store to local storage at Receiver. (Maybe share preferences, by key is message id)
2, when getInitialNotification we can get� from ReactNativeFirebaseMessagingReceiver.notifications otherwise check more get from share preferences by message id and removed it if existed data to free memory.

Thanks for your effort 👍

@mikehardy
Copy link
Collaborator

I will not do this. You, or someone else will do this - for the avoidance of doubt. No one is working on this currently (not me), and we are waiting for a PR. Someone here needs to propose a PR

@vannt1991
Copy link
Contributor

#4203
@mikehardy Pls help me review this pull request, thanks 👍

@mikehardy
Copy link
Collaborator

Patch-package format patch bundle available here for the PR! https://github.com/invertase/react-native-firebase/suites/1137916996/artifacts/16295093 any independent confirmation that it works for you or not is very helpful 🙏😁

@Wedin
Copy link

Wedin commented Sep 4, 2020

I've tested the patch-package and I can no longer reproduce the issue 👍

@mikehardy
Copy link
Collaborator

Excellent! Thank you for testing that - it helps a lot

I've reviewed the PR and it needs a little bit of work to be ready but I believe the style in general of the PR can work. The problem you will experience is that the PR is persisting notification information in your app preferences area, which is great in order to reference them across restarts but there is no cleaning function so there may be unbounded growth in storage space consumed by preferences.

I have proposed a solution to that deficiency and I/we are waiting on response from @vannt1991

@mikehardy mikehardy changed the title Android: any of listeners do not trigger when open old notification. Android: getInitialNotification/onNotificationOpenedApp not triggered for old notifications if app restarts Sep 4, 2020
@mikehardy mikehardy removed the Workflow: Needs Review Pending feedback or review from a maintainer. label Nov 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: android plugin: messaging FCM only - ( messaging() ) - do not use for Notifications
Projects
None yet
7 participants