search

appfeel / node-pushnotifications

Push notifications for GCM, APNS, MPNS, AMZ (automatic detection from device token)
MIT License
534 stars 126 forks source link

Error 401 after update from 1.0.21 to 1.1.3 #93

Closed FabiuxMLM closed 6 years ago

FabiuxMLM commented 6 years ago

After updating node-pushnotifications from version 1.0.21 to 1.1.3, when I try to send notification to Android device, I get the following error:

{ originalRegId: 'fqLI4ZlSrFg:APA91bF7LmHNKORmajS7WQQaRbO0wC4R-s9_lJKp3kLEtvuyjTLmCsl8xXn4Cqj_a8r2E9-HNPsn-fuNIG0YnhfGa2J99ZI770c9bNnXpXbiLEcUmQgf04QvEGIMyFw4PxOzS_evvgdPMEhF9dwIj4CmX8YA77vGsw', regId: 'fqLI4ZlSrFg:APA91bF7LmHNKORmajS7WQQaRbO0wC4R-s9_lJKp3kLEtvuyjTLmCsl8xXn4Cqj_a8r2E9-HNPsn-fuNIG0YnhfGa2J99ZI770c9bNnXpXbiLEcUmQgf04QvEGIMyFw4PxOzS_evvgdPMEhF9dwIj4CmX8YA77vGsw', error: 401, errorMsg: 401 }

The API key is correct (I didn't change anything). If I downgrade to 1.0.21 it work fine, if I force version 1.0.22, it doesn't work with error 401.

miqmago commented 6 years ago

@alex-friedl the only thing that makes me suspect is this line: https://github.com/appfeel/node-pushnotifications/compare/1.0.21...master#diff-ab450d0576edb25d11403fecc40706edR58

-    const id = opts.id;
+    const id = { opts };

I've never seen destructuring in that way, always seen like this:

    const { id } = opts;
miqmago commented 6 years ago

@FabiuxMLM v1.1.4 should fix this, please reopen otherwise

alex-friedl commented 6 years ago

@miqmago ooops, my bad. got sloppy there. Too bad the tests didn't catch this though! Thumbs up for adding the regression tests with your latest commit :)