wonderpush / wonderpush-ios-sdk

iOS SDK for WonderPush − This plugin makes it easy to set up WonderPush push notifications on your iOS apps for iPhone, iPad and Apple devices. High volume, fast delivery and full-featured starting €1/month.
https://docs.wonderpush.com/docs/ios-push-notifications-quickstart
Apache License 2.0
5 stars 8 forks source link

didRegisterForRemoteNotificationsWithDeviceToken Called Multiple Times Before Registration #11

Closed MikeDonahue closed 3 years ago

MikeDonahue commented 3 years ago

I have a typical setup in my AppDelegate that contains the WonderPush setup as recommended:

func application(_ application: UIApplication, willFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]? = nil) -> Bool {
    WonderPush.setClientId(Config.wonderPushClientID, secret: Config.wonderPushClientSecret)
    WonderPush.setupDelegate(for: application)
    WonderPush.setupDelegateForUserNotificationCenter()

    return true
}

func application(_ application: UIApplication, didRegisterForRemoteNotificationsWithDeviceToken deviceToken: Data) {
     if WonderPush.isSubscribedToNotifications() {
         // It even passes this check, before the user is even prompted

         // TODO: Here I need to do something with the installation ID

         // Show installation ID for debugging on an Ad Hoc build
         Alert.show(.info, message: "test \(WonderPush.installationId())")
     }
}

Along with a call to WonderPush.subscribeToNotifications() later in the app lifecycle.

On app start, however, I'm seeing that alert we show (as seen above) multiple times before the prompt to register is even shown to the user. I even created a counter and would just let the app sit there and the alert would keep counting up 4, 5, 6 times. Is there any reason this would happen? I saw the mention that Firebase method swizzling might affect these functions being called at all, so I followed Firebase's docs and add FirebaseAppDelegateProxyEnabled in the app’s Info.plist file and setting it to NO. Still saw multiple alerts shown in success right on app start, before the user is prompted, and also multiple times after closing/reopening the app

Would you recommend the alternative from your documentation by not calling setupDelegateForApplication? If you decide not to call setupDelegateForApplication, you must forward all these methods to the WonderPush object from your application delegate.?

Any insight would be appreciated.

Notes: • Initially found this issue because I was getting an immediate crash on startup accessing WonderPush.installationId() even though didRegisterForRemoteNotificationsWithDeviceToken should not have been called until the user agreed to use push.

• It seems to do it on the first install, then if you kill the app and reopen, it only shows up once (still, even though the user didn't accept push).

• If you do not kill the app, and just close/reopen it, it seems to be called numerous times as well.

• I do see the forwarding call from WPAppDelegate in the stack trace

0   <App Name Redacted> 0x0000000102576248 specialized AppDelegate.application(_:didRegisterForRemoteNotificationsWithDeviceToken:) + 778824 (AppDelegate.swift:106)
1   <App Name Redacted> 0x000000010257610c specialized AppDelegate.application(_:didRegisterForRemoteNotificationsWithDeviceToken:) + 778508 (AppDelegate.swift:106)
2   <App Name Redacted> 0x0000000102574410 @objc AppDelegate.application(_:didRegisterForRemoteNotificationsWithDeviceToken:) + 771088 (<compiler-generated>:0)
3   WonderPush  0x0000000104486d68 -[WPAppDelegate application:didRegisterForRemoteNotificationsWithDeviceToken:] + 109928 
ofavre commented 3 years ago

Hi @MikeDonahue ,

WonderPush refreshes the notification permission state and together ensures that the device token is up to date at multiple occasions. The didRegisterForRemoteNotificationsWithDeviceToken method of the AppDelegate is subsequently at multiple occasions. This method is programmed to be idempotent, so it's safe to be run multiple times.

Note that didRegisterForRemoteNotificationsWithDeviceToken does not depend on the user having accepted the notification permission, or even being prompted. The device token enables the server to push any information using silent push notifications together with the remote-notification background mode.

MikeDonahue commented 3 years ago

Thanks for the response @ofavre. So since in my implementation, we need the installationID, is it best to check if .deviceToken() is non-nil? The issue I was hitting was that accessing the .installationID() seemed to cause a crash. I was assuming this was because of the implicitly unwrapped optional returning nil (In the case it's not yet set up?). But I'm not sure if that is the case.

If it's intended that functions like pushToken(), installationId() all might return nil, they probably should be returned is Swift as optionals, instead of implicitly unwrapped optionals. This seems to be the root of my issue.

MikeDonahue commented 3 years ago

I submitted a pull request that fixes some of the warnings mentioned here. #12.

ofavre commented 3 years ago

Hi @MikeDonahue, Indeed these values can be nil before the SDK has gotten the chance to register with our servers. We'll take a look at your pull request. Thanks for it.