strongloop/loopback-component-push

Component Realiability

Closed this issue · 10 comments

Hi Guys,

I have found three issues with the Push Component that stops it from currently being reliable in a production environment.

When you are sending high-volume Push Notifications to Apple, especially in their development sandbox, sometimes Apple will reject a perfectly good Notification because it will detect a device token error. However, if you have the "retryLimit" set to a positive value, the APN module will retry to send a failed notification.

The problem with this comes in four places:

  1. After a umber of failed notifications, Apple will abruptly kill the socket. This is normal behaviour but it causes an uncaught exception inside of "loopback->contination-local-storage->glue.js" https://github.com/othiym23/async-listener/blob/master/glue.js#L188 This will cause the Node process to die, and you will never get the retry on your Push Notification. When I wrap it with `try { ... } catch(e) {console.log(e);}' I get the expected behaviour.
  2. There seems to be something wrong with how we are emitting transmissionError messages. Here https://github.com/strongloop/loopback-component-push/blob/master/lib/providers/apns.js#L69-L72 It seems like we are trying to emit the transmissionError message, but we are mangling it. This error is important because it is the one that lets you know exactly which message has been rejected by Apple.
  3. If the code above worked just fine, I assume there would be a matching block here: https://github.com/strongloop/loopback-component-push/blob/master/lib/push-manager.js#L129-L131 to catch the transmissionError and to send it up the chain.
  4. However even it is was, I never seem to get these messages from the PushManager. I expected to be able to something like:
var PushModel = app.models.Push;

PushModel.on('error', function (err) {
console.log('PushNotification->error: %j', err);
});

PushModel.on('transmissionError', function (code, notification, recipient) {
console.log('PushNotification->transmissionError: %j', code, notification, recipient);
});

But neither the 'error' or 'transmissionError' events make it further up the chain.

What am I missing here?

@dashby3000 says he has a test case if needed

@dashby3000 Where is your test case? I would like to reproduce the issue.

Will clean it up and post GitHub link later

@dashby3000 Any update?

@dashby3000 I need your test case to finish the story.

@dashby3000 @ijroth Is this not a priority anymore?

Yes, it is. I will provide the test case today.


Dennis Ashby

{ "title" : "Managing Partner",
"email" : "dennis@api.partners”,
"voice" : "415-596-9093",
"skype" : "dennis.ashby",
"twitter" : “@Dennis_Ashby" }

Sometimes your success is not measured in what you achieve, but in what you
overcome.

On Mon, Jun 29, 2015 at 8:42 AM, chandadharap notifications@github.com
wrote:

@dashby3000 https://github.com/dashby3000 @ijroth
https://github.com/ijroth Is this not a priority anymore?


Reply to this email directly or view it on GitHub
#85 (comment)
.

I also faced this issue and after looking at the code, I realized that the emitted error is not propagated to the push model. So, as a workaround, we can use:

push.connector.on('error', function (err) {
    console.log('Error ' + err);
  });

However, if we create a Push model and add the error event listener in the model.js (push.js) file, the connector is not linked yet. So, I tried 'dataSourceAttached' and 'attached' events but these are also not called. At the end, I had to improvise like this in server.js.

boot(app, __dirname, function (err) {
  if (err) {
    throw err;
  }
  app.dataSources.push.connector.on('error', function (err) {
    console.log('Error ' + err);
.....

+1

stale commented

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.