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

mixed type string and Error used in the callback of Sender.prototype.sendNoRetry #230

Closed fabriziomoscon closed 8 years ago

fabriziomoscon commented 8 years ago

I think this should be avoided. It forces the library's users to do type switch in their app, and it is not intuitive.

https://github.com/ToothlessGear/node-gcm/blob/970b3e4e0f899974cd379b9db8fa54bc961e7879/lib/sender.js#L119-L141

eladnava commented 8 years ago

@fabriziomoscon I completely agree.

@hypesystem How about invoking the callback with an Error object instead, containing a message?

E.g.

return callback(new Error('Response is null.'));

And do this for any other error we pass onto the callback?

hypesystem commented 8 years ago

We have been talking about different variations of this question for a long time. We need a better response from send and sendNoRetry.

Right now, you might get a string, an error, or a number, depending on how sending fails.

The problem is that the numbers are functionally meaningful (they are the status code returned from the api, and should be dealt with in distinct ways), so changing the kind of error returned would be a breaking change.

More on this: #89 #71

eladnava commented 8 years ago

This will be resolved in v1.0.0 via https://github.com/ToothlessGear/node-gcm/pull/238/commits/ea0a850ba3a78f32ef78171c3f8a81048d37d84c.