[question]: OneSignal 5.0 iOS PushSubscription.Id crash and PreventDefualt still displaying on iOS

mpiechocki commented 1 year ago

How can we help?

General question is: how do I initialise OneSignal Unity SDK properly on iOS, in version 5.0.0? I'm using OneSignal for pushes only. What I'm currently doing, is that after logging user in, I need to set the subscription id to our server, so that it can authorise the pushes properly. The initialization code is as follows:

OneSignal.Debug.LogLevel = LogLevel.Verbose;
OneSignal.Debug.AlertLevel = LogLevel.Fatal;

OneSignal.Initialize(appId);

OneSignal.Notifications.ForegroundWillDisplay += OnForegroundWillDisplay;
OneSignal.Notifications.Clicked += OnNotificationClicked;
OneSignal.User.PushSubscription.Changed += OnPushSubscriptionChanged;

if (!Settings.ShownPushNotificationsPrompt)
{
    await OneSignal.Notifications.RequestPermissionAsync(true);
    Settings.ShownPushNotificationsPrompt = true;
}

And then, when the session is opened, and I know the user, I call:

OneSignal.Login(user.Id);
UpdatePushToken(OneSignal.User.PushSubscription.Id);

The problem is that on Android, this code works just fine and pushes work as expected. But on iOS, this results in a runtime crash:

2023-08-20 11:44:40.508795+0200 Highrise[29784:1472087] VERBOSE: OneSignal.User login called with externalId: 64a410b70003f4277b2958df
2023-08-20 11:44:40.508948+0200 Highrise[29784:1472087] VERBOSE: OneSignalUserManager internal _login called with externalId: 64a410b70003f4277b2958df
2023-08-20 11:44:40.509024+0200 Highrise[29784:1472087] VERBOSE: OneSignalUserManager.createNewUser: not creating new user due to logging into the same user.)
Highrise(29784,0x104e3c580) malloc: *** error for object 0x283da69d1: pointer being freed was not allocated
Highrise(29784,0x104e3c580) malloc: *** set a breakpoint in malloc_error_break to debug

I tried to guard against nulls, but still, the result is the same. I tried to look into OneSignal objc code and found something that worries me a little bit:

as the propery name is id which is a keyword in objc, but I'm not actually sure it's the problem.

Can anybody point me to any direction on what might be an issue here? Thanks!

P.S. The problem we're trying to actually solve with upgrading to 5.0.0 is that we still didn't find a workaround for getting pushes in foreground bug (see #521) so if anyone has any workaround for this on OneSignal 4.8.3, I'd be happy to hear it :)

Code of Conduct

[X] I agree to follow this project's Code of Conduct

shepherd-l commented 1 year ago

Thanks for bringing this up!

This is a problem in 5.0.0 and has been fixed in the 5.0.1 Release.

Let us know if you have any questions or if the issue is still occurring

mpiechocki commented 1 year ago

Hey @shepherd-l 👋 I can confirm that this particular issue is not occurring anymore, thanks!

However, I still cannot find a way to properly disable pushes while the app is in foreground. After spending some time debugging the objc code, I found some issues that prevented the will display event to be stored, so I added those changes locally:

Initialize _willDisplayEvents dictionary properly in the constructor
(instancetype) init { if (self = [super init]) { [OneSignal.Notifications addPermissionObserver:self]; [OneSignal.Notifications addForegroundLifecycleListener:self]; [OneSignal.Notifications addClickListener:self];
```
_willDisplayEvents = [[NSMutableDictionary alloc] init];
```
}

return self; }
reorder the calls in onWillDisplayNotification so that the dictionary is populated before any delegate calls (as they might call PreventDefault which in turn will look for the events in the dictionary):
(void)onWillDisplayNotification:(OSNotificationWillDisplayEvent *)event { if (_willDisplayDelegate != nil) { _willDisplayEvents[event.notification.notificationId] = event;
```
NSString *stringNotification = [event.notification stringify];
_willDisplayDelegate([stringNotification UTF8String]);
```
} }

Then, in C# code, I just use:

OneSignal.Notifications.ForegroundWillDisplay -= OnForegroundWillDisplay;

private void OnForegroundWillDisplay(object sender, NotificationWillDisplayEventArgs e)
{
    e.PreventDefault();
}

but this still doesn't give me good results, as they push is just being delayed 25 seconds, which is the timeout period set somewhere I couldn't figure out where. Here's the log:

2023-08-24 11:09:54.981413+0200 Highrise[49015:7451445] VERBOSE: onesignalUserNotificationCenter:willPresentNotification:withCompletionHandler:
2023-08-24 11:09:54.982081+0200 Highrise[49015:7451445] VERBOSE: onesignalUserNotificationCenter:willPresentNotification:withCompletionHandler: Fired! @Dontgonear: dasd
>>> on foreground will display OneSignalSDK.iOS.Notifications.iOSNotificationsManager+InternalNotificationWillDisplayEventArgs
Highrise.PushNotifications.OneSignalProvider:OnForegroundWillDisplay(Object, NotificationWillDisplayEventArgs)

2023-08-24 11:09:54.992502+0200 Highrise[49015:7451445] VERBOSE: OSNotificationWillDisplayEvent.preventDefault called.
2023-08-24 11:10:19.984043+0200 Highrise[49015:7451445] WARNING: OSNotificationLifecycleListener:onWillDisplayNotification timed out. Display was not called within 25.000000 seconds. Continue with display notification: 0
2023-08-24 11:10:20.000817+0200 Highrise[49015:7451445] VERBOSE: finishProcessingNotification: Fired!
2023-08-24 11:10:20.000968+0200 Highrise[49015:7451445] VERBOSE: Notification display type: 0
2023-08-24 11:10:20.001127+0200 Highrise[49015:7451445] VERBOSE: notificationReceived called! opened: NO
2023-08-24 11:10:20.002269+0200 Highrise[49015:7451445] VERBOSE: finishProcessingNotification: call completionHandler with options: 0

Not sure where to find the timer that times out. Are there any plans to straightening this up, maybe? 🙏

shepherd-l commented 1 year ago

Thank you for the detailed message!

I agree with the changes you made in ObjectiveC++, we will be creating another release soon with the same changes

In C#, the ForegroundWillDisplay delegate should be added to call event.PreventDefault

So instead of

OneSignal.Notifications.ForegroundWillDisplay -= OnForegroundWillDisplay;

It should be:

OneSignal.Notifications.ForegroundWillDisplay += OnForegroundWillDisplay;

Let us know if that fixes the issue for you

mpiechocki commented 1 year ago

@shepherd-l yeah, sorry, I copied wrong part of the code. I use += to add the delegate :) Yet, it doesn't solve the timeout problem, and I think we can see that the PreventDefault is called correclty in the log:

OSNotificationWillDisplayEvent.preventDefault called.

shepherd-l commented 1 year ago

@mpiechocki

Is the notification still displaying in the foreground? If so, could you please provide your entire log

A notification with a display type of 0 means the notification will not be displayed. The 25 seconds timeout should always occur after PreventDefault. We wait to check if Display is called after PreventDefault to allow notifications to be modified and then displayed.

mpiechocki commented 1 year ago

Yes, it appears in foreground after this 25 second delay

mpiechocki commented 1 year ago

@shepherd-l the only thing I get in the logs is this:

2023-08-25 19:06:35.169549+0200 Highrise[52077:7991627] VERBOSE: onesignalUserNotificationCenter:willPresentNotification:withCompletionHandler:
2023-08-25 19:06:35.172147+0200 Highrise[52077:7991627] VERBOSE: onesignalUserNotificationCenter:willPresentNotification:withCompletionHandler: Fired! @dontgodontgo: go
2023-08-25 19:06:35.174242+0200 Highrise[52077:7991627] VERBOSE: OSNotificationWillDisplayEvent.preventDefault called.

