From 39bae0b82f971b75c4f5944cbf9e90dff2bf200d Mon Sep 17 00:00:00 2001
From: jrconlin <jconlin+git@mozilla.com>
Date: Tue, 21 Feb 2017 11:27:15 -0800
Subject: [PATCH] bug: Do not attempt to register failed GCM registrations

GCM messages that trigger a "fail" were attempting to update
registration with insufficient data. It's better to treat them like
other GCM errors and report back, but take no additional actions.
Also increased testing around these sorts of errors.

closes #828
---
 autopush/router/gcm.py        | 13 ++-----------
 autopush/tests/test_router.py | 10 ++++++++++
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/autopush/router/gcm.py b/autopush/router/gcm.py
index 7928ea11..26f1b402 100644
--- a/autopush/router/gcm.py
+++ b/autopush/router/gcm.py
@@ -7,7 +7,6 @@
 
 from autopush.exceptions import RouterException
 from autopush.router.interface import RouterResponse
-from autopush.utils import ms_time
 
 
 class GCMRouter(object):
@@ -173,14 +172,6 @@ def _process_reply(self, reply, uaid_data, ttl, notification):
                                    self._base_tags)
             self.log.info("GCM failures: {failed()}",
                           failed=lambda: repr(reply.failed.items()))
-            self.router_table.register_user(
-                {"uaid": uaid_data.get('uaid'),
-                 "router_type": uaid_data.get("router_type", "gcm"),
-                 "connected_at": ms_time(),
-                 "critical_failure": "Client is unreachable due to a "
-                                     "configuration error. Unable to "
-                                     "send message.",
-                 })
             raise RouterException("GCM unable to deliver", status_code=410,
                                   response_body="GCM recipient not available.",
                                   log_exception=False,
@@ -188,10 +179,10 @@ def _process_reply(self, reply, uaid_data, ttl, notification):
 
         # retries:
         if reply.needs_retry():
-            self.log.warn("GCM retry requested: {failed()}",
-                          failed=lambda: repr(reply.failed.items()))
             self.metrics.increment("updates.client.bridge.gcm.failed.retry",
                                    self._base_tags)
+            self.log.warn("GCM retry requested: {failed()}",
+                          failed=lambda: repr(reply.failed.items()))
             raise RouterException("GCM failure to deliver, retry",
                                   status_code=503,
                                   response_body="Please try request later.",
diff --git a/autopush/tests/test_router.py b/autopush/tests/test_router.py
index dce99fea..74f08e87 100644
--- a/autopush/tests/test_router.py
+++ b/autopush/tests/test_router.py
@@ -532,9 +532,14 @@ def check_results(result):
     def test_router_notification_gcm_failed_items(self):
         self.mock_result.failed = dict(connect_data=True)
         self.router.gcm['test123'] = self.gcm
+        self.router.metrics = Mock()
         d = self.router.route_notification(self.notif, self.router_data)
 
         def check_results(fail):
+            ok_(self.router.metrics.increment.called)
+            eq_(self.router.metrics.increment.call_args[0][0],
+                'updates.client.bridge.gcm.failed.failure')
+            eq_(fail.value.message, 'GCM unable to deliver')
             self._check_error_call(fail.value, 410)
         d.addBoth(check_results)
         return d
@@ -542,9 +547,14 @@ def check_results(fail):
     def test_router_notification_gcm_needs_retry(self):
         self.mock_result.needs_retry.return_value = True
         self.router.gcm['test123'] = self.gcm
+        self.router.metrics = Mock()
         d = self.router.route_notification(self.notif, self.router_data)
 
         def check_results(fail):
+            ok_(self.router.metrics.increment.called)
+            eq_(self.router.metrics.increment.call_args[0][0],
+                'updates.client.bridge.gcm.failed.retry')
+            eq_(fail.value.message, 'GCM failure to deliver, retry')
             self._check_error_call(fail.value, 503)
         d.addBoth(check_results)
         return d