Sending notifications in parallel gives incorrect response

[alexfoxy]

Hey,

I'm having an issue where if I send two notifications in parallel, one to an ID of a device I know exists and one to a dummy ID, I get an 'InvalidRegistration' error from both .send() functions. The device does receive the 1st notification.

This causes an issue because when I get an error I want to remove that particular device token from the users tokens on the server. Currently it removes both the invalid token and the valid one resulting in the user having zero tokens and not receiving any future notifications.

Here's a gist of the script to cause this error: https://gist.github.com/alexfoxy/1e8cdb7c3f127e6594e8e963e39224e6

You should get an output of:

FCM.send
FCM.send
{"multicast_id":8969037943863178607,"success":0,"failure":1,"canonical_ids":0,"results":[{"error":"InvalidRegistration"}]}
{"multicast_id":8969037943863178607,"success":0,"failure":1,"canonical_ids":0,"results":[{"error":"InvalidRegistration"}]}

If you send them in series it works correctly [ i.e. by moving send2() into the callback of send1() ] - output:

FCM.send
{"multicast_id":7502545296558413858,"success":1,"failure":0,"canonical_ids":0,"results":[{"message_id":"0:1470046298701173%108c2029108c2029"}]}
FCM.send
{"multicast_id":8933073754773609814,"success":0,"failure":1,"canonical_ids":0,"results":[{"error":"InvalidRegistration"}]}

This might be a bug in Firebase itself so I'm also raising this as an issue with them.

A

[jlcvp]

@alexfoxy I reproduced it and I'm already looking for a fix (or a workaround or a new implementation for parallel calls in case this is a google's fault). I'll let you know when I push a fix

[alexfoxy]

@jlcvp awesome thanks. I have it temporarily fixed by doing it in series which is fine for testing as we only have a few users. Firebase said:

Thanks for bringing up this concern. I really wanted to help you here but I think this issue suits better on the that github thread. I saw that you already posted the issue there so let them check it first.

Feels like a bit of a palm off. If you find out any info about the problem, let me know and I'll pass it on to Firebase.

[jlcvp]

@alexfoxy, thanks again for reporting this. The problem was the logic of event emitter used by send function. It was also causing a memory leak in scenarios with 10+ parallel calls.

Pushed a fix and published a new version 1.0.12 on NPM.

[alexfoxy]

@jlcvp excellent thanks. I can confirm it's fixed. It was quite a nasty one to track down, great job on fixing it so fast :)

[antoinerousseau]

Thanks! I'm going to switch from fcm-push to this fork.

Also, did you take a look at the modifications made in other forks in your repo's network?

[jlcvp]

@antoinerousseau Yes, I took a look at eFishery's fork. They've implemented the topic messages too and refactored the send function to use promises instead of callbacks.