Skip to content
This repository has been archived by the owner on Jul 13, 2023. It is now read-only.

Commit

Permalink
fix: improve handling of JSONResponseErrors (#718)
Browse files Browse the repository at this point in the history
have websocket log them instead of passing along to sentry and give them
a metric

closes #703
  • Loading branch information
pjenvey authored and bbangert committed Oct 20, 2016
1 parent 97a453a commit 29ff0fe
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 4 deletions.
9 changes: 6 additions & 3 deletions autopush/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
]
Expand Down
21 changes: 21 additions & 0 deletions autopush/tests/test_websocket.py
Original file line number Diff line number Diff line change
Expand Up @@ -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_
Expand Down Expand Up @@ -865,6 +866,26 @@ def check_result(msg):

return self._check_response(check_result)

def test_hello_jsonresponseerror_logged(self):
self._connect()

def throw_error(*args, **kwargs):
raise JSONResponseError(None, None)

router = self.proto.ap_settings.router
router.table.connection.update_item = Mock(side_effect=throw_error)

self._send_message(dict(messageType="hello", channelIDs=[]))

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()
self.flushLoggedErrors()

return self._check_response(check_result)

def test_hello_check_fail(self):
self._connect()

Expand Down
7 changes: 6 additions & 1 deletion autopush/websocket.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
ProvisionedThroughputExceededException,
ItemNotFound
)
from boto.exception import JSONResponseError
from twisted.internet import reactor
from twisted.internet.defer import (
Deferred,
Expand Down Expand Up @@ -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):
Expand Down

0 comments on commit 29ff0fe

Please sign in to comment.