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

🔥 Notification Object Missing in getInitialNotification() After Restart (Android) #5167

Closed
2 of 9 tasks
mcorner opened this issue Apr 14, 2021 · 2 comments · Fixed by #5181
Closed
2 of 9 tasks
Labels
help: needs-triage Issue needs additional investigation/triaging. platform: android plugin: messaging FCM only - ( messaging() ) - do not use for Notifications type: bug New bug report Workflow: Needs Review Pending feedback or review from a maintainer.

Comments

@mcorner
Copy link
Contributor

mcorner commented Apr 14, 2021

Issue

The issue is similar (or perhaps the same) as described here: #4634

However, there was either a regression, or it wasn't fully fixed.

Notification object is missing if you do this:

  1. Kill the app (this may not be necessary, but the app at least has to be in the background)
  2. Send a notification to the app, but don't open it.
  3. Start the app normally.
  4. Kill the app.
  5. Click on the notification to start the app

The notification object will be missing in getInitialNotification()

In a normal/correct case you get this:

{"notification":{"android":{},"body":"test2","title":"test"},"sentTime":1618360914831,"data":{},"from":"557990742577","messageId":"0:1618360914836382%26d15a5d26d15a5d","ttl":2419200,"collapseKey":"com.repromessagingbug"}

In the broken case described above, you get:

{"sentTime":0,"collapseKey":"com.repromessagingbug","ttl":2419200,"to":"0:1618361154602496%26d15a5d26d15a5d","data":{},"messageId":"0:1618361154602496%26d15a5d26d15a5d"}

Project Files

I put together a minimal reproduction here: https://github.com/mcorner/ReproMessagingBug/

Though it is just a stock RN app with enough code to get the FCM token and print the results of getInitialNotification()

Javascript

Click To Expand

https://github.com/mcorner/ReproMessagingBug/blob/master/App.js

package.json:

https://github.com/mcorner/ReproMessagingBug/blob/master/package.json

firebase.json for react-native-firebase v6:

# N/A

iOS

Click To Expand

N/A, Android issue.

ios/Podfile:

  • I'm not using Pods
  • I'm using Pods and my Podfile looks like:

N/A, Android issue.

AppDelegate.m:

N/A, Android issue.


Android

Click To Expand

Have you converted to AndroidX?

  • my application is an AndroidX application?
  • I am using android/gradle.settings jetifier=true for Android compatibility?
  • I am using the NPM package jetifier for react-native compatibility?

android/build.gradle:

// N/A

android/app/build.gradle:

https://github.com/mcorner/ReproMessagingBug/blob/master/android/app/build.gradle

android/settings.gradle:

https://github.com/mcorner/ReproMessagingBug/blob/master/android/settings.gradle

MainApplication.java:

https://github.com/mcorner/ReproMessagingBug/blob/master/android/app/src/main/java/com/repromessagingbug/MainApplication.java

AndroidManifest.xml:

https://github.com/mcorner/ReproMessagingBug/blob/master/android/app/src/main/AndroidManifest.xml


Environment

Click To Expand

react-native info output:

info Fetching system and libraries information...
System:
    OS: macOS 10.15.7
    CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
    Memory: 192.00 MB / 32.00 GB
    Shell: 5.7.1 - /bin/zsh
  Binaries:
    Node: 12.13.1 - ~/.nvm/versions/node/v12.13.1/bin/node
    Yarn: 1.22.5 - /usr/local/bin/yarn
    npm: 6.12.1 - ~/.nvm/versions/node/v12.13.1/bin/npm
    Watchman: 4.9.0 - /usr/local/bin/watchman
  Managers:
    CocoaPods: 1.10.1 - /usr/local/bin/pod
  SDKs:
    iOS SDK:
      Platforms: iOS 14.4, DriverKit 20.2, macOS 11.1, tvOS 14.3, watchOS 7.2
    Android SDK:
      API Levels: 23, 26, 27, 28, 29
      Build Tools: 28.0.3, 29.0.2, 30.0.0
      System Images: android-27 | Google Play Intel x86 Atom, android-29 | Google Play Intel x86 Atom
      Android NDK: Not Found
  IDEs:
    Android Studio: 4.0 AI-193.6911.18.40.6514223
    Xcode: 12.4/12D4e - /usr/bin/xcodebuild
  Languages:
    Java: 1.8.0_231 - /usr/bin/javac
  npmPackages:
    @react-native-community/cli: Not Found
    react: 17.0.1 => 17.0.1
    react-native: 0.64.0 => 0.64.0
    react-native-macos: Not Found
  npmGlobalPackages:
    *react-native*: Not Found
  • Platform that you're experiencing the issue on:
    • iOS
    • [x ] Android
    • iOS but have not tested behavior on Android
    • Android but have not tested behavior on iOS
    • Both
  • react-native-firebase version you're using that has this issue:
    • 11.2.0
  • Firebase module(s) you're using that has the issue:
    • Messaging
  • Are you using TypeScript?
    • N

@mcorner mcorner added help: needs-triage Issue needs additional investigation/triaging. type: bug New bug report labels Apr 14, 2021
@mikehardy
Copy link
Collaborator

Very interesting! The detailed steps and repro are very much appreciated! This is optional (of course, open source, etc...) but if you want to look at the PR that intended to do this, and reach into node_modules directly in the same area you will likely beat me to reproduction + fix, I have a couple other reproductions (one of which is a crash bug) so I'm not sure when I'll have a time slot for this. Labeling it for attention though of course

@mikehardy mikehardy added plugin: messaging FCM only - ( messaging() ) - do not use for Notifications Workflow: Needs Review Pending feedback or review from a maintainer. platform: android labels Apr 14, 2021
@mcorner
Copy link
Contributor Author

mcorner commented Apr 14, 2021

@mikehardy Thanks for the fast response. And many, many thanks for all of your hard work on this.

@dlindstrm wrote the original pull, so he may have thoughts.

I dug a bit into the code and I think I have a pretty good idea of what is wrong. I have an idea about how to fix it, but perhaps there is an easier way....

The message is serialized and saved in the store just fine, but when it is deserialized into a RemoteMessage:
https://github.com/invertase/react-native-firebase/blob/master/packages/messaging/android/src/main/java/io/invertase/firebase/messaging/ReactNativeFirebaseMessagingStoreImpl.java#L58
and

Only certain keys (TTL, data, etc.) are deserialized into the RemoteMessage object. Notification isn't one of them. There seems to be a good reason for this because the RemoteMessage builder doesn't appear to support setting the notification.

So unless I am missing an easier solution, popRemoteMessageFromMessagingStore and getFirebaseMessage shouldn't pass around RemoteMessage, but should instead pass ReadableMap, which is more flexible.

(I was at least able to confirm this generally works with a half broken solution).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help: needs-triage Issue needs additional investigation/triaging. platform: android plugin: messaging FCM only - ( messaging() ) - do not use for Notifications type: bug New bug report Workflow: Needs Review Pending feedback or review from a maintainer.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants