firebase-ios-sdk icon indicating copy to clipboard operation
firebase-ios-sdk copied to clipboard

Calling `FirebaseApp.configure()` bricks push notifications via no call to `didRegisterForRemoteNotificationsWithDeviceToken`

Open kgaidis opened this issue 5 years ago • 13 comments

Step 1: Describe your environment

  • Xcode version: 12.4
  • Firebase SDK version: 7.5.1
  • Installation method: Swift Package Manager
  • Firebase Component: Auth, FirestoreSwift-Beta, Functions, Crashlytics, Analytics

Screen Shot 2021-02-06 at 12 59 09 PM

Step 2: Describe the problem

I have a SwiftUI project with Firebase via Swift Package Manager. I only actively use Firestore ("Swift Beta") and Auth...I haven't setup anything with push notifications from Firebase (that I know of).

However, as I was setting up push notifications, I couldn't get didRegisterForRemoteNotificationsWithDeviceToken called or didFailToRegisterForRemoteNotificationsWithError.

After countless hours, I tried to see what would happen if I uncomment FirebaseApp.configure(). If I uncomment it, immediately everything starts working as expected.

I've seen some threads that FCM does some swizzling? Probably tries to create some auto-magic with push notifications? Could there be an issue where Firebase is overtaking functions when it does not need to? I can't imagine any other way this would break.

Steps to reproduce:

See code below and comment/uncomment FirebaseApp.configure().

Relevant Code:

struct GrowApp: App {
  @UIApplicationDelegateAdaptor(AppDelegate.self) var appDelegate
class AppDelegate: NSObject, UIApplicationDelegate {

  func application(
    _ application: UIApplication,
    didFinishLaunchingWithOptions
      launchOptions: [UIApplication.LaunchOptionsKey : Any]? = nil
  ) -> Bool {
    FirebaseApp.configure()

    UNUserNotificationCenter.current().requestAuthorization(
      options: [.alert, .badge]
    ) { (success, error) in
      if success {
        // `UNUserNotificationCenter` returns from a background thread
        DispatchQueue.main.async {
          // Register to get `deviceToken`
          UIApplication.shared.registerForRemoteNotifications()
        }
      } else if let error = error {
        print(error)
      }
    }
    return true
  }
  func application(
    _ application: UIApplication,
    didRegisterForRemoteNotificationsWithDeviceToken deviceToken: Data
  ) {
     // not caled
  }

  func application(
    _ application: UIApplication,
    didFailToRegisterForRemoteNotificationsWithError error: Error
  ) {
     // not called
  }

kgaidis avatar Feb 06 '21 18:02 kgaidis

Seems like I can replicate with a simple sample project. Here is a ZIP of it. It pretty much has the code I pasted above.

RandomPush.zip

Edit:

Likely code:

  • https://github.com/firebase/firebase-ios-sdk/blob/ae854d8357ac0d62d033c4c6516b0fc5c5501372/FirebaseAuth/Sources/Auth/FIRAuth.m#L484

PR:

  • https://github.com/firebase/firebase-ios-sdk/commit/76aa88085384aef43a41c3a2def71ad58cc3a9c9?short_path=a421372#diff-6bb6d1c46632fc66405a524071cc4baca5fc6a1a6c0eefef81d8c3e2c89cbc13R298-R321

kgaidis avatar Feb 06 '21 18:02 kgaidis

Adding key GoogleUtilitiesAppDelegateProxyEnabled value NO to the applications Info.plist fixes this issue.

Prints:

7.5.0 - [GoogleUtilities/AppDelegateSwizzler][I-SWZ001011] App Delegate Proxy is disabled.

Docs:

  • https://git.webteam.tools/isaev.ta/firebase-ios-sdk/tree/d686427cc006863a617ab0a9b031eeef90a0224c/GoogleUtilities/AppDelegateSwizzler#disabling-app-delegate-swizzling-by-app-developers

Of course, not sure what this breaks.

kgaidis avatar Feb 07 '21 16:02 kgaidis

Here is an idea that I'm not sure if its right, but might as well throw it out there...

Does it set the associated object on the original delegate?

  • https://git.webteam.tools/isaev.ta/firebase-ios-sdk/blob/d686427cc006863a617ab0a9b031eeef90a0224c/GoogleUtilities/AppDelegateSwizzler/GULAppDelegateSwizzler.m#L306

But then tries to get that associated object from GULAppDelegateSwizzler?

  • https://git.webteam.tools/isaev.ta/firebase-ios-sdk/blob/d686427cc006863a617ab0a9b031eeef90a0224c/GoogleUtilities/AppDelegateSwizzler/GULAppDelegateSwizzler.m#L825

Therefore it can never find/call the original implementation? (if so, this is a bug for multiple methods)

didRegisterForRemoteNotificationsIMP(self, methodSelector, application, deviceToken);

kgaidis avatar Feb 07 '21 17:02 kgaidis

@kgaidis Thanks for sharing your investigation and analysis.

@maksymmalyhin Would you comment on the swizzler behavior?

paulb777 avatar Feb 08 '21 15:02 paulb777

@kgaidis Sorry you are having issues. Thank you for your investigation.

As for the idea, from the first glance the code looks correct. In the context of the donor method self is the swizzled app delegate here, so the associated object is set and read on the same object (unless the app delegate is not replaced by another SDK). I'll take a closer look at the sample project you provided (appreciate it!) to figure out the reason of the issue. We've encountered issues related to UIApplication implementation details before, e.g cacheing the delegate methods availability, etc. We will keep investigation.

Also, we recognize that GULAppDelegateSwizzler is not the ideal solution to forward the app delegate events to Firebase SDKs, so we are considering an alternative approach to allow developers explicitly register an app delegate interceptor. This would would enable us to avoid dealing with swizzling and Objective-C runtime keeping Firebase integration simple.

In the meanwhile our recommendation for you is to keep the app delegate swizzling disabled by adding the key GoogleUtilitiesAppDelegateProxyEnabled value NO to the applications Info.plist as you do and follow steps for integration without the swizzling for each SDK you use, e.g. Firebase Messaging, Auth, etc.

maksymmalyhin avatar Feb 08 '21 16:02 maksymmalyhin

I'm building a new app and got stuck on this again for 1+ hour

What saved me was this StackOverflow comment:

  • https://stackoverflow.com/a/51518574/826435

...but then I realized I had encountered this before (🤦) so bumping to save future dev time.

On the bright side, I clearly keep coming back to Firebase for all the new apps, so thank you for the tools!

kgaidis avatar Jul 31 '22 19:07 kgaidis

Just wanting to let know that this issue persists in version 9.5.0.

HauntedSoul avatar Sep 06 '22 15:09 HauntedSoul

@HauntedSoul and @kgaidis I'm not able to reproduce this issue. Our latest Messaging SwiftUI test app works as expected. https://github.com/firebase/firebase-ios-sdk/tree/master/FirebaseMessaging/Apps/SwiftUISample

You do have to implement the delegate method to setup APNS token.

charlotteliang avatar Oct 04 '22 02:10 charlotteliang

Without investigating that much the issue for me is that with swizzling the delegate methods are not called back by Firebase and thus any code that needs to run there, for other notification services, does not get called.

In essence I was not able to fetch the APN token to configure other platforms unless I disable swizzling.

HauntedSoul avatar Oct 04 '22 09:10 HauntedSoul

@HauntedSoul For SwiftUI app, you will have to manually implement the delegate methods for APNS token. We are updating the documentations to make it more clear. It's not related to FCM swizzling, especially you are not using the SDK.

func application(_ application: UIApplication, didRegisterForRemoteNotificationsWithDeviceToken deviceToken: Data) { // Pass device token to auth Auth.auth().setAPNSToken(deviceToken, type: .prod)

// Further handling of the device token if needed by the app // ... } }

func application(_ application: UIApplication, didReceiveRemoteNotification notification: [AnyHashable : Any], fetchCompletionHandler completionHandler: @escaping (UIBackgroundFetchResult) -> Void) { if Auth.auth().canHandleNotification(notification) { completionHandler(.noData) return } // This notification is not auth related; it should be handled separately. }

charlotteliang avatar Oct 04 '22 21:10 charlotteliang

@kgaidis Sorry you are having issues. Thank you for your investigation.

As for the idea, from the first glance the code looks correct. In the context of the donor method self is the swizzled app delegate here, so the associated object is set and read on the same object (unless the app delegate is not replaced by another SDK). I'll take a closer look at the sample project you provided (appreciate it!) to figure out the reason of the issue. We've encountered issues related to UIApplication implementation details before, e.g cacheing the delegate methods availability, etc. We will keep investigation.

Also, we recognize that GULAppDelegateSwizzler is not the ideal solution to forward the app delegate events to Firebase SDKs, so we are considering an alternative approach to allow developers explicitly register an app delegate interceptor. This would would enable us to avoid dealing with swizzling and Objective-C runtime keeping Firebase integration simple.

In the meanwhile our recommendation for you is to keep the app delegate swizzling disabled by adding the key GoogleUtilitiesAppDelegateProxyEnabled value NO to the applications Info.plist as you do and follow steps for integration without the swizzling for each SDK you use, e.g. Firebase Messaging, Auth, etc.

As mentioned in the quoted message unless GoogleUtilitiesAppDelegateProxyEnabled value is NO the "func application(_ application: UIApplication, didRegisterForRemoteNotificationsWithDeviceToken deviceToken: Data)" is not called on the delegate. As I have not investigated myself I went with the reasons above blaming swizzling, whatever the reason is though this was still an issue for me on SDK 9.5.0. I have recently lost access to the affected project so I can't give much more information at the moment.

HauntedSoul avatar Oct 05 '22 09:10 HauntedSoul

@chliangGoogle

I'm not able to reproduce this issue

Below I attached a ZIP that shows a very easy, reproducible case.

See the AppDelegate.swift class that has some notes.

EasyTest.zip

Aside from running on device and setting up Firebase with Info.plist (+ making sure bundle id is correct), make sure that you are also linking Firebase Auth (this should already be done in the project that is zipped).

From a very simple search, we can see that FIRAuth implements didRegisterForRemoteNotificationsWithDeviceToken:

  • https://github.com/firebase/firebase-ios-sdk/search?q=didRegisterForRemoteNotificationsWithDeviceToken

There is some swizzling going on.

kgaidis avatar Oct 06 '22 01:10 kgaidis

Sorry for the late revert, @kgaidis. I tried reproducing your issue using the sample app you shared, but the didRegisterForRemoteNotificationsWithDeviceToken delegate was also not being called even if I commented out the FirebaseApp.configure() or remove Firebase Auth. I noticed that it only worked when I tried changing the UIApplication.shared.registerForRemoteNotifications() to application.registerForRemoteNotifications() indicated here. As mentioned above, it seems that the issue is not related to FCM swizzling, especially you're not using the Messaging SDK.

rizafran avatar Jan 04 '23 19:01 rizafran

Closing for staleness. Please comment or open a new issue to continue the discussion.

paulb777 avatar Jan 06 '24 16:01 paulb777