invertase / react-native-firebase

πŸ”₯ A well-tested feature-rich modular Firebase implementation for React Native. Supports both iOS & Android platforms for all Firebase services.
https://rnfirebase.io
Other
11.7k stars 2.22k forks source link

πŸ”₯[πŸ›] App crashed when tapping on notification banner when app is at background #4232

Closed TommyLeong closed 4 years ago

TommyLeong commented 4 years ago

Issue

  1. Pushed remote notification from Firebase Portal
  2. Mobile received (app at background)
  3. Tap on the notification
  4. App crashed!
  5. Tested with Physical Device on iOS 12.1.2
Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '-[UIWindow userNotificationCenter:didReceiveNotificationResponse:withCompletionHandler:]: unrecognized selector sent to instance 0x11be51e40'

So I've tried to trace the crash, apparently the crash happens after Line 94. image

What I've tried:

  1. Comment out the method didReceiveNotificationResponse at AppDelegate.m, the app stop crashing. Well, this is not the solution because it simply just runs Line 96 completionHandler() in the screenshot.

  2. Upgraded lib to "@react-native-firebase/messaging": "^7.8.4" (it was 7.6.1 before), and now both the method willPresentNotification and didReceiveNotificationResponse crashed. (My notification does not show up at foreground as well)

  3. Tried to remove node_modules, Pods and Podfile.lock then perform yarn install & pod install

  4. Removing the pod 'Firebase/Messaging' from Podfile, still the same result. Crash for both method willPresentNotification and didReceiveNotificationResponse

Information you might be interested to find out:

  1. My project has iOS 11 as target level

Project Files

Javascript

Click To Expand

#### `package.json`: ```json # N/A ``` #### `firebase.json` for react-native-firebase v6: ```json # N/A ```

iOS

Click To Expand

#### `ios/Podfile`: - [ ] I'm not using Pods - [x] I'm using Pods and my Podfile looks like: ```ruby # Uncomment the next line to define a global platform for your project platform :ios, '11.0' require_relative '../node_modules/@react-native-community/cli-platform-ios/native_modules' source 'https://cdn.cocoapods.org/' target 'projectName' do # Comment the next line if you don't want to use dynamic frameworks use_frameworks! # use_frameworks is needed for xxx Framework # https://github.com/invertase/react-native-firebase/issues/3253#issuecomment-607782071 $RNFirebaseAsStaticFramework = true pod 'Firebase/Messaging' # Pods for projectName pod 'FBLazyVector', :path => "../node_modules/react-native/Libraries/FBLazyVector" pod 'FBReactNativeSpec', :path => "../node_modules/react-native/Libraries/FBReactNativeSpec" pod 'RCTRequired', :path => "../node_modules/react-native/Libraries/RCTRequired" pod 'RCTTypeSafety', :path => "../node_modules/react-native/Libraries/TypeSafety" pod 'React', :path => '../node_modules/react-native/' pod 'React-Core', :path => '../node_modules/react-native/' pod 'React-CoreModules', :path => '../node_modules/react-native/React/CoreModules' pod 'React-Core/DevSupport', :path => '../node_modules/react-native/' pod 'React-RCTActionSheet', :path => '../node_modules/react-native/Libraries/ActionSheetIOS' pod 'React-RCTAnimation', :path => '../node_modules/react-native/Libraries/NativeAnimation' pod 'React-RCTBlob', :path => '../node_modules/react-native/Libraries/Blob' pod 'React-RCTImage', :path => '../node_modules/react-native/Libraries/Image' pod 'React-RCTLinking', :path => '../node_modules/react-native/Libraries/LinkingIOS' pod 'React-RCTNetwork', :path => '../node_modules/react-native/Libraries/Network' pod 'React-RCTSettings', :path => '../node_modules/react-native/Libraries/Settings' pod 'React-RCTText', :path => '../node_modules/react-native/Libraries/Text' pod 'React-RCTVibration', :path => '../node_modules/react-native/Libraries/Vibration' pod 'React-Core/RCTWebSocket', :path => '../node_modules/react-native/' pod 'React-cxxreact', :path => '../node_modules/react-native/ReactCommon/cxxreact' pod 'React-jsi', :path => '../node_modules/react-native/ReactCommon/jsi' pod 'React-jsiexecutor', :path => '../node_modules/react-native/ReactCommon/jsiexecutor' pod 'React-jsinspector', :path => '../node_modules/react-native/ReactCommon/jsinspector' pod 'ReactCommon/jscallinvoker', :path => '../node_modules/react-native/ReactCommon' pod 'ReactCommon/turbomodule/core', :path => '../node_modules/react-native/ReactCommon' pod 'Yoga', :path => '../node_modules/react-native/ReactCommon/yoga' pod 'DoubleConversion', :podspec => '../node_modules/react-native/third-party-podspecs/DoubleConversion.podspec' pod 'glog', :podspec => '../node_modules/react-native/third-party-podspecs/glog.podspec' pod 'Folly', :podspec => '../node_modules/react-native/third-party-podspecs/Folly.podspec' pod 'RNFBApp', :path => '../node_modules/@react-native-firebase/app' pod 'RNFBAnalytics', :path => '../node_modules/@react-native-firebase/analytics' pod 'RNFBCrashlytics', :path => '../node_modules/@react-native-firebase/crashlytics' pod 'React-RCTPushNotification', :path => '../node_modules/react-native/Libraries/PushNotificationIOS' target 'projectName-tvOSTests' do inherit! :search_paths end target 'projectNameTests' do inherit! :search_paths end use_native_modules! end target 'projectName-Shared' do # Comment the next line if you don't want to use dynamic frameworks use_frameworks! end target 'projectName-tvOS' do # Comment the next line if you don't want to use dynamic frameworks use_frameworks! end pre_install do |installer| installer.pod_targets.each do |pod| # This is needed for RNPermissions lib when we use "use_frameworks" in Podfile if pod.name.eql?('RNPermissions') || pod.name.start_with?('Permission-') def pod.build_type; # Uncomment one line depending on your CocoaPods version Pod::BuildType.static_library # >= 1.9 # Pod::Target::BuildType.static_library # < 1.9 end end end end post_install do |installer| installer.pods_project.targets.each do |target| # # Following action is to remove "React" from project's target, otherwise project will have duplicate symbol issues if target.name == "React" target.remove_from_project end end end ``` #### `AppDelegate.m`: ```objc /** * Copyright (c) 2015-present, Facebook, Inc. * All rights reserved. * * This source code is licensed under the BSD-style license found in the * LICENSE file in the root directory of this source tree. An additional grant * of patent rights can be found in the PATENTS file in the same directory. */ #import "AppDelegate.h" #import #import #import #import #import @import Firebase; /** START: Follow Vizury Documentation */ #import "RnVizuryLogger.h" #if defined(__IPHONE_10_0) && __IPHONE_OS_VERSION_MAX_ALLOWED >= __IPHONE_10_0 @import UserNotifications; #endif // Implement UNUserNotificationCenterDelegate to receive display notification via APNS for devices running iOS 10 and above. #if defined(__IPHONE_10_0) && __IPHONE_OS_VERSION_MAX_ALLOWED >= __IPHONE_10_0 @interface AppDelegate () @end #endif // Copied from Apple's header in case it is missing in some cases (e.g. pre-Xcode 8 builds). #ifndef NSFoundationVersionNumber_iOS_9_x_Max #define NSFoundationVersionNumber_iOS_9_x_Max 1299 #endif /** END: Follow Vizury Documentation */ #define SYSTEM_VERSION_GREATER_THAN_OR_EQUAL_TO(v) ([[[UIDevice currentDevice] systemVersion] compare:v options:NSNumericSearch] != NSOrderedAscending) @implementation AppDelegate - (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions { // NSURL *jsCodeLocation; // [Fabric with:@[[Crashlytics class]]]; RCTBridge *bridge = [[RCTBridge alloc] initWithDelegate:self launchOptions:launchOptions]; RCTRootView *rootView = [[RCTRootView alloc] initWithBridge:bridge moduleName:@"projectName" initialProperties:nil]; if (sizeof(void*) == 4) { // Executing in a 32-bit environment NSLog(@"I'm 32bit"); } else if (sizeof(void*) == 8) { NSLog(@"I'm 64bit"); // Executing in a 64-bit environment } // jsCodeLocation = [[RCTBundleURLProvider sharedSettings] jsBundleURLForBundleRoot:@"index" fallbackResource:nil]; if ([FIRApp defaultApp] == nil) { [FIRApp configure]; } self.window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds]; UIViewController *rootViewController = [UIViewController new]; rootViewController.view = rootView; self.window.rootViewController = rootViewController; [self.window makeKeyAndVisible]; [[FBSDKApplicationDelegate sharedInstance] application:application didFinishLaunchingWithOptions:launchOptions]; [RnVizuryLogger initalizationVizury:application packageId:@"400" serverURL:@"https://www.vizury.com/analyze/analyze.php" cachingEnabled:true FCMEnabled:true]; // Define UNUserNotificationCenter // UNUserNotificationCenter *center = [UNUserNotificationCenter currentNotificationCenter]; // center.delegate = self; // iOS 10 or later #if defined(__IPHONE_10_0) && __IPHONE_OS_VERSION_MAX_ALLOWED >= __IPHONE_10_0 // For iOS 10 display notification (sent via APNS) [UNUserNotificationCenter currentNotificationCenter].delegate = self; UNAuthorizationOptions authOptions = UNAuthorizationOptionAlert | UNAuthorizationOptionSound | UNAuthorizationOptionBadge; [[UNUserNotificationCenter currentNotificationCenter] requestAuthorizationWithOptions:authOptions completionHandler:^(BOOL granted, NSError * _Nullable error) { }]; #endif [[UIApplication sharedApplication] registerForRemoteNotifications]; return YES; } - (NSURL *)sourceURLForBridge:(RCTBridge *)bridge { #if DEBUG return [[RCTBundleURLProvider sharedSettings] jsBundleURLForBundleRoot:@"index" fallbackResource:nil]; #else return [[NSBundle mainBundle] URLForResource:@"main" withExtension:@"jsbundle"]; #endif } - (BOOL)application:(UIApplication *)application openURL:(NSURL *)url options:(NSDictionary *)options { BOOL handled = [[FBSDKApplicationDelegate sharedInstance] application:application openURL:url sourceApplication:options[UIApplicationOpenURLOptionsSourceApplicationKey] annotation:options[UIApplicationOpenURLOptionsAnnotationKey] ]; // Add any custom logic here. NSString *urlString = url.absoluteString; if([urlString rangeOfString:@"fb683835668673241://"].location == NSNotFound){ return [RCTLinkingManager application:application openURL:url options:options]; }else{ return handled; } } //// Called when a notification is delivered to a foreground app. //// Handle incoming notification messages while app is in the foreground. //-(void)userNotificationCenter:(UNUserNotificationCenter *)center willPresentNotification:(UNNotification *)notification withCompletionHandler:(void (^)(UNNotificationPresentationOptions options))completionHandler //{ //// NSDictionary *userInfo = notification.request.content.userInfo; //// NSLog(@"%@", userInfo); //// completionHandler(UNAuthorizationOptionSound | UNAuthorizationOptionAlert | UNAuthorizationOptionBadge); // // NSDictionary *userInfo = notification.request.content.userInfo; // NSLog(@"%@", userInfo); // // Change this to your preferred presentation option // completionHandler(UNAuthorizationOptionSound | UNAuthorizationOptionAlert | UNAuthorizationOptionBadge); //// completionHandler(UNNotificationPresentationOptionNone); //} /** VIZURY START: Anything below is for Vizury Purpose */ // Required to register for notifications - (void)application:(UIApplication *)application didRegisterUserNotificationSettings:(UIUserNotificationSettings *)notificationSettings { [application registerForRemoteNotifications]; // [RNCPushNotificationIOS didRegisterUserNotificationSettings:notificationSettings]; } // Required for the register event. - (void)application:(UIApplication *)application didRegisterForRemoteNotificationsWithDeviceToken:(NSData *)deviceToken { NSString *token = [[deviceToken description] stringByTrimmingCharactersInSet: [NSCharacterSet characterSetWithCharactersInString:@"<>"]]; token = [token stringByReplacingOccurrencesOfString:@" " withString:@""]; NSLog(@"token %@",token); [RnVizuryLogger passAPNSToken:deviceToken]; // [RNCPushNotificationIOS didRegisterForRemoteNotificationsWithDeviceToken:deviceToken]; } // Required for the notification event. You must call the completion handler after handling the remote notification. - (void)application:(UIApplication *)application didReceiveRemoteNotification:(NSDictionary *)userInfo fetchCompletionHandler:(void (^)(UIBackgroundFetchResult))completionHandler { [RnVizuryLogger didReceiveRemoteNotificationInApplication:application withUserInfo:userInfo]; if(application.applicationState == UIApplicationStateInactive) { NSLog(@"Appilication Inactive - the user has tapped in the notification when app was closed or in background"); [self customPushHandler:userInfo]; } // [RNCPushNotificationIOS didReceiveRemoteNotification:userInfo fetchCompletionHandler:completionHandler]; } - (void) customPushHandler:(NSDictionary *)notification { if (notification !=nil && [notification objectForKey:@"deeplink"] != nil) { NSString* deeplink = [notification objectForKey:@"deeplink"]; NSLog(@"%@",deeplink); // Here based on the deeplink you can open specific screens that's part of your app } } // Required for the registrationError event. - (void)application:(UIApplication *)application didFailToRegisterForRemoteNotificationsWithError:(NSError *)error { // [RNCPushNotificationIOS didFailToRegisterForRemoteNotificationsWithError:error]; } // IOS 10+ Required for localNotification event // Handle notification messages after display notification is tapped by the user. //- (void)userNotificationCenter:(UNUserNotificationCenter *)center //didReceiveNotificationResponse:(UNNotificationResponse *)response // withCompletionHandler:(void (^)(void))completionHandler //{ // NSDictionary *userInfo = response.notification.request.content.userInfo; // [RnVizuryLogger didReceiveResponseWithUserInfo:userInfo]; // // // Print full message. // NSLog(@"%@", userInfo); // completionHandler(); // //// [RNCPushNotificationIOS didReceiveNotificationResponse:response]; //// completionHandler(); //} //-(void)userNotificationCenter:(UNUserNotificationCenter *)center didReceiveNotificationResponse:(UNNotificationResponse *)response withCompletionHandler:(void (^)(void))completionHandler{ // NSDictionary *userInfo = response.notification.request.content.userInfo; //// [RnVizuryLogger didReceiveResponseWithUserInfo:userInfo]; // // // Print full message. // NSLog(@"%@", userInfo); // completionHandler(); // // // [RNCPushNotificationIOS didReceiveNotificationResponse:response]; // //} #if defined(__IPHONE_10_0) && __IPHONE_OS_VERSION_MAX_ALLOWED >= __IPHONE_10_0 // Handle incoming notification messages while app is in the foreground. - (void)userNotificationCenter:(UNUserNotificationCenter *)center willPresentNotification:(UNNotification *)notification withCompletionHandler:(void (^)(UNNotificationPresentationOptions))completionHandler { NSDictionary *userInfo = notification.request.content.userInfo; NSLog(@"%@", userInfo); // Change this to your preferred presentation option completionHandler(UNNotificationPresentationOptionNone); } // Handle notification messages after display notification is tapped by the user. - (void)userNotificationCenter:(UNUserNotificationCenter *)center didReceiveNotificationResponse:(UNNotificationResponse *)response withCompletionHandler:(void (^)(void))completionHandler { NSDictionary *userInfo = response.notification.request.content.userInfo; [RnVizuryLogger didReceiveResponseWithUserInfo:userInfo]; // Print full message. NSLog(@"%@", userInfo); completionHandler(); } #endif // IOS 4-10 Required for the localNotification event. - (void)application:(UIApplication *)application didReceiveLocalNotification:(UILocalNotification *)notification { // [[RNFirebaseNotifications instance] didReceiveLocalNotification:notification]; // [RNCPushNotificationIOS didReceiveLocalNotification:notification]; } /** VIZURY END: Anything above is for Vizury Purpose */ @end ```


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`: ```groovy // N/A ``` #### `android/app/build.gradle`: ```groovy // N/A ``` #### `android/settings.gradle`: ```groovy // N/A ``` #### `MainApplication.java`: ```java // N/A ``` #### `AndroidManifest.xml`: ```xml ```


Environment

Click To Expand

**`react-native info` output:** ``` System: OS: macOS 10.15.4 CPU: (4) x64 Intel(R) Core(TM) i5-6360U CPU @ 2.00GHz Memory: 98.04 MB / 8.00 GB Shell: 3.2.57 - /bin/bash Binaries: Node: 12.10.0 - /usr/local/bin/node Yarn: 1.19.0 - /usr/local/bin/yarn npm: 6.12.0 - /usr/local/bin/npm Watchman: 4.9.0 - /usr/local/bin/watchman SDKs: iOS SDK: Platforms: iOS 13.6, DriverKit 19.0, macOS 10.15, tvOS 13.4, watchOS 6.2 Android SDK: API Levels: 23, 28, 29 Build Tools: 28.0.3, 29.0.2, 29.0.3 System Images: android-21 | Google APIs ARM EABI v7a, android-21 | Google APIs Intel x86 Atom_64, android-23 | Google APIs ARM EABI v7a, android-23 | Google APIs Intel x86 Atom_64, android-24 | Google APIs Intel x86 Atom_64, android-26 | Google Play Intel x86 Atom Android NDK: 19.2.5345600 IDEs: Android Studio: 3.6 AI-192.7142.36.36.6308749 Xcode: 11.6/11E708 - /usr/bin/xcodebuild npmPackages: react: 16.9.0 => 16.9.0 react-native: 0.61.5 => 0.61.5 npmGlobalPackages: create-react-native-module: 0.19.0 react-native-cli: 2.0.1 ``` - **Platform that you're experiencing the issue on**: - [ x ] iOS - [ ] 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:** ``` "@react-native-firebase/analytics": "^7.3.1", "@react-native-firebase/app": "^8.2.0", "@react-native-firebase/crashlytics": "^8.1.2", "@react-native-firebase/messaging": "^7.8.4", ``` - **`Firebase` module(s) you're using that has the issue:** `@react-native-firebase/messaging` - **Are you using `TypeScript`?** `NO`


mikehardy commented 4 years ago

Interesting - we haven't had any other reports of this. I wonder if the most important part is the ios12 part, and everyone else is running on ios13? Have you looked at the underlying APIs we are using to see if they are not ios12-safe / introduced in ios13?

A link to the underlying source is more useful than an image:

https://github.com/invertase/react-native-firebase/blob/55cd752f6c1c1ab063e1cb9235d3657c5b2ae27f/packages/messaging/ios/RNFBMessaging/RNFBMessaging%2BUNUserNotificationCenter.m#L87-L100

For this reason - now I can click blame and see when that change happened:

https://github.com/invertase/react-native-firebase/blame/master/packages/messaging/ios/RNFBMessaging/RNFBMessaging%2BUNUserNotificationCenter.m#L87-L100

Perhaps something is wrong with this logic?

https://github.com/invertase/react-native-firebase/commit/a800cdbc81bfaeeaccf602aa62ca29d2fbf68c05#diff-2ebe543ff8c8e98b0599c69f717c51eb

messaging@6.5.0 appears to introduce the change you could try going to the last version before then (just as a test!) to see if we our expectations are correct about the lines of code we suspect might have a problem, and the time / code changes that introduced it

If the version prior to 6.5.0 does not crash, then each changed line needs investigation, to see if something was introduced that relies on an ios version higher than 12 without being adequately version protected

TommyLeong commented 4 years ago

@mikehardy .. Roger that, will append link to the source next round when issue report.

I'm a little confused with the versioning here, how should I get version prior messaging@6.5.0? The versioning with Firebase, RNFirebase & NPM/Yarn pkg is not so straight forward.

image

mikehardy commented 4 years ago

It is not easy no, and there are likely incompatibilities across the packages as occasionally they have cross-module dependent changes. I do not have a great answer for you. If it were me I might just try copy pasting in (or manually applying, directly in node-modules) the equivalent of the reverse diff of those lines - just rudely/crudely hacking that specific change out basically. Expecting that I would do a crappy job the first time or so and not compile but that I would eventually have a "fully up to date packages except that change removed" test build.

Alternatively you can look at the peer-deps on the test app from that time to see what they were and manually set versions to the set of dependencies from that time - I see these by git blaming back on that package.json

https://github.com/invertase/react-native-firebase/blame/a800cdbc81bfaeeaccf602aa62ca29d2fbf68c05/tests/package.json

TommyLeong commented 4 years ago

Can't agree more with your statement. I have hard time following the versioning, the only thing I do to dodge issues is to keep my lib up to date as much as possible. Lol

BINGO πŸ˜† @mikehardy. Guess what? Previous commit of the file (RNFBMessaging+UNUserNotificationCenter.m) works!

I have just tested with physical device on

Tested with the following steps

  1. With version "@react-native-firebase/messaging": "^7.8.4", update to previous commit on the affected method
  2. Deploy app (iPhone, iPad)
  3. Tap home button (leave the app in background) || Tested also force close app
  4. Push remote notification from Firebase Portal
  5. Tap on notification
  6. App opens without crash! (:
- (void)userNotificationCenter:(UNUserNotificationCenter *)center didReceiveNotificationResponse:(UNNotificationResponse *)response withCompletionHandler:(void (^)(void))completionHandler {
  NSDictionary *remoteNotification = response.notification.request.content.userInfo;
  if (remoteNotification[@"gcm.message_id"]) {
    NSDictionary *notificationDict = [RNFBMessagingSerializer remoteMessageUserInfoToDict:remoteNotification];
    [[RNFBRCTEventEmitter shared] sendEventWithName:@"messaging_notification_opened" body:notificationDict];
    _initialNotification = notificationDict;
    completionHandler();
  }
}
TommyLeong commented 4 years ago

Hmm..after some thoughts, Im not entirely sure whether this is the right approach to go with. As the commit was to fix quite some issues here. The goal to this PR seems to be keeping compatibility with other RN modules that set the delegate.

Any advise? @mikehardy Because this would means a permanent patch to the project.

Similarly, I have another similar issue with method willPresentNotification. App crash when push notification is pushed during the app is at foreground. (Still using "@react-native-firebase/messaging": "^7.8.4") So I did some hacky way to present the notification by changing Line 83 to completionHandler(UNAuthorizationOptionSound | UNAuthorizationOptionAlert | UNAuthorizationOptionBadge);

- (void)userNotificationCenter:(UNUserNotificationCenter *)center willPresentNotification:(UNNotification *)notification withCompletionHandler:(void (^)(UNNotificationPresentationOptions options))completionHandler {
  if (notification.request.content.userInfo[@"gcm.message_id"]) {
    NSDictionary *notificationDict = [RNFBMessagingSerializer notificationToDict:notification];

    // Don't send an event if contentAvailable is true - application:didReceiveRemoteNotification will send the event
    // for us, we don't want to duplicate them
    if (!notificationDict[@"contentAvailable"]) {
      [[RNFBRCTEventEmitter shared] sendEventWithName:@"messaging_message_received" body:notificationDict];
    }

    // TODO in a later version allow customising completion options in JS code
    completionHandler(UNNotificationPresentationOptionNone);
  }

  if (_originalDelegate != nil && originalDelegateRespondsTo.willPresentNotification) {
    [_originalDelegate userNotificationCenter:center willPresentNotification:notification withCompletionHandler:completionHandler];
  } else {
//    completionHandler(UNNotificationPresentationOptionNone);
    completionHandler(UNAuthorizationOptionSound | UNAuthorizationOptionAlert | UNAuthorizationOptionBadge);
  }
}
mikehardy commented 4 years ago

This was merely a test! We had a hypothesis (something about the specific lines changed there does not work for ios12) and we wanted to test it. It appears our hypothesis is correct! That's a great result. It means we know those lines cause a crash. Now the question is why? There needs to be a fix for the future so that ios12 can work with those lines.

My next hypothesis is that one of the iOS APIs used in those changed lines (which we just verified introduce the crash on ios12) is not present in ios12 or was changed between ios12 and ios >12 - and we did not realize it and used it incorrectly.

To verify this hypothesis you can look at the developer documentation on the Apple site and in the top right corner of those API reference pages they detail which versions of their operating systems first got the API

My guess is one of the APIs used will say "ios13" or similar and that will be the root cause.

assuming (dangerous! I could be guessing wrong) that one of the APIs is ios13+ the trick will be to put an is_available check in there for the minimum supported version on that API and figure out whether we need to do something different for ios12 etc etc

I'm guessing here (obviously - and trying to be very clear about that :-) ) - but that's my guess

Can you scan the API docs and see if the hypothesis is correct? I can assist with more time getting an actual code fix in but still have 10 other issues from other people to scan for the first time yet this morning

TommyLeong commented 4 years ago

Unfortunately 2nd hypothesis is not true because both the APIs didReceiveNotificationResponse and willPresentNotification is available from iOS 10.0+ onwards.

Do let me know if there's more information I could provide, will be happy to help! For the meantime, Im sticking to this "temporary patch (hopefully)" to keep the project rolling.

mikehardy commented 4 years ago

Dang! Hypothesis busted. But great to have a result either way. No reason your temporary patch can't roll for now and it will be temporary, we are not going to leave a crash bug in. As a follow-on to the previous incorrect hypothesis - why was it wrong? Can you reproduce this on ios13 devices or just ios12? That was the root of y previous hypothesis and I was making the assumption (always dangerous) that it was not showing up on ios13. Maybe I was off there.

I'm at a loss though. I just scanned the diff again and I can't see how if:

I mean, how is that possible? What is all this Vizury stuff doing? How can it possible be failing!?

TommyLeong commented 4 years ago

@mikehardy Let me come back to this issue again later, as I just found out there's a different for requesting permission before setting UNUserNotificationCenter currentNotificationCenter to self as pointed here in this post.

With this finding, I did testing with my current code I realized some of my delegate method wasn't being call actually. Example didReceiveNotificationResponse method.

Earlier, I did test to run without having any dependency (Vizury / react-native-push-notification) inside. Yet the app crashed as well. Maybe you could also share me something to your knowledge about requesting permission first before setting delegate (whether or not the sequence plays an important role). For my app, I tend to request the permission later once user logged in.

Meanwhile, I'll try to replicate the same in a new project.

mikehardy commented 4 years ago

I just fixed an issue in my work project a couple days ago where in most cases on ios, using react-native-permissions, I was not even requesting notification permission at all. I was definitely sending push notifications though (some visible with notification payload, some data only). No crashes. I fixed that bug and now in most cases I request notification permission but well after login as well, it is optional. Those users receive notifications (notification or data payload) without crash.

alancwoo commented 4 years ago

To add to this conversation, I'm having a somewhat similar problem where the app crashes when opened via a push when suspended (I believe, after being backgrounded for a few hours). It works perfectly fine if force closed, in foreground, and also when backgrounded for a shorter period and still in memory:

iOS 13.7
"react-native": "0.63.1"
"@react-native-firebase/app": "^8.3.1"
"@react-native-firebase/messaging": "^7.7.2"
"react-native-notifications": "^3.3.2"

My crash log is somewhat different:

Thread 6 Crashed:
0   libsystem_kernel.dylib          0x00000001901add88 __pthread_kill + 8
1   libsystem_pthread.dylib         0x00000001900c61e8 pthread_kill$VARIANT$mp + 136 (pthread.c:1458)
2   libsystem_c.dylib               0x0000000190019934 abort + 100 (abort.c:110)
3   libc++abi.dylib                 0x0000000190181cc0 abort_message + 128 (abort_message.cpp:76)
4   libc++abi.dylib                 0x0000000190173e10 demangling_terminate_handler() + 296 (cxa_default_handlers.cpp:65)
5   libobjc.A.dylib                 0x00000001900dae80 _objc_terminate() + 124 (objc-exception.mm:701)
6   libc++abi.dylib                 0x000000019018114c std::__terminate(void (*)()) + 16 (cxa_handlers.cpp:59)
7   libc++abi.dylib                 0x00000001901810e4 std::terminate() + 44 (cxa_handlers.cpp:88)
8   libdispatch.dylib               0x000000019007e538 _dispatch_client_callout + 36 (object.m:498)
9   libdispatch.dylib               0x000000019002a8a4 _dispatch_lane_serial_drain$VARIANT$mp + 608 (inline_internal.h:2484)
10  libdispatch.dylib               0x000000019002b294 _dispatch_lane_invoke$VARIANT$mp + 416 (queue.c:3863)
11  libdispatch.dylib               0x000000019003478c _dispatch_workloop_worker_thread + 588 (queue.c:6445)
12  libsystem_pthread.dylib         0x00000001900cfb74 _pthread_wqthread + 272 (pthread.c:2352)
13  libsystem_pthread.dylib         0x00000001900d2740 start_wqthread + 8

I'm simply requesting permissions inside a push handler file, so maybe it is also lifecycle related?

mikehardy commented 4 years ago

I don't see anything in that crash log that looks like our code, do you? So it seems unrelated, and not like anything we can actually affect, as our code doesn't appear to be there :thinking:

TommyLeong commented 4 years ago

Back to update, this issue is no longer valid. Apparently, it works fine in a fresh project. I managed to fixed my issue, the issue was with a 3rd party library I tried to integrate earlier.

Thanks @mikehardy for being helpful too! Appreciate (:

himanshug16 commented 6 months ago

hi @mikehardy, @TommyLeong

Having the same issue. Using version 19.2.2. I checked below code is already there in this version.

- (void)userNotificationCenter:(UNUserNotificationCenter *)center didReceiveNotificationResponse:(UNNotificationResponse *)response withCompletionHandler:(void (^)(void))completionHandler {
  NSDictionary *remoteNotification = response.notification.request.content.userInfo;
  if (remoteNotification[@"gcm.message_id"]) {
    NSDictionary *notificationDict = [RNFBMessagingSerializer remoteMessageUserInfoToDict:remoteNotification];
    [[RNFBRCTEventEmitter shared] sendEventWithName:@"messaging_notification_opened" body:notificationDict];
    _initialNotification = notificationDict;
    completionHandler();
  }
}

Still getting crash in iOS.

@mikehardy, Few days back someone raised the same issue but issue template was not followed in that issue so you rejected and I'm also not sure about template things so not creating another issue for this. I would really appreciate your help in this issue. @TommyLeong if you can help something it'll be really great as I'm stuck in this issue for past 2 weeks.

Thank you in advance.

mikehardy commented 6 months ago

I'm also not sure about template things

When you open an issue, there is a template that asks you to provide several files from the project. Do that, or there is not enough information to troubleshoot

And for iOS crashes specifically if you do not include the crash trace, it's unlikely to be a productive use of time. We need to see the crash stack trace to figure out what is crashing where

(some of these look reasonable as background if you have never gotten a crash trace before https://search.brave.com/search?q=how+to+get+ios+stacktrace&source=web - for messaging you need to use a real device so I would plug it in to my laptop then run the app from Xcode so the trace will be in Xcode console, or alternatively you can open Console.app, choose your device on the left side and watch the log that way)