From 8cbe294d8b50d6d627d362df92951d316bd3c081 Mon Sep 17 00:00:00 2001 From: Philip Jenvey Date: Wed, 19 Oct 2016 17:22:15 -0700 Subject: [PATCH] fix: improve handling of JSONResponseErrors have websocket log them instead of passing along to sentry and give them a metric closes #703 --- autopush/db.py | 9 ++++++--- autopush/tests/test_websocket.py | 7 +++++++ autopush/websocket.py | 7 ++++++- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/autopush/db.py b/autopush/db.py index e664e7c5..60614e5e 100644 --- a/autopush/db.py +++ b/autopush/db.py @@ -266,6 +266,9 @@ def wrapper(self, *args, **kwargs): except ProvisionedThroughputExceededException: self.metrics.increment("error.provisioned.%s" % func.__name__) raise + except JSONResponseError: + self.metrics.increment("error.jsonresponse.%s" % func.__name__) + raise except BotoServerError: self.metrics.increment("error.botoserver.%s" % func.__name__) raise @@ -535,9 +538,9 @@ def fetch_messages(self, uaid, limit=10): """ # Eagerly fetches all results in the result set. - results = list(self.table.query_2(uaid__eq=hasher(uaid.hex), - chidmessageid__gt=" ", - consistent=True, limit=limit)) + results = self.table.query_2(uaid__eq=hasher(uaid.hex), + chidmessageid__gt=" ", + consistent=True, limit=limit) return [ WebPushNotification.from_message_table(uaid, x) for x in results ] diff --git a/autopush/tests/test_websocket.py b/autopush/tests/test_websocket.py index 98323ece..2a2bcf98 100644 --- a/autopush/tests/test_websocket.py +++ b/autopush/tests/test_websocket.py @@ -7,6 +7,7 @@ import twisted.internet.base from autopush.tests.test_db import make_webpush_notification from boto.dynamodb2.exceptions import ProvisionedThroughputExceededException +from boto.exception import JSONResponseError from cyclone.web import Application from mock import Mock, patch from nose.tools import assert_raises, eq_, ok_ @@ -14,6 +15,7 @@ from twisted.internet import reactor from twisted.internet.defer import Deferred from twisted.internet.error import ConnectError +from twisted.python.failure import Failure from twisted.trial import unittest import autopush.db as db @@ -2113,6 +2115,11 @@ def test_incomplete_uaid(self): ok_(fr.called) eq_(fr.call_args[0], (mm.drop_user, uaid)) + def test_log_failure_jsonresponseerror(self): + self.proto.log_failure(Failure(JSONResponseError(None, None))) + self.proto.log.info.assert_called() + self.proto.log.failure.assert_not_called() + class RouterHandlerTestCase(unittest.TestCase): def setUp(self): diff --git a/autopush/websocket.py b/autopush/websocket.py index 4704aa3c..68c95718 100644 --- a/autopush/websocket.py +++ b/autopush/websocket.py @@ -41,6 +41,7 @@ ProvisionedThroughputExceededException, ItemNotFound ) +from boto.exception import JSONResponseError from twisted.internet import reactor from twisted.internet.defer import ( Deferred, @@ -317,7 +318,11 @@ def base_tags(self): def log_failure(self, failure, **kwargs): """Log a twisted failure out through twisted's log.failure""" - self.log.failure(format="Unexpected error", failure=failure, **kwargs) + 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) @property def paused(self):