parse-server-modules / parse-server-onesignal-push-adapter

OneSignal push adapter for parse-server
MIT License
32 stars 28 forks source link

Not reliable #30

Closed lolobosse closed 7 years ago

lolobosse commented 7 years ago

Hi there,

I was experiencing the issue described here: http://blog.makingiants.com/onesignal-for-push/

The problem is basically very simple: this adapter uses OneSignal as a transparent way to GCM which seems not to be the case! If you use twice the same installation token, OneSignal just refuses it...

I solved my bug by modifying the adapter to use a oneSignalID that I retrieve in my apps and push to the Installation Object but that's a pretty big work... I also needed to change classifyInstallation to have access to this oneSignalID.

This is basically not a question but more a reflexion I was wondering @flovilmart : do you have this adapter up and running in prod?

And more globally, is OneSignal reliable? 😟 because, seriously, making the call not working twice under the pretext It is not recommended here doesn't inspire me lots of confidence...

Is someone experiencing the same as me?

flovilmart commented 7 years ago

I don't run that adapter in production, it's been originally contributed by a parse-server user. I suggest you have a look to other providers or the default adapter (that should properly deliver push)

lolobosse commented 7 years ago

Hmpf, yeah 😟

Btw, your work is great and I'm glad it exists. Nevertheless, we should maybe tell the other users of this adapter that they're using a 'not recommended way'.

What do you think?

flovilmart commented 7 years ago

Yeah, probably... but I would not close this repo either. If one want to use it, he can :)

lolobosse commented 7 years ago

Yeah, neither would I 😄

I'll do an update on the readme then

gdeglin commented 7 years ago

@lolobosse Hi there. I'm the original creator of this adapter, and one of the developers of OneSignal.

This is a problem that's actually pretty tricky. This happens when you use both the OneSignal Android SDK and the Parse adapter, and in the rare case that GCM/FCM sends us a canonical_id response.

Here's what happens:

  1. Our Android SDK registers with Android token "ABC" and OneSignal creates a registration record for this device.
  2. You send a notification to OneSignal to Android token "EFG" from parse-server
  3. OneSignal does not find this token in it's database, so it creates a new registration record with token "EFG"
  4. OneSignal sends your notification to "EFG"
  5. GCM replies with a "Canonical ID" telling us that device with token "EFG" is now using the token "ABC"
  6. In order to prevent having two registrations for one device (which could result in duplicate notifications), OneSignal marks the device with token "EFG" as unregistered.

It's not clear why GCM/FCM sometimes returns canonical ids for devices, and it's rare for this to happen to an app. However once it does, you can expect a large percentage of your registrations to now have a newer canonical ID.

To prevent this problem from happening, you should check with GCM/FCM to see if your registration tokens are up to date. You can do this by making the following API request:

--header "Content-Type: application/json" https://android.googleapis.com/gcm/send \
-d "{\"registration_ids\":[\"39sJUBhf91YTS7i17kJq_ayGn80QtOshULW1mLW8JW8iWx7yQeZTDOBoJ3UIs3InYLO5l1PNPLCdOOdB05zUO\"]}"

If you get a reply that contains canonical_ids it will look like this:

"success":1,
"failure":0,
"canonical_ids":1,
"results":[
    {"registration_id":"39sJUBhf91YTS7i17kJt_ayGn80QtOshULW1mLW8JW8i7yQeZTDOBoJ3UIs3InYLO5l1PNPLCdOB05zUO"}
  ]
}

You should then replace your old device token with the new one before sending them to OneSignal.

Another option is to always send notifications through OneSignal using other filtering criteria such as segments, filters, or include_player_ids. This would avoid this problem entirely.

flovilmart commented 7 years ago

@gdeglin , I've added you as admin on the repository, after all that's your code ;)

lolobosse commented 7 years ago

@gdeglin

Ok I see the problem and it is actually what I'm doing.

I already supposed that I was registering twice but I understand yet why this is causing a problem.

Nevertheless, I'm a bit lost when it comes to the resolution of this problem: I'm subscribing with Parse.subscribe(whatever) and I guess that OneSignal is also registering. The point is that OneSignal is not writing in my DB as Parse.subscribe does...

Option #1: I remove the call to Parse Subscribe, get the token from OneSignal and push it to the DB. Option #2: I change the adapter as I already did Option #3: You have a better solution :wink:

gdeglin commented 7 years ago

The modification you made to the adapter is probably the best way to go, especially since it seems that it's already done.

lolobosse commented 7 years ago

Done is better than perfect right?

I'll let it like this then and properly test regression 😄

Thanks a lot for your support 😉

lolobosse commented 7 years ago

@gdeglin

After a good night sleep, I have a WAAAAAAAY better fix: I just need to remove the OneSignal Lib from my Android App and use OneSignal as transparent gateway 😄 (Then, I just subscribe once to GCM and then I haven't any issue).

💤 helps 😉

I'm really retarded sometimes... OneSignal is perfectly reliable then... Sorry for the extra stress!