From d67dcb45f85e90af94cd5ca9318979dd4e00ff89 Mon Sep 17 00:00:00 2001 From: jrconlin Date: Thu, 27 Sep 2018 16:48:17 -0700 Subject: [PATCH] feat: make gcm calls use async callbacks Closes #1291 --- autopush/router/gcm.py | 55 ++++++----- autopush/router/gcmclient.py | 73 +++++++++----- autopush/tests/test_gcmclient.py | 118 ++++++++++++++--------- autopush/tests/test_integration.py | 39 ++++++-- autopush/tests/test_router.py | 149 ++++++++++++++++++++--------- requirements.txt | 31 +++--- 6 files changed, 301 insertions(+), 164 deletions(-) diff --git a/autopush/router/gcm.py b/autopush/router/gcm.py index bb4de1c6..3e349b6a 100644 --- a/autopush/router/gcm.py +++ b/autopush/router/gcm.py @@ -1,9 +1,8 @@ """GCM Router""" from typing import Any # noqa -from requests.exceptions import ConnectionError, Timeout -from twisted.internet.threads import deferToThread from twisted.logger import Logger +from twisted.internet.error import ConnectError, TimeoutError from autopush.exceptions import RouterException from autopush.metrics import make_tags @@ -28,7 +27,7 @@ def __init__(self, conf, router_conf, metrics): self.dryRun = router_conf.get("dryrun", False) self.collapseKey = router_conf.get("collapseKey") timeout = router_conf.get("timeout", 10) - self.gcm = {} + self.gcmclients = {} self.senderIDs = {} # Flatten the SenderID list from human readable and init gcmclient if not router_conf.get("senderIDs"): @@ -36,7 +35,8 @@ def __init__(self, conf, router_conf, metrics): for sid in router_conf.get("senderIDs"): auth = router_conf.get("senderIDs").get(sid).get("auth") self.senderIDs[sid] = auth - self.gcm[sid] = gcmclient.GCM(auth, timeout=timeout) + self.gcmclients[sid] = gcmclient.GCM(auth, timeout=timeout, + logger=self.log) self._base_tags = ["platform:gcm"] self.log.debug("Starting GCM router...") @@ -69,7 +69,7 @@ def register(self, uaid, router_data, app_id, *args, **kwargs): def route_notification(self, notification, uaid_data): """Start the GCM notification routing, returns a deferred""" # Kick the entire notification routing off to a thread - return deferToThread(self._route, notification, uaid_data) + return self._route(notification, uaid_data) def _route(self, notification, uaid_data): """Blocking GCM call to route the notification""" @@ -111,41 +111,48 @@ def _route(self, notification, uaid_data): data=data, ) try: - gcm = self.gcm[router_data['creds']['senderID']] - result = gcm.send(payload) - except RouterException: - raise # pragma nocover + client = self.gcmclients[router_data['creds']['senderID']] + d = client.send(payload) + d.addCallback( + self._process_reply, + uaid_data, + router_ttl, + notification) + + d.addErrback( + self._process_error + ) + return d except KeyError: self.log.critical("Missing GCM bridge credentials") raise RouterException("Server error", status_code=500, errno=900) - except gcmclient.GCMAuthenticationError as e: - self.log.error("GCM Authentication Error: %s" % e) + + def _process_error(self, failure): + err = failure.value + if isinstance(err, gcmclient.GCMAuthenticationError): + self.log.error("GCM Authentication Error: %s" % err) raise RouterException("Server error", status_code=500, errno=901) - except ConnectionError as e: - self.log.warn("GCM Unavailable: %s" % e) + if isinstance(err, TimeoutError): + self.log.warn("GCM Timeout: %s" % err) self.metrics.increment("notification.bridge.error", tags=make_tags( self._base_tags, - reason="connection_unavailable")) + reason="timeout")) raise RouterException("Server error", status_code=502, - errno=902, + errno=903, log_exception=False) - except Timeout as e: - self.log.warn("GCM Timeout: %s" % e) + if isinstance(err, ConnectError): + self.log.warn("GCM Unavailable: %s" % err) self.metrics.increment("notification.bridge.error", tags=make_tags( self._base_tags, - reason="timeout")) + reason="connection_unavailable")) raise RouterException("Server error", status_code=502, - errno=903, + errno=902, log_exception=False) - except Exception as e: - self.log.error("Unhandled exception in GCM Routing: %s" % e) - raise RouterException("Server error", status_code=500) - return self._process_reply(result, uaid_data, ttl=router_ttl, - notification=notification) + return failure def _error(self, err, status, **kwargs): """Error handler that raises the RouterException""" diff --git a/autopush/router/gcmclient.py b/autopush/router/gcmclient.py index 137df7ef..5f296fa0 100644 --- a/autopush/router/gcmclient.py +++ b/autopush/router/gcmclient.py @@ -1,6 +1,9 @@ import json -import requests +import treq +from twisted.web.http_headers import Headers +from twisted.logger import Logger +from twisted.internet.error import ConnectError from autopush.exceptions import RouterException @@ -12,7 +15,7 @@ class GCMAuthenticationError(Exception): class Result(object): """Abstraction object for GCM response""" - def __init__(self, message, response): + def __init__(self, response, message): """Process GCM message and response into abstracted object :param message: Message payload @@ -30,14 +33,13 @@ def __init__(self, message, response): self.message = message self.retry_message = None - self.retry_after = response.headers.get('Retry-After', None) + self.retry_after = ( + response.headers.getRawHeaders('Retry-After') or [None])[0] - if response.status_code != 200: - self.retry_message = message - else: - self._parse_response(message, response.content) - - def _parse_response(self, message, content): + def parse_response(self, content, code, message): + # 401 handled in GCM.process() + if code in (400, 404): + raise RouterException(content) data = json.loads(content) if not data.get('results'): raise RouterException("Recv'd invalid response from GCM") @@ -54,6 +56,7 @@ def _parse_response(self, message, content): self.not_registered.append(reg_id) else: self.failed[reg_id] = res['error'] + return self class JSONMessage(object): @@ -124,9 +127,31 @@ def __init__(self, self._endpoint = "https://{}".format(endpoint) self._api_key = api_key self.metrics = metrics - self.log = logger + self.log = logger or Logger() self._options = options - self._sender = requests.post + self._sender = treq.post + + def process(self, response, payload): + if response.code == 401: + raise GCMAuthenticationError("Authentication Error") + + result = Result(response, payload) + + if 500 <= response.code <= 599: + result.retry_message = payload + return result + + # Fetch the content body + d = response.text() + d.addCallback(result.parse_response, response.code, payload) + return d + + def error(self, failure): + if isinstance(failure.value, GCMAuthenticationError) or \ + isinstance(failure.value, ConnectError): + raise failure.value + self.log.error("GCMClient failure: {}".format(failure.value)) + raise RouterException("Server error: {}".format(failure.value)) def send(self, payload): """Send a payload to GCM @@ -136,23 +161,21 @@ def send(self, payload): :return: Result """ - headers = { - 'Content-Type': 'application/json', - 'Authorization': 'key={}'.format(self._api_key), - } + headers = Headers({ + 'Content-Type': ['application/json'], + 'Authorization': ['key={}'.format(self._api_key)], + }) + + if 'timeout' not in self._options: + self._options['timeout'] = 3 - response = self._sender( + d = self._sender( url=self._endpoint, headers=headers, data=json.dumps(payload.payload), **self._options ) - - if response.status_code in (400, 404): - raise RouterException(response.content) - - if response.status_code == 401: - raise GCMAuthenticationError("Authentication Error") - - if response.status_code == 200 or (500 <= response.status_code <= 599): - return Result(payload, response) + # handle the immediate response (which contains no body) + d.addCallback(self.process, payload) + d.addErrback(self.error) + return d diff --git a/autopush/tests/test_gcmclient.py b/autopush/tests/test_gcmclient.py index cbb0ecc9..2d4adb59 100644 --- a/autopush/tests/test_gcmclient.py +++ b/autopush/tests/test_gcmclient.py @@ -1,9 +1,11 @@ import json import pytest -import requests +import treq from mock import Mock +from twisted.internet.defer import Deferred, inlineCallbacks from twisted.trial import unittest +from twisted.web.http_headers import Headers from autopush.exceptions import RouterException from autopush.router import gcmclient @@ -13,10 +15,14 @@ class GCMClientTestCase(unittest.TestCase): def setUp(self): self.gcm = gcmclient.GCM(api_key="FakeValue") - self.gcm._sender = self._m_request = Mock(spec=requests.post) - self._m_response = Mock(spec=requests.Response) - self._m_response.return_value = 200 - self._m_response.headers = dict() + self.gcm._sender = Mock(spec=treq.request) + self._m_request = Deferred() + self.gcm._sender.return_value = self._m_request + self._m_response = Mock(spec=treq.response._Response) + self._m_response.code = 200 + self._m_response.headers = Headers() + self._m_resp_text = Deferred() + self._m_response.text.return_value = self._m_resp_text self.m_payload = gcmclient.JSONMessage( registration_ids="some_reg_id", collapse_key="coll_key", @@ -25,9 +31,9 @@ def setUp(self): data={"foo": "bar"} ) + @inlineCallbacks def test_send(self): - self._m_response.status_code = 200 - self._m_response.content = json.dumps({ + content = json.dumps({ "multicast_id": 5174939174563864884, "success": 1, "failure": 0, @@ -38,16 +44,17 @@ def test_send(self): } ] }) - self._m_request.return_value = self._m_response - result = self.gcm.send(self.m_payload) + self._m_resp_text.callback(content) + self._m_request.callback(self._m_response) + result = yield self.gcm.send(self.m_payload) assert len(result.failed) == 0 assert len(result.canonicals) == 0 assert (len(result.success) == 1 and self.m_payload.registration_ids[0] in result.success) + @inlineCallbacks def test_canonical(self): - self._m_response.status_code = 200 - self._m_response.content = json.dumps({ + content = json.dumps({ "multicast_id": 5174939174563864884, "success": 1, "failure": 0, @@ -59,8 +66,11 @@ def test_canonical(self): } ] }) - self._m_request.return_value = self._m_response - result = self.gcm.send(self.m_payload) + # pre-trigger the callbacks + self._m_resp_text.callback(content) + self._m_request.callback(self._m_response) + # and then trigger the main thread + result = yield self.gcm.send(self.m_payload) assert len(result.failed) == 0 assert len(result.canonicals) == 1 assert (len(result.success) == 1 @@ -76,9 +86,9 @@ def test_bad_jsonmessage(self): data={"foo": "bar"} ) + @inlineCallbacks def test_fail_invalid(self): - self._m_response.status_code = 200 - self._m_response.content = json.dumps({ + content = json.dumps({ "multicast_id": 5174939174563864884, "success": 0, "failure": 1, @@ -89,14 +99,17 @@ def test_fail_invalid(self): } ] }) + self._m_resp_text.callback(content) + self._m_request.callback(self._m_response) self._m_request.return_value = self._m_response - result = self.gcm.send(self.m_payload) + result = yield self.gcm.send(self.m_payload) assert len(result.failed) == 1 assert len(result.success) == 0 + @inlineCallbacks def test_fail_unavailable(self): - self._m_response.status_code = 200 - self._m_response.content = json.dumps({ + self._m_response.code = 200 + content = json.dumps({ "multicast_id": 5174939174563864884, "success": 0, "failure": 1, @@ -107,14 +120,15 @@ def test_fail_unavailable(self): } ] }) - self._m_request.return_value = self._m_response - result = self.gcm.send(self.m_payload) + self._m_resp_text.callback(content) + self._m_request.callback(self._m_response) + result = yield self.gcm.send(self.m_payload) assert len(result.unavailable) == 1 assert len(result.success) == 0 + @inlineCallbacks def test_fail_not_registered(self): - self._m_response.status_code = 200 - self._m_response.content = json.dumps({ + content = json.dumps({ "multicast_id": 5174939174563864884, "success": 0, "failure": 1, @@ -125,53 +139,63 @@ def test_fail_not_registered(self): } ] }) - self._m_request.return_value = self._m_response - result = self.gcm.send(self.m_payload) + self._m_resp_text.callback(content) + self._m_request.callback(self._m_response) + result = yield self.gcm.send(self.m_payload) assert len(result.not_registered) == 1 assert len(result.success) == 0 + @inlineCallbacks def test_fail_bad_response(self): - self._m_response.status_code = 200 - self._m_response.content = json.dumps({ + content = json.dumps({ "multicast_id": 5174939174563864884, "success": 0, "failure": 1, "canonical_ids": 0, }) - self._m_request.return_value = self._m_response + self._m_resp_text.callback(content) + self._m_request.callback(self._m_response) with pytest.raises(RouterException): - self.gcm.send(self.m_payload) + yield self.gcm.send(self.m_payload) + @inlineCallbacks def test_fail_400(self): - self._m_response.status_code = 400 - self._m_response.content = msg = "Invalid JSON" - self._m_request.return_value = self._m_response + self._m_response.code = 400 + content = msg = "Invalid JSON" + self._m_resp_text.callback(content) + self._m_request.callback(self._m_response) with pytest.raises(RouterException) as ex: - self.gcm.send(self.m_payload) + yield self.gcm.send(self.m_payload) assert ex.value.status_code == 500 - assert ex.value.message == msg + assert ex.value.message == "Server error: {}".format(msg) + @inlineCallbacks def test_fail_404(self): - self._m_response.status_code = 404 - self._m_response.content = msg = "Invalid URL" - self._m_request.return_value = self._m_response + self._m_response.code = 404 + content = msg = "Invalid URL" + self._m_resp_text.callback(content) + self._m_request.callback(self._m_response) with pytest.raises(RouterException) as ex: - self.gcm.send(self.m_payload) + yield self.gcm.send(self.m_payload) assert ex.value.status_code == 500 - assert ex.value.message == msg + assert ex.value.message == "Server error: {}".format(msg) + @inlineCallbacks def test_fail_401(self): - self._m_response.status_code = 401 - self._m_response.content = "Unauthorized" - self._m_request.return_value = self._m_response + self._m_response.code = 401 + content = "Unauthorized" + self._m_resp_text.callback(content) + self._m_request.callback(self._m_response) with pytest.raises(gcmclient.GCMAuthenticationError): - self.gcm.send(self.m_payload) + yield self.gcm.send(self.m_payload) + @inlineCallbacks def test_fail_500(self): - self._m_response.status_code = 500 - self._m_response.content = "OMG" - self._m_response.headers['Retry-After'] = 123 - self._m_request.return_value = self._m_response - result = self.gcm.send(self.m_payload) + self._m_response.code = 500 + content = "OMG" + self._m_response.headers.addRawHeader('Retry-After', 123) + self._m_resp_text.callback(content) + self._m_request.callback(self._m_response) + result = yield self.gcm.send(self.m_payload) assert 'some_reg_id' in result.retry_message.registration_ids assert result.retry_after == 123 diff --git a/autopush/tests/test_integration.py b/autopush/tests/test_integration.py index 9f3b4a32..8759a3c5 100644 --- a/autopush/tests/test_integration.py +++ b/autopush/tests/test_integration.py @@ -13,6 +13,7 @@ from httplib import HTTPResponse # noqa import pytest +import treq from mock import Mock, call, patch from unittest.case import SkipTest @@ -1676,10 +1677,31 @@ def _add_router(self): # The problem with calling the live system (even sandboxed) is that # you need a valid credential set from a mobile device, which can be # subject to change. - self._mock_send = Mock() - self._mock_reply = self.MockReply - self._mock_send.return_value = self._mock_reply - gcm.gcm[self.senderID].send = self._mock_send + self._mock_send = Mock(spec=treq.request) + self._m_request = Deferred() + self._mock_send.return_value = self._m_request + self._m_response = Mock(spec=treq.response._Response) + self._m_response.code = 200 + self._m_response.headers = Headers() + self._m_resp_text = Deferred() + self._m_response.text.return_value = self._m_resp_text + gcm.gcmclients[self.senderID]._sender = self._mock_send + + def _set_content(self, content=None): + if content is None: + content = { + "multicast_id": 5174939174563864884, + "success": 1, + "failure": 0, + "canonical_ids": 0, + "results": [ + { + "message_id": "0:1510011451922224%7a0e7efbaab8b7cc" + } + ] + } + self._m_resp_text.callback(json.dumps(content)) + self._m_request.callback(self._m_response) @inlineCallbacks def test_registration(self): @@ -1706,6 +1728,7 @@ def test_registration(self): "SYP0cZWNMJaT7YNaJUiSqBuGUxfRj-9vpTPz5ANmUYq3-u-HWOI") salt = "keyid=p256dh;salt=S82AseB7pAVBJ2143qtM3A" content_encoding = "aesgcm" + self._set_content() response, body = yield _agent( 'POST', @@ -1719,7 +1742,7 @@ def test_registration(self): body=data ) - ca_data = self._mock_send.call_args[0][0].payload['data'] + ca_data = json.loads(self._mock_send.call_args[1]['data'])['data'] assert response.code == 201 # ChannelID here MUST match what we got from the registration call. # Currently, this is a lowercase, hex UUID without dashes. @@ -1767,6 +1790,7 @@ def test_registration_aes128gcm(self): "\xe4\x43\x02\x5a\x72\xe0\x64\x69\xcd\x29\x6f\x65\x44\x53\x78" "\xe1\xd9\xf6\x46\x26\xce\x69") content_encoding = "aes128gcm" + self._set_content() response, body = yield _agent( 'POST', @@ -1778,7 +1802,7 @@ def test_registration_aes128gcm(self): body=data ) - ca_data = self._mock_send.call_args[0][0].payload['data'] + ca_data = json.loads(self._mock_send.call_args[1]['data'])['data'] assert response.code == 201 # ChannelID here MUST match what we got from the registration call. # Currently, this is a lowercase, hex UUID without dashes. @@ -1812,6 +1836,7 @@ def test_registration_aes128gcm_bad_(self): "SYP0cZWNMJaT7YNaJUiSqBuGUxfRj-9vpTPz5ANmUYq3-u-HWOI") salt = "keyid=p256dh;salt=S82AseB7pAVBJ2143qtM3A" content_encoding = "aes128gcm" + self._set_content() response, body = yield _agent( 'POST', @@ -1848,6 +1873,8 @@ def test_registration_no_token(self): "gcm", self.senderID, ) + self._set_content() + response, body = yield _agent('POST', url, body=json.dumps( { "chid": str(uuid.uuid4()), diff --git a/autopush/tests/test_router.py b/autopush/tests/test_router.py index dc90a991..377808f4 100644 --- a/autopush/tests/test_router.py +++ b/autopush/tests/test_router.py @@ -4,19 +4,24 @@ import time import json import decimal -import requests import socket import ssl import pytest +import treq from botocore.exceptions import ClientError from mock import Mock, PropertyMock, patch from twisted.trial import unittest -from twisted.internet.error import ConnectionRefusedError -from twisted.internet.defer import inlineCallbacks +from twisted.internet.error import ( + ConnectionRefusedError, + ConnectError, + TimeoutError, +) +from twisted.internet.defer import inlineCallbacks, Deferred +from twisted.python.failure import Failure from twisted.web.client import Agent +from twisted.web.http_headers import Headers -import hyper import hyper.tls import pyfcm from hyper.http20.exceptions import HTTP20Error @@ -357,9 +362,12 @@ def setUp(self): 'ttl': 60, 'senderIDs': {'test123': {"auth": "12345678abcdefg"}}} - self.response = Mock(spec=requests.Response) - self.response.status_code = 200 - self.response.headers = dict() + self._m_request = Deferred() + self.response = Mock(spec=treq.response._Response) + self.response.code = 200 + self.response.headers = Headers() + self._m_resp_text = Deferred() + self.response.text.return_value = self._m_resp_text self.response.content = json.dumps({ "multicast_id": 5174939174563864884, "success": 1, @@ -371,10 +379,11 @@ def setUp(self): } ] }) - self.gcm = gcmclient.GCM(api_key="SomeKey") - self.gcm._sender = Mock(return_value=self.response) + self.gcmclient = gcmclient.GCM(api_key="SomeKey") + self.gcmclient._sender = Mock() + self.gcmclient._sender.return_value = self._m_request self.router = GCMRouter(conf, self.gcm_config, SinkMetrics()) - self.router.gcm['test123'] = self.gcm + self.router.gcmclients['test123'] = self.gcmclient self.headers = {"content-encoding": "aesgcm", "encryption": "test", "encryption-key": "test"} @@ -393,14 +402,20 @@ def setUp(self): token="connect_data", creds=dict(senderID="test123", auth="12345678abcdefg"))) + def _set_content(self, content=None): + if content is None: + content = self.response.content + self._m_resp_text.callback(content) + self._m_request.callback(self.response) + def _check_error_call(self, exc, code, response=None, errno=None): assert isinstance(exc, RouterException) assert exc.status_code == code if errno is not None: assert exc.errno == errno - assert self.gcm._sender.called + assert self.gcmclient._sender.called if response: - assert exc.response_body == response + assert response in exc.response_body self.flushLoggedErrors() def test_init(self): @@ -432,14 +447,15 @@ def test_register_bad(self): app_id="invalid123") def test_route_notification(self): - self.router.gcm['test123'] = self.gcm + self.router.gcmclients['test123'] = self.gcmclient d = self.router.route_notification(self.notif, self.router_data) + self._set_content() def check_results(result): assert isinstance(result, RouterResponse) - assert self.gcm._sender.called + assert self.gcmclient._sender.called # Make sure the data was encoded as base64 - payload = json.loads(self.gcm._sender.call_args[1]['data']) + payload = json.loads(self.gcmclient._sender.call_args[1]['data']) data = payload['data'] assert data['body'] == 'q60d6g' assert data['enc'] == 'test' @@ -450,7 +466,7 @@ def check_results(result): return d def test_ttl_none(self): - self.router.gcm['test123'] = self.gcm + self.router.gcmclients['test123'] = self.gcmclient self.notif = WebPushNotification( uaid=uuid.UUID(dummy_uaid), channel_id=uuid.UUID(dummy_chid), @@ -459,6 +475,7 @@ def test_ttl_none(self): ttl=None ) self.notif.cleanup_headers() + self._set_content() d = self.router.route_notification(self.notif, self.router_data) def check_results(result): @@ -466,9 +483,9 @@ def check_results(result): assert result.status_code == 201 assert result.logged_status == 200 assert "TTL" in result.headers - assert self.gcm._sender.called + assert self.gcmclient._sender.called # Make sure the data was encoded as base64 - payload = json.loads(self.gcm._sender.call_args[1]['data']) + payload = json.loads(self.gcmclient._sender.call_args[1]['data']) data = payload['data'] assert data['body'] == 'q60d6g' assert data['enc'] == 'test' @@ -481,7 +498,7 @@ def check_results(result): return d def test_ttl_high(self): - self.router.gcm['test123'] = self.gcm + self.router.gcmclients['test123'] = self.gcmclient self.notif = WebPushNotification( uaid=uuid.UUID(dummy_uaid), channel_id=uuid.UUID(dummy_chid), @@ -490,13 +507,14 @@ def test_ttl_high(self): ttl=5184000 ) self.notif.cleanup_headers() + self._set_content() d = self.router.route_notification(self.notif, self.router_data) def check_results(result): assert isinstance(result, RouterResponse) - assert self.gcm._sender.called + assert self.gcmclient._sender.called # Make sure the data was encoded as base64 - payload = json.loads(self.gcm._sender.call_args[1]['data']) + payload = json.loads(self.gcmclient._sender.call_args[1]['data']) data = payload['data'] assert data['body'] == 'q60d6g' assert data['enc'] == 'test' @@ -509,7 +527,7 @@ def check_results(result): return d def test_long_data(self): - self.router.gcm['test123'] = self.gcm + self.router.gcmclients['test123'] = self.gcmclient bad_notif = WebPushNotification( uaid=uuid.UUID(dummy_uaid), channel_id=uuid.UUID(dummy_chid), @@ -517,30 +535,31 @@ def test_long_data(self): headers=self.headers, ttl=200 ) + self._set_content() - d = self.router.route_notification(bad_notif, self.router_data) - - def check_results(result): - assert isinstance(result.value, RouterException) - assert result.value.status_code == 413 - assert result.value.errno == 104 + with pytest.raises(RouterException) as ex: + self.router.route_notification(bad_notif, self.router_data) - d.addBoth(check_results) - return d + assert isinstance(ex.value, RouterException) + assert ex.value.status_code == 413 + assert ex.value.errno == 104 def test_route_crypto_notification(self): del(self.notif.headers['encryption_key']) self.notif.headers['crypto_key'] = 'crypto' + self._set_content() + d = self.router.route_notification(self.notif, self.router_data) def check_results(result): assert isinstance(result, RouterResponse) - assert self.gcm._sender.called + assert self.gcmclient._sender.called d.addCallback(check_results) return d def test_router_notification_gcm_auth_error(self): - self.response.status_code = 401 + self.response.code = 401 + self._set_content() d = self.router.route_notification(self.notif, self.router_data) def check_results(fail): @@ -549,7 +568,7 @@ def check_results(fail): return d def test_router_notification_gcm_other_error(self): - self.gcm._sender.side_effect = Exception + self._m_request.errback(Failure(Exception)) d = self.router.route_notification(self.notif, self.router_data) def check_results(fail): @@ -558,13 +577,9 @@ def check_results(fail): 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._sender.side_effect = throw_other - self.router.gcm['test123'] = self.gcm + self.router.gcmclients['test123'] = self.gcmclient + self._m_request.errback(Failure(ConnectError("oh my!"))) d = self.router.route_notification(self.notif, self.router_data) def check_results(fail): @@ -586,6 +601,7 @@ def test_router_notification_gcm_id_change(self): ] }) self.router.metrics = Mock() + self._set_content() d = self.router.route_notification(self.notif, self.router_data) def check_results(result): @@ -596,7 +612,7 @@ def check_results(result): self.router.metrics.increment.call_args[1]['tags'].sort() assert self.router.metrics.increment.call_args[1]['tags'] == [ 'platform:gcm', 'reason:reregister'] - assert self.gcm._sender.called + assert self.gcmclient._sender.called d.addCallback(check_results) return d @@ -613,6 +629,7 @@ def test_router_notification_gcm_not_regged(self): ] }) self.router.metrics = Mock() + self._set_content() d = self.router.route_notification(self.notif, self.router_data) def check_results(result): @@ -623,7 +640,7 @@ def check_results(result): self.router.metrics.increment.call_args[1]['tags'].sort() assert self.router.metrics.increment.call_args[1]['tags'] == [ 'platform:gcm', 'reason:unregistered'] - assert self.gcm._sender.called + assert self.gcmclient._sender.called d.addCallback(check_results) return d @@ -640,6 +657,7 @@ def test_router_notification_gcm_failed_items(self): ] }) self.router.metrics = Mock() + self._set_content() d = self.router.route_notification(self.notif, self.router_data) def check_results(fail): @@ -655,10 +673,11 @@ def check_results(fail): return d def test_router_notification_gcm_needs_retry(self): - self.response.headers['Retry-After'] = "123" - self.response.status_code = 500 + self.response.headers.addRawHeader('Retry-After', "123") + self.response.code = 500 self.response.content = "" self.router.metrics = Mock() + self._set_content() d = self.router.route_notification(self.notif, self.router_data) def check_results(fail): @@ -674,14 +693,50 @@ def check_results(fail): return d def test_router_notification_gcm_no_auth(self): - d = self.router.route_notification(self.notif, + self._set_content() + with pytest.raises(RouterException) as ex: + self.router.route_notification(self.notif, {"router_data": {"token": "abc"}}) + assert isinstance(ex.value, RouterException) + assert ex.value.message == "Server error" + assert ex.value.status_code == 500 + assert ex.value.errno == 900 + + def test_router_timeout(self): + self.router.metrics = Mock() + + def timeout(*args, **kwargs): + self._m_request.errback(Failure(TimeoutError())) + return self._m_request + + self.gcmclient._sender.side_effect = timeout + d = self.router.route_notification(self.notif, self.router_data) + + def check_results(fail): + assert self.router.metrics.increment.called + assert self.router.metrics.increment.call_args[0][0] == ( + 'notification.bridge.error') + self.router.metrics.increment.call_args[1]['tags'].sort() + assert self.router.metrics.increment.call_args[1]['tags'] == [ + 'platform:gcm', 'reason:timeout'] + + d.addBoth(check_results) + return d + + def test_router_unknown_err(self): + self.router.metrics = Mock() + + def timeout(*args, **kwargs): + self._m_request.errback(Failure(Exception())) + return self._m_request + + self.gcmclient._sender.side_effect = timeout + d = self.router.route_notification(self.notif, self.router_data) + def check_results(fail): assert isinstance(fail.value, RouterException) - assert fail.value.message == "Server error" - assert fail.value.status_code == 500 - assert fail.value.errno == 900 + d.addBoth(check_results) return d diff --git a/requirements.txt b/requirements.txt index 0557d07c..d7147404 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,17 +1,17 @@ -e git+https://github.com/habnabit/txstatsd.git@157ef85fbdeafe23865c7c4e176237ffcb3c3f1f#egg=txStatsD-master apns==2.0.1 -asn1crypto==0.24.0 # via cryptography +asn1crypto==0.24.0 # via cryptography, treq attrs==18.1.0 autobahn[twisted]==18.6.1 -automat==0.7.0 # via twisted +automat==0.7.0 # via twisted, treq boto3==1.7.57 botocore==1.10.57 # via boto3, s3transfer -certifi==2018.4.16 # via requests +certifi==2018.4.16 # via requests, treq cffi==1.11.5 -chardet==3.0.4 # via requests +chardet==3.0.4 # via requests, treq click==6.7 configargparse==0.13.0 -constantly==15.1.0 # via twisted +constantly==15.1.0 # via twisted, treq contextlib2==0.5.5 # via raven cryptography==2.3 cyclone==1.1 @@ -19,7 +19,7 @@ datadog==0.22.0 decorator==4.3.0 # via datadog docutils==0.14 # via botocore ecdsa==0.13 # via python-jose -enum34==1.1.6 # via cryptography, h2 +enum34==1.1.6 # via cryptography, h2, treq future==0.16.0 # via python-jose futures==3.2.0 # via s3transfer gcm-client==0.1.4 @@ -28,19 +28,19 @@ h2==2.6.2 # via hyper hpack==3.0.0 # via h2 hyper==0.7.0 hyperframe==3.2.0 # via h2, hyper -hyperlink==18.0.0 # via twisted -idna==2.7 # via cryptography, hyperlink, requests -incremental==17.5.0 # via twisted -ipaddress==1.0.22 # via cryptography +hyperlink==18.0.0 # via twisted, treq +idna==2.7 # via cryptography, hyperlink, requests, treq +incremental==17.5.0 # via twisted, treq +ipaddress==1.0.22 # via cryptography, treq jmespath==0.9.3 # via boto3, botocore marshmallow-polyfield==3.2 marshmallow==2.15.3 objgraph==3.4.0 -pyasn1-modules==0.2.2 # via service-identity +pyasn1-modules==0.2.2 # via service-identity, treq pyasn1==0.4.3 -pycparser==2.18 # via cffi +pycparser==2.18 # via cffi, treq pyfcm==1.4.5 -pyhamcrest==1.9.0 # via twisted +pyhamcrest==1.9.0 # via twisted, treq pyopenssl==18.0.0 python-dateutil==2.7.3 # via botocore python-jose==3.0.0 @@ -51,11 +51,12 @@ rsa==3.4.2 # via python-jose s3transfer==0.1.13 # via boto3 service-identity==17.0.0 simplejson==3.16.0 -six==1.11.0 # via autobahn, automat, cryptography, pyhamcrest, pyopenssl, python-dateutil, python-jose, txaio +six==1.11.0 # via autobahn, automat, cryptography, pyhamcrest, pyopenssl, python-dateutil, python-jose, txaio, treq +treq==18.6.0 twisted==18.7.0 txaio==2.10.0 # via autobahn typing==3.6.4 ua-parser==0.8.0 -urllib3==1.23 # via requests +urllib3==1.23 # via requests, treq wsaccel==0.6.2 ; platform_python_implementation == "CPython" zope.interface==4.5.0