fix: improve handling of JSONResponseErrors

have websocket log them instead of passing along to sentry and give them
a metric

closes #703

autopush/router/apns2.py

@@ -122,16 +122,13 @@ def send(self, router_token, payload, apns_id,
                 raise RouterException(
                     "APNS Transmit Error {}:{}".format(response.status,
                                                        reason),
-                    status_code=500,
+                    status_code=502,
                     response_body="APNS could not process "
-                                  "your message {}".format(reason))
-        except HTTP20Error as ex:
+                                  "your message {}".format(reason)
+                )
+        except HTTP20Error:
             connection.close()
-            raise RouterException(
-                "APNS Processing error: {}".format(repr(ex)),
-                status_code=503,
-                response_body="APNS returned an error processing request",
-            )
+            raise
         finally:
             # Returning a closed connection to the pool is ok.
             # hyper will reconnect on .request()

autopush/router/apnsrouter.py

@@ -1,8 +1,9 @@
 """APNS Router"""
 import uuid
 
-from twisted.logger import Logger
+from hyper.http20.exceptions import ConnectionError, HTTP20Error
 from twisted.internet.threads import deferToThread
+from twisted.logger import Logger
 
 from autopush.exceptions import RouterException
 from autopush.router.apns2 import (
@@ -131,9 +132,27 @@ def _route(self, notification, router_data):
                                                           'Mozilla Push')),
                 content_available=1)
         apns_id = str(uuid.uuid4()).lower()
+        try:
+            apns_client.send(router_token=router_token, payload=payload,
+                             apns_id=apns_id)
+        except ConnectionError:
+            self.ap_settings.metrics.increment(
+                "updates.client.bridge.apns.connection_err",
+                self._base_tags
+            )
+            raise RouterException(
+                "Server error",
+                status_code=502,
+                response_body="APNS returned an error processing request",
+                log_exception=False,
+            )
+        except HTTP20Error:
+            raise RouterException(
+                "Server error",
+                status_code=502,
+                response_body="APNS returned an error processing request",
+            )
 
-        apns_client.send(router_token=router_token, payload=payload,
-                         apns_id=apns_id)
         location = "%s/m/%s" % (self.ap_settings.endpoint_url,
                                 notification.version)
         self.ap_settings.metrics.increment(

autopush/router/fcm.py

@@ -1,6 +1,7 @@
 """FCM Router"""
 
 import pyfcm
+from requests.exceptions import ConnectionError
 from twisted.internet.threads import deferToThread
 from twisted.logger import Logger
 
@@ -195,6 +196,12 @@ def _route(self, notification, router_data):
         except pyfcm.errors.AuthenticationError as e:
             self.log.error("Authentication Error: %s" % e)
             raise RouterException("Server error", status_code=500)
+        except ConnectionError as e:
+            self.metrics.increment("updates.client.bridge.fcm.connection_err",
+                                   self._base_tags)
+            self.log.warn("Could not connect to FCM server: %s" % e)
+            raise RouterException("Server error", status_code=502,
+                                  log_exception=False)
         except Exception as e:
             self.log.error("Unhandled FCM Error: %s" % e)
             raise RouterException("Server error", status_code=500)

autopush/router/gcm.py

@@ -1,6 +1,7 @@
 """GCM Router"""
 
 import gcmclient
+from requests.exceptions import ConnectionError
 from twisted.internet.threads import deferToThread
 from twisted.logger import Logger
 
@@ -121,6 +122,12 @@ def _route(self, notification, uaid_data):
         except gcmclient.GCMAuthenticationError as e:
             self.log.error("GCM Authentication Error: %s" % e)
             raise RouterException("Server error", status_code=500)
+        except ConnectionError as e:
+            self.log.warn("GCM Unavailable: %s" % e)
+            self.metrics.increment("updates.client.bridge.gcm.connection_err",
+                                   self._base_tags)
+            raise RouterException("Server error", status_code=502,
+                                  log_exception=False)
         except Exception as e:
             self.log.error("Unhandled exception in GCM Routing: %s" % e)
             raise RouterException("Server error", status_code=500)
@@ -175,7 +182,9 @@ def _process_reply(self, reply, uaid_data, ttl, notification):
                                      "send message.",
                  })
             raise RouterException("GCM unable to deliver", status_code=410,
-                                  response_body="GCM recipient not available.")
+                                  response_body="GCM recipient not available.",
+                                  log_exception=False,
+                                  )
 
         # retries:
         if reply.needs_retry():
