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

OneSignal seems to swizzle app delegate methods and didReceiveRemoteNotification is not called #1045

Closed
hyouuu opened this issue Feb 8, 2022 · 16 comments · Fixed by #1091
Closed

Comments

@hyouuu
Copy link

hyouuu commented Feb 8, 2022

Description:
Just updated OneSignal from 2.x to the latest 3.10.0 with SPM. In 2.x there was a OSHandleNotificationReceivedBlock that we can set to deal with received remote notification, but 3.x removed it and from the doc we should handle it in appDelegate's

func application(
        _ application: UIApplication,
        didReceiveRemoteNotification userInfo: [AnyHashable : Any],
        fetchCompletionHandler completionHandler: @escaping (UIBackgroundFetchResult) -> Void)

However, it is never called nor didRegisterForRemoteNotificationsWithDeviceToken or didFailToRegisterForRemoteNotificationsWithError. I turned OneSignal Verbose logging on, and when a silent notification is sent I got these stacktrace:

VERBOSE: onesignalUserNotificationCenter:willPresentNotification:withCompletionHandler: Fired! some message
VERBOSE: finishProcessingNotification: Fired!
VERBOSE: Notification display type: 7
VERBOSE: notificationReceived called! opened: NO
VERBOSE: callLegacyAppDeletegateSelector:withCompletionHandler: Fired!
VERBOSE: finishProcessingNotification: call completionHandler with options: 7
VERBOSE: oneSignalReceiveRemoteNotification:UserInfo:fetchCompletionHandler:
VERBOSE: notificationReceived called! opened: NO

So apparently OneSignal got the remote notification received trigger, but masked the original appDelegate. If in didFinishLaunchingWithOptions I explicitly set application.delegate = self other weird behaviors would happen.

Could you restore OSHandleNotificationReceivedBlock and make oneSignalReceiveRemoteNotification call it? Should be pretty straightforward and it would keep the 2.x behavior.

Environment
3.10.0 with SPM using the XCFramework version

Steps to Reproduce Issue:

  1. Install iOS SDK version 2.8.2 with Cocoapods into a project
  2. Initialize the SDK in didFinishLaunchingWithOptions:
  3. Attempt to receive a push notification
@jkasten2
Copy link
Member

jkasten2 commented Feb 8, 2022

@hyouuu Thanks for reporting, providing logging, and providing detailed behavior details.

From the log it looks like the OneSignal SDK is trying to call your didReceiveRemoteNotification: fetchCompletionHandler method based on this entry:

VERBOSE: callLegacyAppDeletegateSelector:withCompletionHandler: Fired!

In the OneSignal SDK source this comes from this line:
https://github.com/OneSignal/OneSignal-iOS-SDK/blob/3.10.0/iOS_SDK/OneSignalSDK/Source/UNUserNotificationCenter%2BOneSignal.m#L347

There are some checks below this, so it is possible something is falling here. Does your method fire if you tap on the notification?

On the didRegisterForRemoteNotificationsWithDeviceToken not firing, the OneSignal SDK should also be calling your method if you have this setup. This is done from within OneSignal's oneSignalDidRegisterForRemoteNotifications:deviceToken method at the line noted below:
https://github.com/OneSignal/OneSignal-iOS-SDK/blob/3.10.0/iOS_SDK/OneSignalSDK/Source/UIApplicationDelegate%2BOneSignal.m#L145

We can attempt to reproduce the issue with a new Swift project but I suspect a basic project like this won't reproduce the issue. If possible if you could provide an example project reproducing the issue and / or provide more details about how your AppDelegate is setup that will help find the root issue faster.

@hyouuu
Copy link
Author

hyouuu commented Feb 8, 2022

Thanks for the quick response @jkasten2 ! Sorry forgot to mention an important part, that my app is using the new SwiftUI app lifecycle and the app delegate is set up using @UIApplicationDelegateAdaptor(AppDelegate.self) var appDelegate like here: https://www.swiftbysundell.com/tips/using-an-app-delegate-with-swiftui-app-lifecycle/ - if you follow that pattern, I believe a basic app would make the issue appear.

So I guess the reason is because the callLegacySomething is trying to call UIApplication.shared, which is not where I define the UIApplicationDelegate methods (should be AppDelegate class in my case) - am I understanding it correctly?

