Found an issue which causing the App crash.

[inampaki]

Resolved an issue which crashing the application that when string is passed to the object in 2nd argument instead of Array

sender.send(message, { registrationTokens: "device id"}, function (err, response) {
});

[eladnava]

@inampaki Thanks for your contribution!

I'm not sure that this is behavior that we'd like to support -- the documentation clearly demonstrates that the sender.send method accepts { registrationTokens: regTokens } as an array, and the name of the property registrationTokens is plural which would indicate an array.

var regTokens = ['YOUR_REG_TOKEN_HERE'];

// Now the sender can be used to send messages
sender.send(message, { registrationTokens: regTokens }, function (err, response) {
    if(err) console.error(err);
    else    console.log(response);
});

@hypesystem What do you think?

[inampaki]

I agree with you but in that case we should have some error returned instead of App crash due to undfined in response key at line 207

[eladnava]

@inampaki You're correct, it should definitely return an error to the callback provided and not crash the app.

Let's see if @hypesystem agrees, in that case, you can modify your PR to return an error instead of automatically converting the string to an array.

[inampaki]

@eladnava I have returned an error message as we discussed and reverted the conversion of object value to array as it was originally.

[eladnava]

@inampaki Also, be sure to run npm test before you publish commits to verify that your modifications haven't broken any tests.

[hypesystem]

First of all, thank you for your time, and well spotted error!

I agree that returning a legible error when passing a string where array is expected is a good idea.

This current code, as I see it, is less readable than the code before, and needs to be improved before merge. I will try to attach a few comments. In addition, @eladnava is correct in noting that it does not support topic and notificationKey properly. You would have seen this had you run npm test.

[hypesystem]

Now that we're at it, I think we could probably extract the whole extractRecipient function to its own file, as it is entirely standalone. This would make the sender and the extractRecipient code easier to get an overview of.

[inampaki]

@hypesystem thanks for the tips, you are right I think I should close this issue then.