From 90b37d5ab5ca970900be74086993be76f770b226 Mon Sep 17 00:00:00 2001
From: vladikoff <vlad.filippov@gmail.com>
Date: Fri, 28 Oct 2016 20:11:54 -0400
Subject: [PATCH] fix(push): do not throw if push fails on the notify endpoint

Fixes #1510
---
 bin/key_server.js            |  2 +-
 lib/routes/account.js        | 13 +++++++++++++
 test/local/account_routes.js | 29 ++++++++++++++++++++++++++++-
 3 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/bin/key_server.js b/bin/key_server.js
index fefe3e531..e74bfc594 100644
--- a/bin/key_server.js
+++ b/bin/key_server.js
@@ -113,7 +113,7 @@ function main() {
                 function (err) {
                   if (err) {
                     log.error({ op: 'server.start.1', msg: 'failed startup with error',
-                                err: { message: err.message } })
+                      err: { message: err.message } })
                     process.exit(1)
                   } else {
                     log.info({ op: 'server.start.1', msg: 'running on ' + server.info.uri })
diff --git a/lib/routes/account.js b/lib/routes/account.js
index a92c63aae..2ea85eb83 100644
--- a/lib/routes/account.js
+++ b/lib/routes/account.js
@@ -1201,12 +1201,25 @@ module.exports = function (
 
         var endpointAction = 'devicesNotify'
         var stringUid = uid.toString('hex')
+
+        function catchPushError(err) {
+          // push may fail due to not found devices or a bad push action
+          // log the error but still respond with a 200.
+          log.error({
+            op: 'Account.devicesNotify',
+            uid: stringUid,
+            error: err
+          })
+        }
+
         return customs.checkAuthenticated(endpointAction, ip, stringUid)
           .then(function () {
             if (body.to === 'all') {
               return push.pushToAllDevices(uid, endpointAction, pushOptions)
+                .catch(catchPushError)
             } else {
               return push.pushToDevices(uid, body.to, endpointAction, pushOptions)
+                .catch(catchPushError)
             }
           })
           .done(
diff --git a/test/local/account_routes.js b/test/local/account_routes.js
index 043796de5..4e28e5465 100644
--- a/test/local/account_routes.js
+++ b/test/local/account_routes.js
@@ -466,7 +466,7 @@ test('/account/device', function (t) {
 })
 
 test('/account/devices/notify', function (t) {
-  t.plan(5)
+  t.plan(6)
   var config = {}
   var uid = uuid.v4('binary')
   var mockRequest = mocks.mockRequest({
@@ -599,6 +599,33 @@ test('/account/devices/notify', function (t) {
       t.equal(err.message, 'Client has sent too many requests')
     })
   })
+
+  t.test('logs error if no devices found', function (t) {
+    t.plan(1)
+    mockRequest.payload = {
+      to: ['bogusid1', 'bogusid2'],
+      TTL: 60,
+      payload: pushPayload
+    }
+
+    var mockLog = mocks.spyLog()
+    var mockPush = mocks.mockPush({
+      pushToDevices: () => P.reject('Devices ids not found in devices')
+    })
+    var mockCustoms = {
+      checkAuthenticated: () => P.resolve()
+    }
+
+    route = getRoute(makeRoutes({
+      customs: mockCustoms,
+      log: mockLog,
+      push: mockPush
+    }), '/account/devices/notify')
+
+    return runTest(route, mockRequest, function (response) {
+      t.equal(JSON.stringify(response), '{}', 'response should not throw push errors')
+    })
+  })
 })
 
 test('/account/device/destroy', function (t) {