What's the reason removing the OSHandleNotificationReceivedBlock in the first place? If it needs to be removed, could you expose an API to assign appDelegate to OneSignal?

@hyouuu
Copy link
Author

hyouuu commented Feb 8, 2022

So a sample app would look like this:

class AppDelegate: NSObject, UIApplicationDelegate {
    func application(_ application: UIApplication, didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey : Any]? = nil) -> Bool {
       
        // Setup OneSignal etc - this method will be called as App Delegate from SwiftUI lifecycle

        return true
    }
    
    func application(
        _ application: UIApplication,
        didReceiveRemoteNotification userInfo: [AnyHashable : Any],
        fetchCompletionHandler completionHandler: @escaping (UIBackgroundFetchResult) -> Void)
    {
        // This will not get called because OneSignal is not calling AppDelegate's methods
        print("appDidReceiveRemoteNotification")        
    }

    func application(_ application: UIApplication, didRegisterForRemoteNotificationsWithDeviceToken deviceToken: Data) {
        // This will not get called because OneSignal is not calling AppDelegate's methods
        print("appDidRegisterForRemoteNotif")
    }

    func application(_ application: UIApplication, didFailToRegisterForRemoteNotificationsWithError error: Error) {
        // This will not get called because OneSignal is not calling AppDelegate's methods
        print("appDidFailToRegisterForRemoteNotif !!!")
    }
}

@main
struct MyApp: App {
    @UIApplicationDelegateAdaptor(AppDelegate.self) var appDelegate

    var body: some Scene {
        WindowGroup {
            ContentView()
        }
    }
}

@hyouuu
Copy link
Author

hyouuu commented Feb 9, 2022

@jkasten2 Here are some insights on the SwiftUI appDelegate's relationship with UIApplication.shared.delegate: https://stackoverflow.com/questions/66156857/swiftui-2-accessing-appdelegate

@hyouuu
Copy link
Author

hyouuu commented Feb 11, 2022

@jkasten2 put up a possible fix #1048 please take a look. Also do you know how do I create the XCFramework locally to test it?

@hyouuu
Copy link
Author

hyouuu commented Feb 11, 2022

Also cc @JKalash & @Nightsd01 listed as authors in the XCFramework pod file. Thanks in advance!

@jkasten2
Copy link
Member

@hyouuu Thanks for sharing the code! I don't believe we have tested UIApplicationDelegateAdaptor, so there is a good chance this is the root of the issue. We will attempt to reproduce and report back.

@hyouuu
Copy link
Author

hyouuu commented Feb 13, 2022

That would be awesome thank you!

@bierzorutas
Copy link

Same problem here, updated from v2 in IOS to the last version 3.. and now didReceiveRemoteNotification:fetchCompletionHandler is not getting called at all, nor when app is in background either foreground.. Any clue ?

@hyouuu
Copy link
Author

hyouuu commented Feb 15, 2022

@bierzorutas besides this problem I also got the app freezing when receiving any notifications in v3, so what I ended up doing was to clone the repo, switch to the release-2.x.x branch and build XCFramework locally. You probably want to try the same as I don't see advantages of v3 but only unnecessary changes and bugs so far :(

@jkasten2
Copy link
Member

@hyouuu Could you provide more details on the freezing you are seeing? We would like to investigate and address this as well. Could you create another issue with those details so we can look into it?

@hyouuu
Copy link
Author

hyouuu commented Feb 15, 2022

Sorry don't have time right now to switch back to 3.x and reproduce the issues. But I believe I saw the verbose log saying the foreground notification will show and calling legacy selectors and then the app would freeze. Might be the SwiftUI app delegator causing an infinite loop or something. It never happens with 2.x tho

@OneSignal OneSignal deleted a comment Feb 18, 2022
@zac
Copy link

zac commented Mar 4, 2022

@jkasten2 Is there any update on when we might have a solution for this in the SDK? This same issue is affecting us as well and prevents us from acting on silent push notifications in our use-case.

Thanks 👍

@chris
Copy link

chris commented Mar 4, 2022

+1 this is a blocker for our app.

@emawby
Copy link
Contributor

emawby commented May 18, 2022

I reproduced this issue in a SwiftUI app and am looking into the PR provided I apologize for the delay

@jkasten2
Copy link
Member

@chris @zac @hyouuu @bierzorutas A fix for this was released in OneSignal 3.11.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants