-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
iOS background handler not always triggered #4104
Comments
This would go a long way to explaining some of the non-deterministic behavior some people have seen, especially I remember a couple redux-saga folks mentioning this. Great digging We'll see if we can make this less "hard-coded delay that should work" and more either configurable delay (not great but would be better) or "actually event based" (best option, most likely) Community contributions are driving the repo though, so if you (or anyone else reading this) has ideas, a PR directly submitted will be the fastest way to fix, as with any issue @compojoom you specifically could reach right in there and make it work for yourself now (so as not to be blocked) and persist that with patch-package while we work through the details |
Wow, this is a great find! Will help this repo a ton if it gets merged 🤞🏽 |
Hey Daniel! Was chatting about this internally and the way our event emitter works on iOS & Android is that the events are corked/pooled on native until JS tells native it's ready, https://github.com/invertase/react-native-firebase/blob/master/packages/app/ios/RNFBApp/RNFBRCTEventEmitter.m#L61 So only once JS has loaded does these events get uncorked and they flow through to JS, this happens here: https://github.com/invertase/react-native-firebase/blob/master/packages/app/lib/internal/RNFBNativeEventEmitter.js#L30 Perhaps whats happening here is that in that JS code we need to add the listener first before calling that ready method, right now it's telling native it's ready then adding the subscription after - and the event could be getting lost as the listener isn't registered yet Could you try swapping them around in the JS code? |
Modify it like this? addListener(eventType, listener, context) {
RNFBAppModule.eventsAddListener(eventType);
const listeners = super.addListener(`rnfb_${eventType}`, listener, context);
if (!this.ready) {
RNFBAppModule.eventsNotifyReady(true);
this.ready = true;
}
return listeners
} Not sure if that is what you want, but I tried it and it doesn't seem to work. I'm not receiving the message with that change. |
I'm attaching a patch that can be used with patch-package (for those that don't want to manually modify obj-c files). in this discussion another user confirmed that the modification to AppDelegate helped him. |
Perhaps we can do a temporary fix of this to increase the timing to 8s whilst a more permanent solution is being worked on that isn't time based. Could you send a PR for that Daniel and I'll get it merged and published today |
This should be viewed as a temporary fix. A lot of people are reporting that the background data messages on iOS are not being delivered. The app wakes up, but the background listener doesn’t receive the message. Some apps that have a lot of JS apparently need more than the 2s timeout to be ready. So what happens is - app is started, 2s elapse, RNFBMessaging sends the message to the js handler, but the JS handler is not ready yet and misses the message. We mitigate this by delaying the delivery of the message to the JS side. This solution however is not optimal becasue we are sending a completionHandler after about 25s which means that our JS code would have only 16 to do something with the data message. A correct fix would be to figure out how to dispatch the message once the JS side is actually ready. invertase#4104
|
Had same issue, this helped me also. But for me 4/5 seconds is enough and messages were handled. |
@davx1992 cross-linking you to this issue, @russellwheatley I think you were thinking about looking at this? I might be wrong - sorry if so - either way, some new findings in user testing documented here #3805 (comment)
I would like to keep this centralized - it is a tough issue and will require lots of detail, having it one spot will help |
Hello All, I think I found a solution for my problem. After a few hours of googling and learning Obj.c. found possible solution.
Only question left for me, is where is the best place to end bg task, when messaging_message_received_background or on when completion handler called. |
@davx1992 why is that necessary in the context of the initialization/handling infrastructure here: https://github.com/invertase/react-native-firebase/tree/master/packages/messaging/ios/RNFBMessaging and with reported user success in almost all situations? More specifically: what exact cases are not handled by existing code (with links to line numbers and what's missing) |
@mikehardy Will try to explain. So the initial problem which was reported in this issue was partially fixed by this WA - extending timeout. This WA fixed issue, that handler is not triggered when app was in quit state.
As I found the information in google, then the queue is not executed when app is in the background state. It means that when the message received, all sync code is being executed, but this async code is not executed. And this explains the behavior that the message which was received, is handled only when the app is resumed, or the next message received, then the queue executes the previous code. To fix this, I implemented a background task, which is registering a background task, and when complete handler executed, I clear background task. This is fixing an issue, that queue is not frozen, and is dispatch_after being executed.
Then when we are calling complete handler, I am clearing background task:
|
@davx1992 fascinating! Great explanation, thank you. If you could put this in a PR we could review and either ingest it (to solve yet another facet of this endless problem!) or re-shape it to solve the root cause you've found somehow / some way. |
@mikehardy will try to do this, found info on this. |
@davx1992 your PR looked great, passed review and I just merged, I'll get a release out shortly after our CI tests run through it to make sure it landed on the main branch successfully. Fantastic work - we are slowly - one by one - fixing all the corner cases that cause FCM receipt to fail. |
@mikehardy Great to hear that! Thanks! |
Here's what I posted on the other ones as I closed them, same applies here - we think everything can work now, and if there are still failures we need extra care to see an App.js that cleanly reproduces it: As part of the work on #4104 several PRs have been merged, including one just merged and released now as We believe FCM messages will be handled now:
We are no longer aware of cases that don't have known solutions. If you can reproduce a case after updating to the latest code and running |
@davx1992 @mikehardy in order to make this solution works, one should enable the "Background processing" capability in the app? |
It doesn't work for you? In my project, I have Background Processing. |
@acarpe there is never a time you will succeed without doing all the listed-as-required steps: https://rnfirebase.io/messaging/usage/ios-setup#enable-background-modes |
First things first... I'm not an iOS dev expert :) @davx1992 I'm trying to clean up my app code to isolate if that capability is required or not but it could take me some days :(, right now I have enabled it. |
It took me a moment to figure it out because I am also not a iOS dev expert - I was confusing which ability was which. There is a new "Background Tasks" API for iOS13+ that is used (among other places) by react-native-background-fetch optionally, but I think this is different. This explains all the background modes: I believe the call in the PR we just merged - https://developer.apple.com/documentation/uikit/uiapplication/1623031-beginbackgroundtaskwithexpiratio - available since iOS 4, requires no special permission to use, as opposed to the new full-featured Background Tasks API which does require permission @davx1992 did I get that right to your knowledge ? |
@davx1992 I cannot get this method to be called when sending data-only push to the iOS app, when its totally killed (swiped-up and killed). |
@nishanttatva as I see from above information mentioned by Mike then no. (I am also not an IOS expert). |
If you swipe the app away iOS believes you. You just said "go away". iOS will not wake the app again for data-only things. If you send a visible notification the underlying SDKs will post the notification to the notification center but you will not see your app wake up and you get no opportunity to run code again until the user interacts with the notification (which the iOS understands as "okay, the user wants to use the app again"). |
Tried it, doesn't change anything. And yes all other cases, as in app in foreground (onMessage is called), or background (setBackgroundMessageHandler is called) work as intended. @mikehardy According to the docs,
I've been debugging this since 2 days, at my wits end now. |
Quit != Killed/Swiped Quit is just that it went to the background and then the OS harvested the resources. iOS is aggressive about this, but will wake your app again. If the user kills or swipes the app though iOS takes the user at their "word" and your app is dead until interacted with again |
@mikehardy |
If I understand correctly Android does something similar but only with the "force close" button, or on new app install. Until the app runs again (which is a user interaction), you don't receive any broadcast intents. From a programmer perspective it could be perceived as a bummer but as a user it's fantastic, they should have the control. Options available as programmer are cloud message with notification payload (hope the user taps it!), be a VOIP app (I kid, but it is a path), hook to Motion or Location events (but you better have a reason! or you will be denied) and that's about it. Gotta craft an app the user is motivated to use |
@mikehardy i.e App is background, data-only push is sent, and app is not opened for some time. When app is opened, its crashed. Idk, if this is related to it. |
Post a stack trace. If it is the same, it is possibly related |
Just a heads-up on how I moved from |
Can you tell me if this is working or not and if yes so in which file should i put it. |
same issues with some iphone`s(iphone 12 work). Its iisues from firebase. messaging.setBackgroundMessageHandler not triggered when data-only |
I've spend more time debugging this than I'm comfortable admitting, but well that's life.
If you were to search this repo for "ios background" -> you'll find around 30 open issues. Which makes 30% of all open issues. To those of you who think that react-native-firebase doesn't work with ios background notifications - you are wrong. It works! I've been in that boat and I can tell you - it works. Make sure that you compose the right message, with the correct headers. If your app is still not woken up, then use the console.app on your Mac to find out if iOS is blocking the push notifications for some reason.
Putting those things aside. I could see that our app was being woken up, but the JS backgroundHandler was not being triggered. Initially I thought that we have a bug in our JS code as the moment I commented some lines out - I could see the handler working. After some time - I noticed that it's not always the same commented out lines that make it work. It was all kind of random. I'll comment a line out - and it would work. Then I'll re-enable this line and it will continue to work... So I thought that we must be having some race condition in our code that provokes this. But after few days I realized that the part that causes this is not in our JS code, but rather on the native side. More precisely here:
https://github.com/invertase/react-native-firebase/blob/master/packages/messaging/ios/RNFBMessaging/RNFBMessaging%2BAppDelegate.m#L122
In our app the background task actually needs to load the redux store and the redux-saga code. We are talking about a lot of imports. So it turned out that despite setting the listener as one of the first things in index.js we apparently need more than 2s. I increased this to 8s and I can confirm that we are now receiving the message all the time.
So with this knowledge I think that some of the users who have opened a "ios background data message" issue actually suffer from the above problem. The OS actually delivers the message to the app, but the native code is executed before the bundle is ready and they are left with no message.
Environment
Click To Expand
react-native info
output:System:
OS: macOS 10.15.6
CPU: (16) x64 Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
Memory: 247.13 MB / 32.00 GB
Shell: 5.7.1 - /bin/zsh
Binaries:
Node: 14.4.0 - /usr/local/bin/node
Yarn: 1.17.3 - /usr/local/bin/yarn
npm: 6.14.4 - /usr/local/bin/npm
Watchman: 4.9.0 - /usr/local/bin/watchman
Managers:
CocoaPods: 1.9.1 - /usr/local/bin/pod
SDKs:
iOS SDK:
Platforms: iOS 13.6, DriverKit 19.0, macOS 10.15, tvOS 13.4, watchOS 6.2
Android SDK:
API Levels: 28, 29
Build Tools: 28.0.3, 29.0.0, 29.0.2, 29.0.3
System Images: android-24 | Google Play Intel x86 Atom, android-28 | Intel x86 Atom_64, android-28 | Google Play Intel x86 Atom, android-29 | Google APIs Intel x86 Atom, android-29 | Google Play Intel x86 Atom
Android NDK: 19.0.5232133
IDEs:
Android Studio: 4.0 AI-193.6911.18.40.6514223
Xcode: 11.6/11E708 - /usr/bin/xcodebuild
Languages:
Java: 1.8.0_222 - /usr/bin/javac
Python: 2.7.16 - /usr/bin/python
npmPackages:
@react-native-community/cli: Not Found
react: 16.11.0 => 16.11.0
react-native: 0.62.2 => 0.62.2
Platform that you're experiencing the issue on:
react-native-firebase
version you're using that has this issue:"8.3.0",
Firebase
module(s) you're using that has the issue:Are you using
TypeScript
?The text was updated successfully, but these errors were encountered: