ToothlessGear / node-gcm

A NodeJS wrapper library port to send data to Android devices via Google Cloud Messaging
https://github.com/ToothlessGear/node-gcm
Other
1.3k stars 206 forks source link

Better errors on wrongly-typed recipients #186

Closed hypesystem closed 8 years ago

hypesystem commented 8 years ago

Following discussion in #185, I think we should handle invalidly typed keys in some meaningful way (eg. returning a sensical error).

inampaki commented 8 years ago

@hypesystem also it would be really nice if you guys list them all in the documentation.

eladnava commented 8 years ago

@inampaki They types of keys? registrationTokens, topic, etc?

inampaki commented 8 years ago

@eladnava I meant if returned error would be part of documentation will help the user of module in development to check on those messages in his / her code.

eladnava commented 8 years ago

@inampaki Currently we return either a string or an error object as the err callback param when sending messages:

Are you suggesting that we return an object with an error code?

@hypesystem I think we need to normalize this as well.

inampaki commented 8 years ago

@eladnava I am just saying that whatever error is returned. Please document in readme so that a developer who is using the API can check those in the code

hypesystem commented 8 years ago

@eladnava this has been discussed previously. The only problem is that it would be a breaking change, so it has been deferred until we are ready for 1.0

We could, however, document the error codes that are currently returned as errors :)

eladnava commented 8 years ago

@hypesystem I see, so we should hold off on that for now.

How would you like to return the invalidly typed keys?

hypesystem commented 8 years ago

I am open to suggestions :smile:

eladnava commented 8 years ago

@hypesystem how about the following err object:

{
   "message": "Please provide 'registrationTokens' as an array."
}

Is there any other property you'd like added to the err object? A code maybe?

hypesystem commented 8 years ago

Seeing as this is a programmer error and they will probably read the message, I think this is fine :) I don't think a code would add anything in this case.

hypesystem commented 8 years ago

Maybe it could even be a proper Error object, so we get a stack trace :)

eladnava commented 8 years ago

@hypesystem I think an Error object is a great idea!

All we have to do is:

new Error("Please provide 'registrationTokens' as an array.")

And return that as the err callback param.

I'll be making the following changes in extractRecipient located in sender.js#L33:

And, add tests to senderSpec.js for invoking and testing these errors. Would you like to add anything before I begin?

hypesystem commented 8 years ago

Sounds good :+1:

eladnava commented 8 years ago

@hypesystem I'll also be listing them in the documentation.

hypesystem commented 8 years ago

:+1: