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

Commit

Permalink
Merge pull request #1349 from mozilla-services/bug/1348
Browse files Browse the repository at this point in the history
bug: Handle FCM client not available
  • Loading branch information
pjenvey authored Sep 13, 2019
2 parents 7982514 + 91db1fa commit 4779682
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 6 deletions.
17 changes: 16 additions & 1 deletion autopush/router/fcm_v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@
from autopush.metrics import make_tags
from autopush.router.interface import RouterResponse
from autopush.router.fcm import FCMRouter
from autopush.router.fcmv1client import (FCMv1, FCMAuthenticationError)
from autopush.router.fcmv1client import (
FCMv1,
FCMAuthenticationError,
FCMNotFoundError
)
from autopush.types import JSONDict # noqa


Expand Down Expand Up @@ -143,6 +147,17 @@ def _process_error(self, failure):
raise RouterException("Server error", status_code=502,
errno=902,
log_exception=False)
if isinstance(err, FCMNotFoundError):
self.log.debug("FCM Recipient not found: %s" % err)
self.metrics.increment("notification.bridge.error",
tags=make_tags(
self._base_tags,
reason="recpient_gone"
))
raise RouterException("FCM Recipient no longer available",
status_code=404,
errno=106,
log_exception=False)
if isinstance(err, RouterException):
self.log.warn("FCM Error: {}".format(err))
self.metrics.increment("notification.bridge.error",
Expand Down
11 changes: 9 additions & 2 deletions autopush/router/fcmv1client.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ class FCMAuthenticationError(Exception):
pass


class FCMNotFoundError(Exception):
pass


class Result(object):

def __init__(self, response):
Expand Down Expand Up @@ -52,11 +56,13 @@ def parse_response(self, content):
return self
try:
data = json.loads(content)
if self.code in (400, 403, 404) or data.get('error'):
if self.code in (400, 403, 404, 410) or data.get('error'):
# Having a hard time finding information about how some
# things are handled in FCM, e.g. retransmit requests.
# For now, catalog them as errors and provide back-pressure.
err = data.get("error")
if err.get("status") == "NOT_FOUND":
raise FCMNotFoundError("FCM recipient no longer available")
raise RouterException("{}: {}".format(err.get("status"),
err.get("message")))
if "name" in data:
Expand Down Expand Up @@ -116,7 +122,8 @@ def process(self, response, payload=None):

def error(self, failure):
if isinstance(failure.value,
(FCMAuthenticationError, TimeoutError, ConnectError)):
(FCMAuthenticationError, FCMNotFoundError,
TimeoutError, ConnectError)):
raise failure.value
self.logger.error("FCMv1Client failure: {}".format(failure.value))
raise RouterException("Server error: {}".format(failure.value))
Expand Down
21 changes: 19 additions & 2 deletions autopush/tests/test_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -964,7 +964,7 @@ def check_results(result):
d.addCallback(check_results)
return d

def test_router_notification_gcm_auth_error(self):
def test_router_notification_fcm_auth_error(self):
self.response.code = 401
self._set_content()
d = self.router.route_notification(self.notif, self.router_data)
Expand All @@ -974,7 +974,24 @@ def check_results(fail):
d.addBoth(check_results)
return d

def test_router_notification_gcm_other_error(self):
def test_router_notification_fcm_not_found_error(self):
self.response.code = 404
self._set_content(json.dumps(
{"error":
{"code": 404,
"status": "NOT_FOUND",
"message": "Requested entity was not found."
}
}))
d = self.router.route_notification(self.notif, self.router_data)

def check_results(fail):
self._check_error_call(fail.value, 404,
"FCM Recipient no longer available", 106)
d.addBoth(check_results)
return d

def test_router_notification_fcm_other_error(self):
self._m_request.errback(Failure(Exception))
d = self.router.route_notification(self.notif, self.router_data)

Expand Down
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ service-identity==18.1.0
simplejson==3.16.0
six==1.12.0 # via autobahn, automat, cryptography, firebase-admin, google-api-core, google-api-python-client, google-auth, google-resumable-media, grpcio, marshmallow-polyfield, oauth2client, protobuf, pyhamcrest, pyopenssl, python-dateutil, python-jose, treq, txaio
treq==18.6.0
twisted[tls]==19.2.1
twisted[tls]==19.7.0
txaio==18.8.1 # via autobahn
typing==3.7.4
ua-parser==0.8.0
Expand Down

0 comments on commit 4779682

Please sign in to comment.