Merge pull request #892 from mozilla-services/feat/issue-744

feat: handle JSONResponse errors like provisioned errors

autopush/router/simple.py

@@ -11,10 +11,8 @@
 from typing import Any  # noqa
 
 import requests
-from boto.dynamodb2.exceptions import (
-    ItemNotFound,
-    ProvisionedThroughputExceededException,
-)
+from boto.dynamodb2.exceptions import ItemNotFound
+from boto.exception import JSONResponseError
 from twisted.internet.threads import deferToThread
 from twisted.internet.defer import (
     inlineCallbacks,
@@ -107,8 +105,8 @@ def route_notification(self, notification, uaid_data):
             if result is False:
                 self.metrics.increment("router.broadcast.miss")
                 returnValue(self.stored_response(notification))
-        except ProvisionedThroughputExceededException:
-            raise RouterException("Provisioned throughput error",
+        except JSONResponseError:
+            raise RouterException("Error saving to database",
                                   status_code=503,
                                   response_body="Retry Request",
                                   errno=201)
@@ -124,7 +122,7 @@ def route_notification(self, notification, uaid_data):
         #   - Error (no client) : Done, return 404
         try:
             uaid_data = yield deferToThread(router.get_uaid, uaid)
-        except ProvisionedThroughputExceededException:
+        except JSONResponseError:
             self.metrics.increment("router.broadcast.miss")
             returnValue(self.stored_response(notification))
         except ItemNotFound:
@@ -214,4 +212,4 @@ def _send_notification_check(self, uaid, node_id):
     #############################################################
     def _eat_db_err(self, fail):
         """errBack for ignoring provisioned throughput errors"""
-        fail.trap(ProvisionedThroughputExceededException)
+        fail.trap(JSONResponseError)

autopush/tests/test_websocket.py

@@ -937,9 +937,12 @@ def check_result(msg):
 
         return self._check_response(check_result)
 
-    def test_hello_jsonresponseerror_logged(self):
+    def test_hello_jsonresponseerror(self):
         self._connect()
 
+        self.proto.randrange = Mock()
+        self.proto.randrange.return_value = 0.1
+
         def throw_error(*args, **kwargs):
             raise JSONResponseError(None, None)
 
@@ -950,9 +953,7 @@ def throw_error(*args, **kwargs):
 
         def check_result(msg):
             eq_(msg["status"], 503)
-            eq_(msg["reason"], "error")
-            self.proto.log.info.assert_called()
-            self.proto.log.failure.assert_not_called()
+            eq_(msg["reason"], "error - overloaded")
             self.flushLoggedErrors()
 
         return self._check_response(check_result)

autopush/websocket.py

@@ -445,12 +445,8 @@ def base_tags(self):
 
     def log_failure(self, failure, **kwargs):
         """Log a twisted failure out through twisted's log.failure"""
-        exc = failure.value
-        if isinstance(exc, JSONResponseError):
-            self.log.info("JSONResponseError: {exc}", exc=exc, **kwargs)
-        else:
-            self.log.failure(format="Unexpected error", failure=failure,
-                             **kwargs)
+        self.log.failure(format="Unexpected error", failure=failure,
+                         **kwargs)
 
     @property
     def paused(self):
@@ -724,7 +720,7 @@ def returnError(self, messageType, reason, statusCode, close=True,
             self.sendClose()
 
     def err_overload(self, failure, message_type, disconnect=True):
-        """Handle database overloads
+        """Handle database overloads and errors
 
         If ``disconnect`` is False, the an overload error is returned and the
         client is not disconnected.
@@ -738,7 +734,7 @@ def err_overload(self, failure, message_type, disconnect=True):
         :param disconnect: Whether the client should be disconnected or not.
 
         """
-        failure.trap(ProvisionedThroughputExceededException)
+        failure.trap(JSONResponseError)
 
         if disconnect:
             self.transport.pauseProducing()