@@ -185,7 +194,8 @@ def _process_reply(self, reply, uaid_data, ttl, notification):
                                    self._base_tags)
             raise RouterException("GCM failure to deliver, retry",
                                   status_code=503,
-                                  response_body="Please try request later.")
+                                  response_body="Please try request later.",
+                                  log_exception=False)
 
         self.metrics.increment("updates.client.bridge.gcm.succeeded",
                                self._base_tags)

autopush/tests/test_router.py

@@ -145,6 +145,23 @@ def test_register_bad_channel(self):
                 router_data={"token": "connect_data"},
                 app_id="unknown")
 
+    @inlineCallbacks
+    def test_connection_error(self):
+        from hyper.http20.exceptions import ConnectionError
+
+        def raiser(*args, **kwargs):
+            raise ConnectionError("oops")
+
+        self.router.apns['firefox'].connections[1].request = Mock(
+            side_effect=raiser)
+
+        with assert_raises(RouterException) as e:
+            yield self.router.route_notification(self.notif, self.router_data)
+
+        eq_(e.exception.response_body, 'APNS returned an error '
+                                       'processing request')
+        eq_(e.exception.status_code, 502)
+
     @inlineCallbacks
     def test_route_notification(self):
         result = yield self.router.route_notification(self.notif,
@@ -195,7 +212,7 @@ def test_bad_send(self):
         with assert_raises(RouterException) as ex:
             yield self.router.route_notification(self.notif, self.router_data)
         ok_(isinstance(ex.exception, RouterException))
-        eq_(ex.exception.status_code, 500)
+        eq_(ex.exception.status_code, 502)
         eq_(ex.exception.message, 'APNS Transmit Error 400:boo')
         eq_(ex.exception.response_body, 'APNS could not process your '
                                         'message boo')
@@ -209,9 +226,8 @@ def throw(*args, **kwargs):
         with assert_raises(RouterException) as ex:
             yield self.router.route_notification(self.notif, self.router_data)
         ok_(isinstance(ex.exception, RouterException))
-        eq_(ex.exception.status_code, 503)
-        eq_(ex.exception.message, "APNS Processing error: "
-                                  "HTTP20Error('oops',)")
+        eq_(ex.exception.status_code, 502)
+        eq_(ex.exception.message, "Server error")
         eq_(ex.exception.response_body, 'APNS returned an error processing '
                                         'request')
 
@@ -475,6 +491,21 @@ def check_results(fail):
         d.addBoth(check_results)
         return d
 
+    def test_router_notification_connection_error(self):
+        from requests.exceptions import ConnectionError
+
+        def throw_other(*args, **kwargs):
+            raise ConnectionError("oh my!")
+
+        self.gcm.send.side_effect = throw_other
+        self.router.gcm['test123'] = self.gcm
+        d = self.router.route_notification(self.notif, self.router_data)
+
+        def check_results(fail):
+            self._check_error_call(fail.value, 502, "Server error")
+        d.addBoth(check_results)
+        return d
+
     def test_router_notification_gcm_id_change(self):
         self.mock_result.canonical["old"] = "new"
         self.router.gcm['test123'] = self.gcm
@@ -754,6 +785,21 @@ def check_results(fail):
         d.addBoth(check_results)
         return d
 
+    def test_router_notification_connection_error(self):
+        from requests.exceptions import ConnectionError
+
+        def throw_other(*args, **kwargs):
+            raise ConnectionError("oh my!")
+
+        self.fcm.notify_single_device.side_effect = throw_other
+        self.router.fcm = self.fcm
+        d = self.router.route_notification(self.notif, self.router_data)
+
+        def check_results(fail):
+            self._check_error_call(fail.value, 502)
+        d.addBoth(check_results)
+        return d
+
     def test_router_notification_fcm_id_change(self):
         self.mock_result['canonical_ids'] = 1
         self.mock_result['results'][0] = {'registration_id': "new"}