<--- ~25s delay --->

2023-08-25 19:07:00.177327+0200 Highrise[52077:7991627] WARNING: OSNotificationLifecycleListener:onWillDisplayNotification timed out. Display was not called within 25.000000 seconds. Continue with display notification: 0
2023-08-25 19:07:00.190623+0200 Highrise[52077:7991627] VERBOSE: finishProcessingNotification: Fired!
2023-08-25 19:07:00.190950+0200 Highrise[52077:7991627] VERBOSE: Notification display type: 0
2023-08-25 19:07:00.191306+0200 Highrise[52077:7991627] VERBOSE: notificationReceived called! opened: NO
2023-08-25 19:07:00.192663+0200 Highrise[52077:7991627] VERBOSE: finishProcessingNotification: call completionHandler with options: 0

and I still get the notification in foreground after the delay

shepherd-l commented 1 year ago

@mpiechocki

Are you using any other SDKs in your project?

What version of Unity and Xcode are you using?

What devices and their iOS versions are you seeing this issue on?

I was unable to reproduce the issue myself - if you have time, could you try reproducing this issue in a new project and share the steps and project with us

mpiechocki commented 1 year ago

@shepherd-l I'm using Unity 2022.3.4f1, Xcode 14.3.1. I can repro it on both iPhone 7 Plus, iOS 15.7 and iPhone Xr, iOS 16.6. We're using multiple other SDKs as our project it pretty big.

shepherd-l commented 1 year ago

@mpiechocki Could you provide a list of the SDKs you're using?

mpiechocki commented 1 year ago

@shepherd-l hmm, I tried to create a vanilla project to reproduce the issue and noticed some weird behaviour:

I opened a vanilla project on phone A. Gave permissions for pushes etc. It just initializes Onesignal and logs into an existing user.
I messaged this user from phone B, which should cause a push notification on phone A.
I didn't get the notification phone A, but after some delay I got the notification on phone B.

Which kind of gives me the idea that something might be wrong with the configuration or subscriptions. I recently have been testing the pushes on those two phones many times, on many different app versions and such. What do you think?

shepherd-l commented 1 year ago

@mpiechocki

If the same user is logged into multiple devices then each device would have its own push subscription attached to the user. If the user X is logged into phone A and phone B, then user X would have 2 push subscriptions - Push Subscription A and Push Subscription B.

On the dashboard it would look something like this:

Device | Subscription ID |    OneSignal ID    |    External ID     | Push Token |
---------------------------------------------------------------------------------
PhoneA |  push_sub_id_A  | onesigal_id_user_x | external_id_user_x | push_token_A
PhoneB |  push_sub_id_B  | onesigal_id_user_x | external_id_user_x | push_token_B

Visualizing user x would be something be like:

User_X {
OneSignal ID: onesigal_id_user_x
External ID: external_id_user_x
Push Subscriptions: [{push_sub_id_A, push_token_A}, {push_sub_id_B, push_token_B}]
Emails: []
Sms: []
Aliases: []
Tags: []
}

In the SDK, calling OneSignal.User.PushSubscription will get the Push Subscription of the current device it is on. Sending a push notification to phone A would send a notification to Push Subscription A - the first row

How are you sending the notification? Is phone B also logged into the same user as phone A?

Let me know if I can clarify anything

mpiechocki commented 1 year ago

Thanks for thorough explanation. I'm logged on two different accounts while testing.

I guess my question would be: is it possible that one device would have 2 simultaneous subscription for one user? I'm sort of blind guessing now, but I'm really confused by the results I'm getting

shepherd-l commented 1 year ago

On OneSignal.Initialize("app_id") and when consent is given for push notifications, the SDK requests a push token for the device and creates a Push Subscription with it.

It is possible that we cannot get a token from APNS but we don't need one in order to create a Push Subscription. In that case we may receive it later and will update the Push Subscription with the token. A Push Subscription without a push token can still receive in app messages. Sorry for that mix up, I will remove it from my last post.

A device only has one unique Push Subscription in each OneSignal app that initializes.

If an end user uninstalls and reinstalls the app, opens the app and initializes OneSignal, a new push token will be requested and a new Push Subscription will be created for the app for the device.

Sending a notification to the old Push Subscription Id will not display notifications on the device because the Push Subscription has changed.

Glad I can help. I'm not sure if I answered your question, let me know if I missed anything

mpiechocki commented 1 year ago

Thanks @shepherd-l ! I'll be testing it out more on my end then. Any chance you release the patched version soon? We want to release the app soon so we are on tight schedule 😄

shepherd-l commented 1 year ago

Np! Thanks for bringing attention to these bugs and even diving in to find a fix.

We plan to release the patch early next week

mpiechocki commented 1 year ago

@shepherd-l I tried to move to native implementation of OneSignal on iOS (version 5.0.1) but... ended up with just the same results. I get to call preventDefault but after the timeout, the notification is shown anyways. After some digging, I found (in some Android documentation ) some info about the timeout and having to call "complete" for OneSignal not to take over and show the notification. Not sure how to do it as I can't find anything about it in the iOS docs, and still, I wanted to avoid the Notification Service Extension as we just wanted to use simple notifications.

mpiechocki commented 1 year ago

After some more testing, I figured out that if I spam push notifications while app is in foreground, then I'll get only the last push (after 25s delay). When app is in background, everything is fine, I get all the pushes

mpiechocki commented 1 year ago

The other thing I found is:

2023-08-28 15:01:14.645588+0200 Highrise[12134:335758] VERBOSE: setOneSignalDelegate:
2023-08-28 15:01:14.645670+0200 Highrise[12134:335758] VERBOSE: ONESIGNAL setOneSignalDelegate CALLED: (null)
2023-08-28 15:01:14.645719+0200 Highrise[12134:335758] VERBOSE: setOneSignalDelegate:
2023-08-28 15:01:14.645776+0200 Highrise[12134:335758] VERBOSE: ONESIGNAL setOneSignalDelegate CALLED: <SingularAppDelegate: 0x282c5c360>
2023-08-28 15:01:14.645823+0200 Highrise[12134:335758] VERBOSE: OneSignal already swizzled GUL_SingularAppDelegate-E1A7938C-E46D-4E7A-B4AF-4208A582C548 in super class: SingularAppDelegate

We're using Singular SDK also, can it be problematic?

mpiechocki commented 1 year ago

Ok, one more thing that might shed more light onto the problem: I found out that Unity's own handling of push notifications was getting in the way. What I had to do to prevent notifications from appearing while in foreground is to set

iOSNotificationCenter.OnRemoteNotificationReceived += OnRemoteNotificationReceived;

somewhere in the C# code. If this callback is added, Unity will prevent notifications from appearing while in foreground.

shepherd-l commented 1 year ago

I couldn’t reproduce the issue - I created a new project with just the OneSignal SDK and the fix.

Could you try creating a new project with only the OneSignal SDK and see if PreventDefualt still doesn’t work? If you are still able to reproduce the issue, could you share the project with us

If the issue does not occur, I suspect that there is a conflict with OneSignal and another SDK you are using. Could you try testing your SDKs one at a time with OneSignal in a new project to narrow down which SDK is causing the problem with OneSignal and share the project with us

You mentioned the Singular SDK, could you clarify to make sure

mpiechocki commented 1 year ago

@shepherd-l I'm currently off, but I'll try to reproduce it once I'm back :)

sherwinski commented 3 weeks ago

Hi, we have had no further reports of this. Please upgrade the OneSignal SDK if you or anyone is still having this issue. If this is still an issue, please open a new report with updated information and we will investigate. Thanks

OneSignal / OneSignal-Unity-SDK

[question]: OneSignal 5.0 iOS PushSubscription.Id crash and PreventDefualt still displaying on iOS #628

How can we help?

Code of Conduct