From 3218695b30ded412b354ee70a3362cfa8a72287f Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Wed, 12 Jun 2019 16:48:21 -0700 Subject: [PATCH 01/11] Migrated FCM send APIs to the new error handling regime --- firebase_admin/_messaging_utils.py | 32 ++++++ firebase_admin/_utils.py | 26 ++++- firebase_admin/messaging.py | 86 ++++++++------- integration/test_messaging.py | 21 +++- tests/test_messaging.py | 163 +++++++++++++++-------------- 5 files changed, 203 insertions(+), 125 deletions(-) diff --git a/firebase_admin/_messaging_utils.py b/firebase_admin/_messaging_utils.py index 17067f175..127221367 100644 --- a/firebase_admin/_messaging_utils.py +++ b/firebase_admin/_messaging_utils.py @@ -22,6 +22,8 @@ import six +from firebase_admin import exceptions + class Message(object): """A message that can be sent via Firebase Cloud Messaging. @@ -797,3 +799,33 @@ def default(self, obj): # pylint: disable=method-hidden if target_count != 1: raise ValueError('Exactly one of token, topic or condition must be specified.') return result + + +class ThirdPartyAuthError(exceptions.UnauthenticatedError): + """APNs certificate or web push auth key was invalid or missing.""" + + def __init__(self, message, cause=None, http_response=None): + exceptions.UnauthenticatedError.__init__(self, message, cause, http_response) + + +class QuotaExceededError(exceptions.ResourceExhaustedError): + """Sending limit exceeded for the message target.""" + + def __init__(self, message, cause=None, http_response=None): + exceptions.ResourceExhaustedError.__init__(self, message, cause, http_response) + + +class SenderIdMismatchError(exceptions.PermissionDeniedError): + """The authenticated sender ID is different from the sender ID for the registration token.""" + + def __init__(self, message, cause=None, http_response=None): + exceptions.PermissionDeniedError.__init__(self, message, cause, http_response) + + +class UnregisteredError(exceptions.NotFoundError): + """App instance was unregistered from FCM. + + This usually means that the token used is no longer valid and a new one must be used.""" + + def __init__(self, message, cause=None, http_response=None): + exceptions.NotFoundError.__init__(self, message, cause, http_response) diff --git a/firebase_admin/_utils.py b/firebase_admin/_utils.py index 61fe8eb1c..81e603221 100644 --- a/firebase_admin/_utils.py +++ b/firebase_admin/_utils.py @@ -29,6 +29,22 @@ 429: exceptions.ResourceExhaustedError, 500: exceptions.InternalError, 503: exceptions.UnavailableError, + + exceptions.INVALID_ARGUMENT: exceptions.InvalidArgumentError, + exceptions.FAILED_PRECONDITION: exceptions.FailedPreconditionError, + exceptions.OUT_OF_RANGE: exceptions.OutOfRangeError, + exceptions.UNAUTHENTICATED: exceptions.UnauthenticatedError, + exceptions.PERMISSION_DENIED: exceptions.PermissionDeniedError, + exceptions.NOT_FOUND: exceptions.NotFoundError, + exceptions.ABORTED: exceptions.AbortedError, + exceptions.ALREADY_EXISTS: exceptions.AlreadyExistsError, + exceptions.RESOURCE_EXHAUSTED: exceptions.ResourceExhaustedError, + exceptions.CANCELLED: exceptions.CancelledError, + exceptions.DATA_LOSS: exceptions.DataLossError, + exceptions.UNKNOWN: exceptions.UnknownError, + exceptions.INTERNAL: exceptions.InternalError, + exceptions.UNAVAILABLE: exceptions.UnavailableError, + exceptions.DEADLINE_EXCEEDED: exceptions.DeadlineExceededError, } @@ -56,8 +72,9 @@ def handle_requests_error(error, message=None, status=None): error: An error raised by the reqests module while making an HTTP call. message: A message to be included in the resulting ``FirebaseError`` (optional). If not specified the string representation of the ``error`` argument is used as the message. - status: An HTTP status code that will be used to determine the resulting error type - (optional). If not specified the HTTP status code on the error response is used. + status: An HTTP status code or GCP error code that will be used to determine the resulting + error type (optional). If not specified the HTTP status code on the error response is + used. Returns: FirebaseError: A ``FirebaseError`` that can be raised to the user code. @@ -79,5 +96,8 @@ def handle_requests_error(error, message=None, status=None): status = error.response.status_code if not message: message = str(error) - err_type = _STATUS_TO_EXCEPTION_TYPE.get(status, exceptions.UnknownError) + err_type = lookup_error_type(status) return err_type(message=message, cause=error, http_response=error.response) + +def lookup_error_type(status): + return _STATUS_TO_EXCEPTION_TYPE.get(status, exceptions.UnknownError) diff --git a/firebase_admin/messaging.py b/firebase_admin/messaging.py index 35d9e4ccd..270da32e9 100644 --- a/firebase_admin/messaging.py +++ b/firebase_admin/messaging.py @@ -75,6 +75,11 @@ WebpushNotification = _messaging_utils.WebpushNotification WebpushNotificationAction = _messaging_utils.WebpushNotificationAction +QuotaExceededError = _messaging_utils.QuotaExceededError +SenderIdMismatchError = _messaging_utils.SenderIdMismatchError +ThirdPartyAuthError = _messaging_utils.ThirdPartyAuthError +UnregisteredError = _messaging_utils.UnregisteredError + def _get_messaging_service(app): return _utils.get_app_service(app, _MESSAGING_ATTRIBUTE, _MessagingService) @@ -310,21 +315,12 @@ class _MessagingService(object): INTERNAL_ERROR = 'internal-error' UNKNOWN_ERROR = 'unknown-error' - FCM_ERROR_CODES = { - # FCM v1 canonical error codes - 'NOT_FOUND': 'registration-token-not-registered', - 'PERMISSION_DENIED': 'mismatched-credential', - 'RESOURCE_EXHAUSTED': 'message-rate-exceeded', - 'UNAUTHENTICATED': 'invalid-apns-credentials', - - # FCM v1 new error codes - 'APNS_AUTH_ERROR': 'invalid-apns-credentials', - 'INTERNAL': INTERNAL_ERROR, - 'INVALID_ARGUMENT': 'invalid-argument', - 'QUOTA_EXCEEDED': 'message-rate-exceeded', - 'SENDER_ID_MISMATCH': 'mismatched-credential', - 'UNAVAILABLE': 'server-unavailable', - 'UNREGISTERED': 'registration-token-not-registered', + FCM_ERROR_TYPES = { + 'APNS_AUTH_ERROR': ThirdPartyAuthError, + 'QUOTA_EXCEEDED': QuotaExceededError, + 'SENDER_ID_MISMATCH': SenderIdMismatchError, + 'THIRD_PARTY_AUTH_ERROR': ThirdPartyAuthError, + 'UNREGISTERED': UnregisteredError, } IID_ERROR_CODES = { 400: 'invalid-argument', @@ -367,11 +363,7 @@ def send(self, message, dry_run=False): timeout=self._timeout ) except requests.exceptions.RequestException as error: - if error.response is not None: - self._handle_fcm_error(error) - else: - msg = 'Failed to call messaging API: {0}'.format(error) - raise ApiCallError(self.INTERNAL_ERROR, msg, error) + self._handle_fcm_error(error) else: return resp['name'] @@ -459,17 +451,15 @@ def _postproc(self, _, body): def _handle_fcm_error(self, error): """Handles errors received from the FCM API.""" - data = {} - try: - parsed_body = error.response.json() - if isinstance(parsed_body, dict): - data = parsed_body - except ValueError: - pass + status, msg = None, None + if error.response is not None: + status, msg = _MessagingService._parse_fcm_error( + error.response.content.decode(), error.response.status_code) + exc_type = _MessagingService.FCM_ERROR_TYPES.get(status) + if exc_type: + raise exc_type(msg, cause=error, http_response=error.response) - code, msg = _MessagingService._parse_fcm_error( - data, error.response.content, error.response.status_code) - raise ApiCallError(code, msg, error) + raise _utils.handle_requests_error(error, message=msg, status=status) def _handle_iid_error(self, error): """Handles errors received from the Instance ID API.""" @@ -490,25 +480,34 @@ def _handle_iid_error(self, error): raise ApiCallError(code, msg, error) def _parse_batch_error(self, error): - """Parses a googleapiclient.http.HttpError content in to an ApiCallError.""" - if error.content is None: - msg = 'Failed to call messaging API: {0}'.format(error) - return ApiCallError(self.INTERNAL_ERROR, msg, error) + """Parses a googleapiclient.http.HttpError content and constructs a FirebaseError.""" + resp = requests.models.Response() + resp.raw = error.content + resp.status_code = error.resp.status + + status, msg = _MessagingService._parse_fcm_error(error.content.decode(), error.resp.status) + exc_type = _MessagingService.FCM_ERROR_TYPES.get(status) + if exc_type: + return exc_type(message=msg, cause=error, http_response=resp) + + if not status: + status = error.resp.status + if not msg: + msg = str(error) + err_type = _utils.lookup_error_type(status) + return err_type(message=msg, cause=error, http_response=resp) + @classmethod + def _parse_fcm_error(cls, content, status_code): + """Parses an error response from the FCM API and extracts the status and message fields.""" data = {} try: - parsed_body = json.loads(error.content.decode()) + parsed_body = json.loads(content) if isinstance(parsed_body, dict): data = parsed_body except ValueError: pass - code, msg = _MessagingService._parse_fcm_error(data, error.content, error.resp.status) - return ApiCallError(code, msg, error) - - @classmethod - def _parse_fcm_error(cls, data, content, status_code): - """Parses an error response from the FCM API to a ApiCallError.""" error_dict = data.get('error', {}) server_code = None for detail in error_dict.get('details', []): @@ -517,10 +516,9 @@ def _parse_fcm_error(cls, data, content, status_code): break if not server_code: server_code = error_dict.get('status') - code = _MessagingService.FCM_ERROR_CODES.get(server_code, _MessagingService.UNKNOWN_ERROR) msg = error_dict.get('message') if not msg: msg = 'Unexpected HTTP response with status: {0}; body: {1}'.format( - status_code, content.decode()) - return code, msg + status_code, content) + return server_code, msg diff --git a/integration/test_messaging.py b/integration/test_messaging.py index 7ebd5866a..ef5281523 100644 --- a/integration/test_messaging.py +++ b/integration/test_messaging.py @@ -16,6 +16,9 @@ import re +import pytest + +from firebase_admin import exceptions from firebase_admin import messaging @@ -47,6 +50,22 @@ def test_send(): msg_id = messaging.send(msg, dry_run=True) assert re.match('^projects/.*/messages/.*$', msg_id) +def test_send_invalid_token(): + msg = messaging.Message( + token=_REGISTRATION_TOKEN, + notification=messaging.Notification('test-title', 'test-body') + ) + with pytest.raises(messaging.SenderIdMismatchError): + messaging.send(msg, dry_run=True) + +def test_send_malformed_token(): + msg = messaging.Message( + token='not-a-token', + notification=messaging.Notification('test-title', 'test-body') + ) + with pytest.raises(exceptions.InvalidArgumentError): + messaging.send(msg, dry_run=True) + def test_send_all(): messages = [ messaging.Message( @@ -75,7 +94,7 @@ def test_send_all(): response = batch_response.responses[2] assert response.success is False - assert response.exception is not None + assert isinstance(response.exception, exceptions.InvalidArgumentError) assert response.message_id is None def test_send_one_hundred(): diff --git a/tests/test_messaging.py b/tests/test_messaging.py index de940b591..8bb07869f 100644 --- a/tests/test_messaging.py +++ b/tests/test_messaging.py @@ -23,6 +23,7 @@ from googleapiclient.http import HttpMockSequence import firebase_admin +from firebase_admin import exceptions from firebase_admin import messaging from tests import testutils @@ -31,7 +32,21 @@ NON_DICT_ARGS = ['', list(), tuple(), True, False, 1, 0, {1: 'foo'}, {'foo': 1}] NON_OBJECT_ARGS = [list(), tuple(), dict(), 'foo', 0, 1, True, False] NON_LIST_ARGS = ['', tuple(), dict(), True, False, 1, 0, [1], ['foo', 1]] -HTTP_ERRORS = [400, 404, 500] +HTTP_ERRORS = [400, 404, 500] # TODO(hkj): Remove this when IID tests are updated. +HTTP_ERROR_CODES = { + 400: exceptions.InvalidArgumentError, + 404: exceptions.NotFoundError, + 500: exceptions.InternalError, + 503: exceptions.UnavailableError, +} +FCM_ERROR_CODES = { + 'APNS_AUTH_ERROR': messaging.ThirdPartyAuthError, + 'QUOTA_EXCEEDED': messaging.QuotaExceededError, + 'SENDER_ID_MISMATCH': messaging.SenderIdMismatchError, + 'THIRD_PARTY_AUTH_ERROR': messaging.ThirdPartyAuthError, + 'UNREGISTERED': messaging.UnregisteredError, + 'SOMETHING_NEW': exceptions.UnknownError, +} def check_encoding(msg, expected=None): @@ -39,6 +54,13 @@ def check_encoding(msg, expected=None): if expected: assert encoded == expected +def check_exception(exception, message, status): + assert isinstance(exception, exceptions.FirebaseError) + assert str(exception) == message + assert exception.cause is not None + assert exception.http_response is not None + assert exception.http_response.status_code == status + class TestMulticastMessage(object): @@ -1258,15 +1280,14 @@ def test_send(self): body = {'message': messaging._MessagingService.encode_message(msg)} assert json.loads(recorder[0].body.decode()) == body - @pytest.mark.parametrize('status', HTTP_ERRORS) - def test_send_error(self, status): + @pytest.mark.parametrize('status,exc_type', HTTP_ERROR_CODES.items()) + def test_send_error(self, status, exc_type): _, recorder = self._instrument_messaging_service(status=status, payload='{}') msg = messaging.Message(topic='foo') - with pytest.raises(messaging.ApiCallError) as excinfo: + with pytest.raises(exc_type) as excinfo: messaging.send(msg) expected = 'Unexpected HTTP response with status: {0}; body: {{}}'.format(status) - assert str(excinfo.value) == expected - assert str(excinfo.value.code) == messaging._MessagingService.UNKNOWN_ERROR + check_exception(excinfo.value, expected, status) assert len(recorder) == 1 assert recorder[0].method == 'POST' assert recorder[0].url == self._get_url('explicit-project-id') @@ -1275,7 +1296,7 @@ def test_send_error(self, status): body = {'message': messaging._MessagingService.JSON_ENCODER.default(msg)} assert json.loads(recorder[0].body.decode()) == body - @pytest.mark.parametrize('status', HTTP_ERRORS) + @pytest.mark.parametrize('status', HTTP_ERROR_CODES) def test_send_detailed_error(self, status): payload = json.dumps({ 'error': { @@ -1285,17 +1306,16 @@ def test_send_detailed_error(self, status): }) _, recorder = self._instrument_messaging_service(status=status, payload=payload) msg = messaging.Message(topic='foo') - with pytest.raises(messaging.ApiCallError) as excinfo: + with pytest.raises(exceptions.InvalidArgumentError) as excinfo: messaging.send(msg) - assert str(excinfo.value) == 'test error' - assert str(excinfo.value.code) == 'invalid-argument' + check_exception(excinfo.value, 'test error', status) assert len(recorder) == 1 assert recorder[0].method == 'POST' assert recorder[0].url == self._get_url('explicit-project-id') body = {'message': messaging._MessagingService.JSON_ENCODER.default(msg)} assert json.loads(recorder[0].body.decode()) == body - @pytest.mark.parametrize('status', HTTP_ERRORS) + @pytest.mark.parametrize('status', HTTP_ERROR_CODES) def test_send_canonical_error_code(self, status): payload = json.dumps({ 'error': { @@ -1305,18 +1325,18 @@ def test_send_canonical_error_code(self, status): }) _, recorder = self._instrument_messaging_service(status=status, payload=payload) msg = messaging.Message(topic='foo') - with pytest.raises(messaging.ApiCallError) as excinfo: + with pytest.raises(exceptions.NotFoundError) as excinfo: messaging.send(msg) - assert str(excinfo.value) == 'test error' - assert str(excinfo.value.code) == 'registration-token-not-registered' + check_exception(excinfo.value, 'test error', status) assert len(recorder) == 1 assert recorder[0].method == 'POST' assert recorder[0].url == self._get_url('explicit-project-id') body = {'message': messaging._MessagingService.JSON_ENCODER.default(msg)} assert json.loads(recorder[0].body.decode()) == body - @pytest.mark.parametrize('status', HTTP_ERRORS) - def test_send_fcm_error_code(self, status): + @pytest.mark.parametrize('status', HTTP_ERROR_CODES) + @pytest.mark.parametrize('fcm_error_code, exc_type', FCM_ERROR_CODES.items()) + def test_send_fcm_error_code(self, status, fcm_error_code, exc_type): payload = json.dumps({ 'error': { 'status': 'INVALID_ARGUMENT', @@ -1324,17 +1344,16 @@ def test_send_fcm_error_code(self, status): 'details': [ { '@type': 'type.googleapis.com/google.firebase.fcm.v1.FcmError', - 'errorCode': 'UNREGISTERED', + 'errorCode': fcm_error_code, }, ], } }) _, recorder = self._instrument_messaging_service(status=status, payload=payload) msg = messaging.Message(topic='foo') - with pytest.raises(messaging.ApiCallError) as excinfo: + with pytest.raises(exc_type) as excinfo: messaging.send(msg) - assert str(excinfo.value) == 'test error' - assert str(excinfo.value.code) == 'registration-token-not-registered' + check_exception(excinfo.value, 'test error', status) assert len(recorder) == 1 assert recorder[0].method == 'POST' assert recorder[0].url == self._get_url('explicit-project-id') @@ -1418,7 +1437,7 @@ def test_send_all(self): assert all([r.success for r in batch_response.responses]) assert not any([r.exception for r in batch_response.responses]) - @pytest.mark.parametrize('status', HTTP_ERRORS) + @pytest.mark.parametrize('status', HTTP_ERROR_CODES) def test_send_all_detailed_error(self, status): success_payload = json.dumps({'name': 'message-id'}) error_payload = json.dumps({ @@ -1441,12 +1460,11 @@ def test_send_all_detailed_error(self, status): error_response = batch_response.responses[1] assert error_response.message_id is None assert error_response.success is False - assert error_response.exception is not None exception = error_response.exception - assert str(exception) == 'test error' - assert str(exception.code) == 'invalid-argument' + assert isinstance(exception, exceptions.InvalidArgumentError) + check_exception(exception, 'test error', status) - @pytest.mark.parametrize('status', HTTP_ERRORS) + @pytest.mark.parametrize('status', HTTP_ERROR_CODES) def test_send_all_canonical_error_code(self, status): success_payload = json.dumps({'name': 'message-id'}) error_payload = json.dumps({ @@ -1469,13 +1487,13 @@ def test_send_all_canonical_error_code(self, status): error_response = batch_response.responses[1] assert error_response.message_id is None assert error_response.success is False - assert error_response.exception is not None exception = error_response.exception - assert str(exception) == 'test error' - assert str(exception.code) == 'registration-token-not-registered' + assert isinstance(exception, exceptions.NotFoundError) + check_exception(exception, 'test error', status) - @pytest.mark.parametrize('status', HTTP_ERRORS) - def test_send_all_fcm_error_code(self, status): + @pytest.mark.parametrize('status', HTTP_ERROR_CODES) + @pytest.mark.parametrize('fcm_error_code, exc_type', FCM_ERROR_CODES.items()) + def test_send_all_fcm_error_code(self, status, fcm_error_code, exc_type): success_payload = json.dumps({'name': 'message-id'}) error_payload = json.dumps({ 'error': { @@ -1484,7 +1502,7 @@ def test_send_all_fcm_error_code(self, status): 'details': [ { '@type': 'type.googleapis.com/google.firebase.fcm.v1.FcmError', - 'errorCode': 'UNREGISTERED', + 'errorCode': fcm_error_code, }, ], } @@ -1503,22 +1521,20 @@ def test_send_all_fcm_error_code(self, status): error_response = batch_response.responses[1] assert error_response.message_id is None assert error_response.success is False - assert error_response.exception is not None exception = error_response.exception - assert str(exception) == 'test error' - assert str(exception.code) == 'registration-token-not-registered' + assert isinstance(exception, exc_type) + check_exception(exception, 'test error', status) - @pytest.mark.parametrize('status', HTTP_ERRORS) - def test_send_all_batch_error(self, status): + @pytest.mark.parametrize('status, exc_type', HTTP_ERROR_CODES.items()) + def test_send_all_batch_error(self, status, exc_type): _ = self._instrument_batch_messaging_service(status=status, payload='{}') msg = messaging.Message(topic='foo') - with pytest.raises(messaging.ApiCallError) as excinfo: + with pytest.raises(exc_type) as excinfo: messaging.send_all([msg]) expected = 'Unexpected HTTP response with status: {0}; body: {{}}'.format(status) - assert str(excinfo.value) == expected - assert str(excinfo.value.code) == messaging._MessagingService.UNKNOWN_ERROR + check_exception(excinfo.value, expected, status) - @pytest.mark.parametrize('status', HTTP_ERRORS) + @pytest.mark.parametrize('status', HTTP_ERROR_CODES) def test_send_all_batch_detailed_error(self, status): payload = json.dumps({ 'error': { @@ -1528,12 +1544,11 @@ def test_send_all_batch_detailed_error(self, status): }) _ = self._instrument_batch_messaging_service(status=status, payload=payload) msg = messaging.Message(topic='foo') - with pytest.raises(messaging.ApiCallError) as excinfo: + with pytest.raises(exceptions.InvalidArgumentError) as excinfo: messaging.send_all([msg]) - assert str(excinfo.value) == 'test error' - assert str(excinfo.value.code) == 'invalid-argument' + check_exception(excinfo.value, 'test error', status) - @pytest.mark.parametrize('status', HTTP_ERRORS) + @pytest.mark.parametrize('status', HTTP_ERROR_CODES) def test_send_all_batch_canonical_error_code(self, status): payload = json.dumps({ 'error': { @@ -1543,12 +1558,11 @@ def test_send_all_batch_canonical_error_code(self, status): }) _ = self._instrument_batch_messaging_service(status=status, payload=payload) msg = messaging.Message(topic='foo') - with pytest.raises(messaging.ApiCallError) as excinfo: + with pytest.raises(exceptions.NotFoundError) as excinfo: messaging.send_all([msg]) - assert str(excinfo.value) == 'test error' - assert str(excinfo.value.code) == 'registration-token-not-registered' + check_exception(excinfo.value, 'test error', status) - @pytest.mark.parametrize('status', HTTP_ERRORS) + @pytest.mark.parametrize('status', HTTP_ERROR_CODES) def test_send_all_batch_fcm_error_code(self, status): payload = json.dumps({ 'error': { @@ -1564,10 +1578,9 @@ def test_send_all_batch_fcm_error_code(self, status): }) _ = self._instrument_batch_messaging_service(status=status, payload=payload) msg = messaging.Message(topic='foo') - with pytest.raises(messaging.ApiCallError) as excinfo: + with pytest.raises(messaging.UnregisteredError) as excinfo: messaging.send_all([msg]) - assert str(excinfo.value) == 'test error' - assert str(excinfo.value.code) == 'registration-token-not-registered' + check_exception(excinfo.value, 'test error', status) class TestSendMulticast(TestBatch): @@ -1599,7 +1612,7 @@ def test_send_multicast(self): assert all([r.success for r in batch_response.responses]) assert not any([r.exception for r in batch_response.responses]) - @pytest.mark.parametrize('status', HTTP_ERRORS) + @pytest.mark.parametrize('status', HTTP_ERROR_CODES) def test_send_multicast_detailed_error(self, status): success_payload = json.dumps({'name': 'message-id'}) error_payload = json.dumps({ @@ -1624,10 +1637,10 @@ def test_send_multicast_detailed_error(self, status): assert error_response.success is False assert error_response.exception is not None exception = error_response.exception - assert str(exception) == 'test error' - assert str(exception.code) == 'invalid-argument' + assert isinstance(exception, exceptions.InvalidArgumentError) + check_exception(exception, 'test error', status) - @pytest.mark.parametrize('status', HTTP_ERRORS) + @pytest.mark.parametrize('status', HTTP_ERROR_CODES) def test_send_multicast_canonical_error_code(self, status): success_payload = json.dumps({'name': 'message-id'}) error_payload = json.dumps({ @@ -1652,10 +1665,10 @@ def test_send_multicast_canonical_error_code(self, status): assert error_response.success is False assert error_response.exception is not None exception = error_response.exception - assert str(exception) == 'test error' - assert str(exception.code) == 'registration-token-not-registered' + assert isinstance(exception, exceptions.NotFoundError) + check_exception(exception, 'test error', status) - @pytest.mark.parametrize('status', HTTP_ERRORS) + @pytest.mark.parametrize('status', HTTP_ERROR_CODES) def test_send_multicast_fcm_error_code(self, status): success_payload = json.dumps({'name': 'message-id'}) error_payload = json.dumps({ @@ -1686,20 +1699,19 @@ def test_send_multicast_fcm_error_code(self, status): assert error_response.success is False assert error_response.exception is not None exception = error_response.exception - assert str(exception) == 'test error' - assert str(exception.code) == 'registration-token-not-registered' + assert isinstance(exception, messaging.UnregisteredError) + check_exception(exception, 'test error', status) - @pytest.mark.parametrize('status', HTTP_ERRORS) - def test_send_multicast_batch_error(self, status): + @pytest.mark.parametrize('status, exc_type', HTTP_ERROR_CODES.items()) + def test_send_multicast_batch_error(self, status, exc_type): _ = self._instrument_batch_messaging_service(status=status, payload='{}') msg = messaging.MulticastMessage(tokens=['foo']) - with pytest.raises(messaging.ApiCallError) as excinfo: + with pytest.raises(exc_type) as excinfo: messaging.send_multicast(msg) expected = 'Unexpected HTTP response with status: {0}; body: {{}}'.format(status) - assert str(excinfo.value) == expected - assert str(excinfo.value.code) == messaging._MessagingService.UNKNOWN_ERROR + check_exception(excinfo.value, expected, status) - @pytest.mark.parametrize('status', HTTP_ERRORS) + @pytest.mark.parametrize('status', HTTP_ERROR_CODES) def test_send_multicast_batch_detailed_error(self, status): payload = json.dumps({ 'error': { @@ -1709,12 +1721,11 @@ def test_send_multicast_batch_detailed_error(self, status): }) _ = self._instrument_batch_messaging_service(status=status, payload=payload) msg = messaging.MulticastMessage(tokens=['foo']) - with pytest.raises(messaging.ApiCallError) as excinfo: + with pytest.raises(exceptions.InvalidArgumentError) as excinfo: messaging.send_multicast(msg) - assert str(excinfo.value) == 'test error' - assert str(excinfo.value.code) == 'invalid-argument' + check_exception(excinfo.value, 'test error', status) - @pytest.mark.parametrize('status', HTTP_ERRORS) + @pytest.mark.parametrize('status', HTTP_ERROR_CODES) def test_send_multicast_batch_canonical_error_code(self, status): payload = json.dumps({ 'error': { @@ -1724,12 +1735,11 @@ def test_send_multicast_batch_canonical_error_code(self, status): }) _ = self._instrument_batch_messaging_service(status=status, payload=payload) msg = messaging.MulticastMessage(tokens=['foo']) - with pytest.raises(messaging.ApiCallError) as excinfo: + with pytest.raises(exceptions.NotFoundError) as excinfo: messaging.send_multicast(msg) - assert str(excinfo.value) == 'test error' - assert str(excinfo.value.code) == 'registration-token-not-registered' + check_exception(excinfo.value, 'test error', status) - @pytest.mark.parametrize('status', HTTP_ERRORS) + @pytest.mark.parametrize('status', HTTP_ERROR_CODES) def test_send_multicast_batch_fcm_error_code(self, status): payload = json.dumps({ 'error': { @@ -1745,10 +1755,9 @@ def test_send_multicast_batch_fcm_error_code(self, status): }) _ = self._instrument_batch_messaging_service(status=status, payload=payload) msg = messaging.MulticastMessage(tokens=['foo']) - with pytest.raises(messaging.ApiCallError) as excinfo: + with pytest.raises(messaging.UnregisteredError) as excinfo: messaging.send_multicast(msg) - assert str(excinfo.value) == 'test error' - assert str(excinfo.value.code) == 'registration-token-not-registered' + check_exception(excinfo.value, 'test error', status) class TestTopicManagement(object): From e734cf9dbf84fde80027c1a3cd54d28eade3b1d2 Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Wed, 12 Jun 2019 17:12:16 -0700 Subject: [PATCH 02/11] Moved error parsing logic to _utils --- firebase_admin/_utils.py | 35 ++++++++++++++++++++++++++ firebase_admin/messaging.py | 49 +++++++++++++------------------------ 2 files changed, 52 insertions(+), 32 deletions(-) diff --git a/firebase_admin/_utils.py b/firebase_admin/_utils.py index 81e603221..d13ab5c9b 100644 --- a/firebase_admin/_utils.py +++ b/firebase_admin/_utils.py @@ -14,6 +14,8 @@ """Internal utilities common to all modules.""" +import json + import requests import firebase_admin @@ -100,4 +102,37 @@ def handle_requests_error(error, message=None, status=None): return err_type(message=message, cause=error, http_response=error.response) def lookup_error_type(status): + """Maps an error code to an exception type.""" return _STATUS_TO_EXCEPTION_TYPE.get(status, exceptions.UnknownError) + +def parse_platform_error(content, status_code, parse_func=None): + """Parses an HTTP error response from a Google Cloud Platform API and extracts the error code + and message fields. + + Args: + content: Decoded content of the response body. + status_code: HTTP status code. + parse_func: A custom function to extract the code from the error body (optional). + + Returns: + tuple: A tuple containing error code and message. Either or both could be ``None``. + """ + data = {} + try: + parsed_body = json.loads(content) + if isinstance(parsed_body, dict): + data = parsed_body + except ValueError: + pass + + error_dict = data.get('error', {}) + server_code = None + if parse_func: + server_code = parse_func(error_dict) + if not server_code: + server_code = error_dict.get('status') + + msg = error_dict.get('message') + if not msg: + msg = 'Unexpected HTTP response with status: {0}; body: {1}'.format(status_code, content) + return server_code, msg diff --git a/firebase_admin/messaging.py b/firebase_admin/messaging.py index 270da32e9..a314dedd3 100644 --- a/firebase_admin/messaging.py +++ b/firebase_admin/messaging.py @@ -451,15 +451,16 @@ def _postproc(self, _, body): def _handle_fcm_error(self, error): """Handles errors received from the FCM API.""" - status, msg = None, None + code, msg = None, None if error.response is not None: - status, msg = _MessagingService._parse_fcm_error( - error.response.content.decode(), error.response.status_code) - exc_type = _MessagingService.FCM_ERROR_TYPES.get(status) + code, msg = _utils.parse_platform_error( + content=error.response.content.decode(), status_code=error.response.status_code, + parse_func=_MessagingService._parse_fcm_error) + exc_type = _MessagingService.FCM_ERROR_TYPES.get(code) if exc_type: raise exc_type(msg, cause=error, http_response=error.response) - raise _utils.handle_requests_error(error, message=msg, status=status) + raise _utils.handle_requests_error(error, message=msg, status=code) def _handle_iid_error(self, error): """Handles errors received from the Instance ID API.""" @@ -485,40 +486,24 @@ def _parse_batch_error(self, error): resp.raw = error.content resp.status_code = error.resp.status - status, msg = _MessagingService._parse_fcm_error(error.content.decode(), error.resp.status) - exc_type = _MessagingService.FCM_ERROR_TYPES.get(status) + code, msg = _utils.parse_platform_error( + content=error.content.decode(), status_code=error.resp.status, + parse_func=_MessagingService._parse_fcm_error) + exc_type = _MessagingService.FCM_ERROR_TYPES.get(code) if exc_type: return exc_type(message=msg, cause=error, http_response=resp) - if not status: - status = error.resp.status + if not code: + code = error.resp.status if not msg: msg = str(error) - err_type = _utils.lookup_error_type(status) + err_type = _utils.lookup_error_type(code) return err_type(message=msg, cause=error, http_response=resp) @classmethod - def _parse_fcm_error(cls, content, status_code): - """Parses an error response from the FCM API and extracts the status and message fields.""" - data = {} - try: - parsed_body = json.loads(content) - if isinstance(parsed_body, dict): - data = parsed_body - except ValueError: - pass - - error_dict = data.get('error', {}) - server_code = None + def _parse_fcm_error(cls, error_dict): + """Parses an error response from the FCM API and extracts the error code.""" for detail in error_dict.get('details', []): if detail.get('@type') == 'type.googleapis.com/google.firebase.fcm.v1.FcmError': - server_code = detail.get('errorCode') - break - if not server_code: - server_code = error_dict.get('status') - - msg = error_dict.get('message') - if not msg: - msg = 'Unexpected HTTP response with status: {0}; body: {1}'.format( - status_code, content) - return server_code, msg + return detail.get('errorCode') + return None From 8df345ae9e76f11f0ef35f5ef16935c1a9b69b1f Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Thu, 13 Jun 2019 00:06:47 -0700 Subject: [PATCH 03/11] Refactored OP error handling code --- firebase_admin/_utils.py | 59 +++++++++++++++++++++++++++---------- firebase_admin/messaging.py | 17 +++++------ 2 files changed, 51 insertions(+), 25 deletions(-) diff --git a/firebase_admin/_utils.py b/firebase_admin/_utils.py index d13ab5c9b..5bfd79e81 100644 --- a/firebase_admin/_utils.py +++ b/firebase_admin/_utils.py @@ -22,7 +22,7 @@ from firebase_admin import exceptions -_STATUS_TO_EXCEPTION_TYPE = { +_ERROR_CODE_TO_EXCEPTION_TYPE = { 400: exceptions.InvalidArgumentError, 401: exceptions.UnauthenticatedError, 403: exceptions.PermissionDeniedError, @@ -50,6 +50,18 @@ } +_HTTP_STATUS_TO_ERROR_CODE = { + 400: exceptions.INVALID_ARGUMENT, + 401: exceptions.UNAUTHENTICATED, + 403: exceptions.PERMISSION_DENIED, + 404: exceptions.NOT_FOUND, + 409: exceptions.CONFLICT, + 429: exceptions.RESOURCE_EXHAUSTED, + 500: exceptions.INTERNAL, + 503: exceptions.UNAVAILABLE, +} + + def _get_initialized_app(app): if app is None: return firebase_admin.get_app() @@ -63,18 +75,20 @@ def _get_initialized_app(app): raise ValueError('Illegal app argument. Argument must be of type ' ' firebase_admin.App, but given "{0}".'.format(type(app))) + def get_app_service(app, name, initializer): app = _get_initialized_app(app) return app._get_service(name, initializer) # pylint: disable=protected-access -def handle_requests_error(error, message=None, status=None): + +def handle_requests_error(error, message=None, code=None): """Constructs a ``FirebaseError`` from the given requests error. Args: error: An error raised by the reqests module while making an HTTP call. message: A message to be included in the resulting ``FirebaseError`` (optional). If not specified the string representation of the ``error`` argument is used as the message. - status: An HTTP status code or GCP error code that will be used to determine the resulting + code: An HTTP status code or GCP error code that will be used to determine the resulting error type (optional). If not specified the HTTP status code on the error response is used. @@ -94,16 +108,24 @@ def handle_requests_error(error, message=None, status=None): message='Unknown error while making a remote service call: {0}'.format(error), cause=error) - if not status: - status = error.response.status_code + if not code: + code = error.response.status_code if not message: message = str(error) - err_type = lookup_error_type(status) + err_type = lookup_error_type(code) return err_type(message=message, cause=error, http_response=error.response) -def lookup_error_type(status): + +def lookup_error_type(code): """Maps an error code to an exception type.""" - return _STATUS_TO_EXCEPTION_TYPE.get(status, exceptions.UnknownError) + return _ERROR_CODE_TO_EXCEPTION_TYPE.get(code, exceptions.UnknownError) + + +def parse_requests_platform_error(response, parse_func=None): + content = response.content.decode() + status_code = response.status_code + return parse_platform_error(content, status_code, parse_func) + def parse_platform_error(content, status_code, parse_func=None): """Parses an HTTP error response from a Google Cloud Platform API and extracts the error code @@ -115,7 +137,7 @@ def parse_platform_error(content, status_code, parse_func=None): parse_func: A custom function to extract the code from the error body (optional). Returns: - tuple: A tuple containing error code and message. Either or both could be ``None``. + tuple: A tuple containing error code and message. """ data = {} try: @@ -126,13 +148,20 @@ def parse_platform_error(content, status_code, parse_func=None): pass error_dict = data.get('error', {}) - server_code = None - if parse_func: - server_code = parse_func(error_dict) - if not server_code: - server_code = error_dict.get('status') - + server_code = _get_error_code(error_dict, status_code, parse_func) msg = error_dict.get('message') if not msg: msg = 'Unexpected HTTP response with status: {0}; body: {1}'.format(status_code, content) return server_code, msg + + +def _get_error_code(error_dict, status_code, parse_func): + code = _try_get_error_code_from_body(error_dict, parse_func) + return code if code else _HTTP_STATUS_TO_ERROR_CODE.get(status_code, exceptions.UNKNOWN) + + +def _try_get_error_code_from_body(error_dict, parse_func): + code = None + if parse_func: + code = parse_func(error_dict) + return code if code else error_dict.get('status') diff --git a/firebase_admin/messaging.py b/firebase_admin/messaging.py index a314dedd3..350abee83 100644 --- a/firebase_admin/messaging.py +++ b/firebase_admin/messaging.py @@ -451,16 +451,16 @@ def _postproc(self, _, body): def _handle_fcm_error(self, error): """Handles errors received from the FCM API.""" - code, msg = None, None + code, message = None, None if error.response is not None: - code, msg = _utils.parse_platform_error( - content=error.response.content.decode(), status_code=error.response.status_code, + code, message = _utils.parse_requests_platform_error( + response=error.response, parse_func=_MessagingService._parse_fcm_error) exc_type = _MessagingService.FCM_ERROR_TYPES.get(code) if exc_type: - raise exc_type(msg, cause=error, http_response=error.response) + raise exc_type(message, cause=error, http_response=error.response) - raise _utils.handle_requests_error(error, message=msg, status=code) + raise _utils.handle_requests_error(error, message=message, code=code) def _handle_iid_error(self, error): """Handles errors received from the Instance ID API.""" @@ -487,16 +487,13 @@ def _parse_batch_error(self, error): resp.status_code = error.resp.status code, msg = _utils.parse_platform_error( - content=error.content.decode(), status_code=error.resp.status, + content=error.content.decode(), + status_code=error.resp.status, parse_func=_MessagingService._parse_fcm_error) exc_type = _MessagingService.FCM_ERROR_TYPES.get(code) if exc_type: return exc_type(message=msg, cause=error, http_response=resp) - if not code: - code = error.resp.status - if not msg: - msg = str(error) err_type = _utils.lookup_error_type(code) return err_type(message=msg, cause=error, http_response=resp) From ce4a789bb10484ed6a6433895d17ee4383d8576c Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Thu, 13 Jun 2019 10:03:06 -0700 Subject: [PATCH 04/11] Fixing a broken test --- tests/test_exceptions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index f2897ab3c..0e4f3f916 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -78,7 +78,7 @@ def test_http_response_with_status(): resp = models.Response() resp.status_code = 500 error = requests.exceptions.RequestException('Test error', response=resp) - firebase_error = _utils.handle_requests_error(error, status=503) + firebase_error = _utils.handle_requests_error(error, code=503) assert isinstance(firebase_error, exceptions.UnavailableError) assert str(firebase_error) == 'Test error' assert firebase_error.cause is error @@ -89,7 +89,7 @@ def test_http_response_with_message_and_status(): resp.status_code = 500 error = requests.exceptions.RequestException('Test error', response=resp) firebase_error = _utils.handle_requests_error( - error, message='Explicit error message', status=503) + error, message='Explicit error message', code=503) assert isinstance(firebase_error, exceptions.UnavailableError) assert str(firebase_error) == 'Explicit error message' assert firebase_error.cause is error From 3b6b78d263f3c63b7b44cef3053bd58dd64d7c34 Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Thu, 13 Jun 2019 11:10:30 -0700 Subject: [PATCH 05/11] Added utils for handling googleapiclient errors --- firebase_admin/_utils.py | 51 ++++++++++++++++++++++++++++++++++++- firebase_admin/messaging.py | 43 +++++++++++++++---------------- 2 files changed, 70 insertions(+), 24 deletions(-) diff --git a/firebase_admin/_utils.py b/firebase_admin/_utils.py index 5bfd79e81..239e5397d 100644 --- a/firebase_admin/_utils.py +++ b/firebase_admin/_utils.py @@ -15,7 +15,10 @@ """Internal utilities common to all modules.""" import json +import socket +import googleapiclient +import httplib2 import requests import firebase_admin @@ -116,17 +119,63 @@ def handle_requests_error(error, message=None, code=None): return err_type(message=message, cause=error, http_response=error.response) +def handle_googleapiclient_error(error, message=None, code=None): + """Constructs a ``FirebaseError`` from the given googleapiclient error. + + Args: + error: An error raised by the googleapiclient module while making an HTTP call. + message: A message to be included in the resulting ``FirebaseError`` (optional). If not + specified the string representation of the ``error`` argument is used as the message. + code: An HTTP status code or GCP error code that will be used to determine the resulting + error type (optional). If not specified the HTTP status code on the error response is + used. + + Returns: + FirebaseError: A ``FirebaseError`` that can be raised to the user code. + """ + if isinstance(error, socket.error) and 'timed out' in str(error): + return exceptions.DeadlineExceededError( + message='Timed out while making an API call: {0}'.format(error), + cause=error) + elif isinstance(error, httplib2.ServerNotFoundError): + return exceptions.UnavailableError( + message='Failed to establish a connection: {0}'.format(error), + cause=error) + elif not isinstance(error, googleapiclient.errors.HttpError): + return exceptions.UnknownError( + message='Unknown error while making a remote service call: {0}'.format(error), + cause=error) + + if not code: + code = error.resp.status + if not message: + message = str(error) + + resp = requests.models.Response() + resp.raw = error.content + resp.status_code = error.resp.status + err_type = lookup_error_type(code) + return err_type(message=message, cause=error, http_response=resp) + + def lookup_error_type(code): """Maps an error code to an exception type.""" return _ERROR_CODE_TO_EXCEPTION_TYPE.get(code, exceptions.UnknownError) -def parse_requests_platform_error(response, parse_func=None): +def parse_platform_error_from_requests(error, parse_func=None): + response = error.response content = response.content.decode() status_code = response.status_code return parse_platform_error(content, status_code, parse_func) +def parse_platform_error_from_googleapiclient(error, parse_func=None): + content = error.content.decode() + status_code = error.resp.status + return parse_platform_error(content, status_code, parse_func) + + def parse_platform_error(content, status_code, parse_func=None): """Parses an HTTP error response from a Google Cloud Platform API and extracts the error code and message fields. diff --git a/firebase_admin/messaging.py b/firebase_admin/messaging.py index 350abee83..9cf9222d5 100644 --- a/firebase_admin/messaging.py +++ b/firebase_admin/messaging.py @@ -363,7 +363,7 @@ def send(self, message, dry_run=False): timeout=self._timeout ) except requests.exceptions.RequestException as error: - self._handle_fcm_error(error) + raise self._handle_fcm_error(error) else: return resp['name'] @@ -379,7 +379,7 @@ def send_all(self, messages, dry_run=False): def batch_callback(_, response, error): exception = None if error: - exception = self._parse_batch_error(error) + exception = self._handle_batch_error(error) send_response = SendResponse(response, exception) responses.append(send_response) @@ -399,7 +399,7 @@ def batch_callback(_, response, error): try: batch.execute() except googleapiclient.http.HttpError as error: - raise self._parse_batch_error(error) + raise self._handle_batch_error(error) else: return BatchResponse(responses) @@ -453,14 +453,13 @@ def _handle_fcm_error(self, error): """Handles errors received from the FCM API.""" code, message = None, None if error.response is not None: - code, message = _utils.parse_requests_platform_error( - response=error.response, - parse_func=_MessagingService._parse_fcm_error) + code, message = _utils.parse_platform_error_from_requests( + error=error, parse_func=_MessagingService._parse_fcm_error) exc_type = _MessagingService.FCM_ERROR_TYPES.get(code) if exc_type: - raise exc_type(message, cause=error, http_response=error.response) + return exc_type(message, cause=error, http_response=error.response) - raise _utils.handle_requests_error(error, message=message, code=code) + return _utils.handle_requests_error(error, message=message, code=code) def _handle_iid_error(self, error): """Handles errors received from the Instance ID API.""" @@ -480,22 +479,20 @@ def _handle_iid_error(self, error): error.response.status_code, error.response.content.decode()) raise ApiCallError(code, msg, error) - def _parse_batch_error(self, error): + def _handle_batch_error(self, error): """Parses a googleapiclient.http.HttpError content and constructs a FirebaseError.""" - resp = requests.models.Response() - resp.raw = error.content - resp.status_code = error.resp.status - - code, msg = _utils.parse_platform_error( - content=error.content.decode(), - status_code=error.resp.status, - parse_func=_MessagingService._parse_fcm_error) - exc_type = _MessagingService.FCM_ERROR_TYPES.get(code) - if exc_type: - return exc_type(message=msg, cause=error, http_response=resp) - - err_type = _utils.lookup_error_type(code) - return err_type(message=msg, cause=error, http_response=resp) + code, message = None, None + if isinstance(error, googleapiclient.errors.HttpError): + code, message = _utils.parse_platform_error_from_googleapiclient( + error=error, parse_func=_MessagingService._parse_fcm_error) + exc_type = _MessagingService.FCM_ERROR_TYPES.get(code) + if exc_type: + resp = requests.models.Response() + resp.raw = error.content + resp.status_code = error.resp.status + return exc_type(message, cause=error, http_response=resp) + + return _utils.handle_googleapiclient_error(error, message=message, code=code) @classmethod def _parse_fcm_error(cls, error_dict): From 20d2e8735a96bd76dd97ac29e1fcb3ae140add6a Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Thu, 13 Jun 2019 14:00:26 -0700 Subject: [PATCH 06/11] Added tests for new error handling logic --- firebase_admin/_utils.py | 112 +++++++--- firebase_admin/messaging.py | 38 +--- tests/test_exceptions.py | 430 +++++++++++++++++++++++++++++------- tests/test_messaging.py | 26 ++- 4 files changed, 470 insertions(+), 136 deletions(-) diff --git a/firebase_admin/_utils.py b/firebase_admin/_utils.py index 239e5397d..e002aea29 100644 --- a/firebase_admin/_utils.py +++ b/firebase_admin/_utils.py @@ -20,6 +20,7 @@ import googleapiclient import httplib2 import requests +import six import firebase_admin from firebase_admin import exceptions @@ -84,6 +85,34 @@ def get_app_service(app, name, initializer): return app._get_service(name, initializer) # pylint: disable=protected-access +def handle_platform_error_from_requests(error, handle_func=None): + """Constructs a ``FirebaseError`` from the given requests error. + + This can be used to handle errors returned by Google Cloud Platform (GCP) APIs. + + Args: + error: An error raised by the reqests module while making an HTTP call to a GCP API. + handle_func: A function that can be used to handle platform errors in a custom way. When + specified, this function will be called with four arguments -- parsed error response, + error message, source exception from requests and the HTTP response object. + + Returns: + FirebaseError: A ``FirebaseError`` that can be raised to the user code. + """ + if error.response is None: + return handle_requests_error(error) + + response = error.response + content = response.content.decode() + status_code = response.status_code + error_dict, code, message = _parse_platform_error(content, status_code) + exc = None + if handle_func: + exc = handle_func(error_dict, message, error, error.response) + + return exc if exc else handle_requests_error(error, message, code) + + def handle_requests_error(error, message=None, code=None): """Constructs a ``FirebaseError`` from the given requests error. @@ -115,10 +144,39 @@ def handle_requests_error(error, message=None, code=None): code = error.response.status_code if not message: message = str(error) - err_type = lookup_error_type(code) + + err_type = _lookup_error_type(code) return err_type(message=message, cause=error, http_response=error.response) +def handle_platform_error_from_googleapiclient(error, handle_func=None): + """Constructs a ``FirebaseError`` from the given googleapiclient error. + + This can be used to handle errors returned by Google Cloud Platform (GCP) APIs. + + Args: + error: An error raised by the googleapiclient while making an HTTP call to a GCP API. + handle_func: A function that can be used to handle platform errors in a custom way. When + specified, this function will be called with four arguments -- parsed error response, + error message, source exception from googleapiclient and a HTTP response object. + + Returns: + FirebaseError: A ``FirebaseError`` that can be raised to the user code. + """ + if not isinstance(error, googleapiclient.errors.HttpError): + return handle_googleapiclient_error(error) + + content = error.content.decode() + status_code = error.resp.status + error_dict, code, message = _parse_platform_error(content, status_code) + exc = None + if handle_func: + http_response = _http_response_from_googleapiclient_error(error) + exc = handle_func(error_dict, message, error, http_response) + + return exc if exc else handle_googleapiclient_error(error, message, code) + + def handle_googleapiclient_error(error, message=None, code=None): """Constructs a ``FirebaseError`` from the given googleapiclient error. @@ -133,7 +191,8 @@ def handle_googleapiclient_error(error, message=None, code=None): Returns: FirebaseError: A ``FirebaseError`` that can be raised to the user code. """ - if isinstance(error, socket.error) and 'timed out' in str(error): + if isinstance(error, socket.timeout) or ( + isinstance(error, socket.error) and 'timed out' in str(error)): return exceptions.DeadlineExceededError( message='Timed out while making an API call: {0}'.format(error), cause=error) @@ -151,39 +210,31 @@ def handle_googleapiclient_error(error, message=None, code=None): if not message: message = str(error) + http_response = _http_response_from_googleapiclient_error(error) + err_type = _lookup_error_type(code) + return err_type(message=message, cause=error, http_response=http_response) + + +def _http_response_from_googleapiclient_error(error): + """Creates a requests HTTP Response object from the given googleapiclient error.""" resp = requests.models.Response() - resp.raw = error.content + resp.raw = six.BytesIO(error.content) resp.status_code = error.resp.status - err_type = lookup_error_type(code) - return err_type(message=message, cause=error, http_response=resp) + return resp -def lookup_error_type(code): +def _lookup_error_type(code): """Maps an error code to an exception type.""" return _ERROR_CODE_TO_EXCEPTION_TYPE.get(code, exceptions.UnknownError) -def parse_platform_error_from_requests(error, parse_func=None): - response = error.response - content = response.content.decode() - status_code = response.status_code - return parse_platform_error(content, status_code, parse_func) - - -def parse_platform_error_from_googleapiclient(error, parse_func=None): - content = error.content.decode() - status_code = error.resp.status - return parse_platform_error(content, status_code, parse_func) - - -def parse_platform_error(content, status_code, parse_func=None): +def _parse_platform_error(content, status_code): """Parses an HTTP error response from a Google Cloud Platform API and extracts the error code and message fields. Args: content: Decoded content of the response body. status_code: HTTP status code. - parse_func: A custom function to extract the code from the error body (optional). Returns: tuple: A tuple containing error code and message. @@ -197,20 +248,11 @@ def parse_platform_error(content, status_code, parse_func=None): pass error_dict = data.get('error', {}) - server_code = _get_error_code(error_dict, status_code, parse_func) + code = error_dict.get('status') + if not code: + code = _HTTP_STATUS_TO_ERROR_CODE.get(status_code, exceptions.UNKNOWN) + msg = error_dict.get('message') if not msg: msg = 'Unexpected HTTP response with status: {0}; body: {1}'.format(status_code, content) - return server_code, msg - - -def _get_error_code(error_dict, status_code, parse_func): - code = _try_get_error_code_from_body(error_dict, parse_func) - return code if code else _HTTP_STATUS_TO_ERROR_CODE.get(status_code, exceptions.UNKNOWN) - - -def _try_get_error_code_from_body(error_dict, parse_func): - code = None - if parse_func: - code = parse_func(error_dict) - return code if code else error_dict.get('status') + return error_dict, code, msg diff --git a/firebase_admin/messaging.py b/firebase_admin/messaging.py index 9cf9222d5..6a392b490 100644 --- a/firebase_admin/messaging.py +++ b/firebase_admin/messaging.py @@ -451,15 +451,8 @@ def _postproc(self, _, body): def _handle_fcm_error(self, error): """Handles errors received from the FCM API.""" - code, message = None, None - if error.response is not None: - code, message = _utils.parse_platform_error_from_requests( - error=error, parse_func=_MessagingService._parse_fcm_error) - exc_type = _MessagingService.FCM_ERROR_TYPES.get(code) - if exc_type: - return exc_type(message, cause=error, http_response=error.response) - - return _utils.handle_requests_error(error, message=message, code=code) + return _utils.handle_platform_error_from_requests( + error, _MessagingService._build_fcm_error) def _handle_iid_error(self, error): """Handles errors received from the Instance ID API.""" @@ -480,24 +473,17 @@ def _handle_iid_error(self, error): raise ApiCallError(code, msg, error) def _handle_batch_error(self, error): - """Parses a googleapiclient.http.HttpError content and constructs a FirebaseError.""" - code, message = None, None - if isinstance(error, googleapiclient.errors.HttpError): - code, message = _utils.parse_platform_error_from_googleapiclient( - error=error, parse_func=_MessagingService._parse_fcm_error) - exc_type = _MessagingService.FCM_ERROR_TYPES.get(code) - if exc_type: - resp = requests.models.Response() - resp.raw = error.content - resp.status_code = error.resp.status - return exc_type(message, cause=error, http_response=resp) - - return _utils.handle_googleapiclient_error(error, message=message, code=code) + """Handles errors received from the googleapiclient while making batch requests.""" + return _utils.handle_platform_error_from_googleapiclient( + error, _MessagingService._build_fcm_error) @classmethod - def _parse_fcm_error(cls, error_dict): - """Parses an error response from the FCM API and extracts the error code.""" + def _build_fcm_error(cls, error_dict, message, cause, http_response): + """Parses an error response from the FCM API and creates a FCM-specific exception if + appropriate.""" + fcm_code = None for detail in error_dict.get('details', []): if detail.get('@type') == 'type.googleapis.com/google.firebase.fcm.v1.FcmError': - return detail.get('errorCode') - return None + fcm_code = detail.get('errorCode') + exc_type = _MessagingService.FCM_ERROR_TYPES.get(fcm_code) + return exc_type(message, cause=cause, http_response=http_response) if exc_type else None diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index 0e4f3f916..447242811 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -12,85 +12,367 @@ # See the License for the specific language governing permissions and # limitations under the License. +import json +import socket +import httplib2 +import pytest import requests from requests import models +import six +from googleapiclient import errors from firebase_admin import exceptions from firebase_admin import _utils -def test_timeout_error(): - error = requests.exceptions.Timeout('Test error') - firebase_error = _utils.handle_requests_error(error) - assert isinstance(firebase_error, exceptions.DeadlineExceededError) - assert str(firebase_error) == 'Timed out while making an API call: Test error' - assert firebase_error.cause is error - assert firebase_error.http_response is None - -def test_connection_error(): - error = requests.exceptions.ConnectionError('Test error') - firebase_error = _utils.handle_requests_error(error) - assert isinstance(firebase_error, exceptions.UnavailableError) - assert str(firebase_error) == 'Failed to establish a connection: Test error' - assert firebase_error.cause is error - assert firebase_error.http_response is None - -def test_unknown_transport_error(): - error = requests.exceptions.RequestException('Test error') - firebase_error = _utils.handle_requests_error(error) - assert isinstance(firebase_error, exceptions.UnknownError) - assert str(firebase_error) == 'Unknown error while making a remote service call: Test error' - assert firebase_error.cause is error - assert firebase_error.http_response is None - -def test_http_response(): - resp = models.Response() - resp.status_code = 500 - error = requests.exceptions.RequestException('Test error', response=resp) - firebase_error = _utils.handle_requests_error(error) - assert isinstance(firebase_error, exceptions.InternalError) - assert str(firebase_error) == 'Test error' - assert firebase_error.cause is error - assert firebase_error.http_response is resp - -def test_http_response_with_unknown_status(): - resp = models.Response() - resp.status_code = 501 - error = requests.exceptions.RequestException('Test error', response=resp) - firebase_error = _utils.handle_requests_error(error) - assert isinstance(firebase_error, exceptions.UnknownError) - assert str(firebase_error) == 'Test error' - assert firebase_error.cause is error - assert firebase_error.http_response is resp - -def test_http_response_with_message(): - resp = models.Response() - resp.status_code = 500 - error = requests.exceptions.RequestException('Test error', response=resp) - firebase_error = _utils.handle_requests_error(error, message='Explicit error message') - assert isinstance(firebase_error, exceptions.InternalError) - assert str(firebase_error) == 'Explicit error message' - assert firebase_error.cause is error - assert firebase_error.http_response is resp - -def test_http_response_with_status(): - resp = models.Response() - resp.status_code = 500 - error = requests.exceptions.RequestException('Test error', response=resp) - firebase_error = _utils.handle_requests_error(error, code=503) - assert isinstance(firebase_error, exceptions.UnavailableError) - assert str(firebase_error) == 'Test error' - assert firebase_error.cause is error - assert firebase_error.http_response is resp - -def test_http_response_with_message_and_status(): - resp = models.Response() - resp.status_code = 500 - error = requests.exceptions.RequestException('Test error', response=resp) - firebase_error = _utils.handle_requests_error( - error, message='Explicit error message', code=503) - assert isinstance(firebase_error, exceptions.UnavailableError) - assert str(firebase_error) == 'Explicit error message' - assert firebase_error.cause is error - assert firebase_error.http_response is resp +class TestRequests(object): + + def test_timeout_error(self): + error = requests.exceptions.Timeout('Test error') + firebase_error = _utils.handle_requests_error(error) + assert isinstance(firebase_error, exceptions.DeadlineExceededError) + assert str(firebase_error) == 'Timed out while making an API call: Test error' + assert firebase_error.cause is error + assert firebase_error.http_response is None + + def test_requests_connection_error(self): + error = requests.exceptions.ConnectionError('Test error') + firebase_error = _utils.handle_requests_error(error) + assert isinstance(firebase_error, exceptions.UnavailableError) + assert str(firebase_error) == 'Failed to establish a connection: Test error' + assert firebase_error.cause is error + assert firebase_error.http_response is None + + def test_unknown_transport_error(self): + error = requests.exceptions.RequestException('Test error') + firebase_error = _utils.handle_requests_error(error) + assert isinstance(firebase_error, exceptions.UnknownError) + assert str(firebase_error) == 'Unknown error while making a remote service call: Test error' + assert firebase_error.cause is error + assert firebase_error.http_response is None + + def test_http_response(self): + resp, error = self._create_response() + firebase_error = _utils.handle_requests_error(error) + assert isinstance(firebase_error, exceptions.InternalError) + assert str(firebase_error) == 'Test error' + assert firebase_error.cause is error + assert firebase_error.http_response is resp + + def test_http_response_with_unknown_status(self): + resp, error = self._create_response(status=501) + firebase_error = _utils.handle_requests_error(error) + assert isinstance(firebase_error, exceptions.UnknownError) + assert str(firebase_error) == 'Test error' + assert firebase_error.cause is error + assert firebase_error.http_response is resp + + def test_http_response_with_message(self): + resp, error = self._create_response() + firebase_error = _utils.handle_requests_error(error, message='Explicit error message') + assert isinstance(firebase_error, exceptions.InternalError) + assert str(firebase_error) == 'Explicit error message' + assert firebase_error.cause is error + assert firebase_error.http_response is resp + + def test_http_response_with_status(self): + resp, error = self._create_response() + firebase_error = _utils.handle_requests_error(error, code=503) + assert isinstance(firebase_error, exceptions.UnavailableError) + assert str(firebase_error) == 'Test error' + assert firebase_error.cause is error + assert firebase_error.http_response is resp + + def test_http_response_with_message_and_status(self): + resp, error = self._create_response() + firebase_error = _utils.handle_requests_error( + error, message='Explicit error message', code=503) + assert isinstance(firebase_error, exceptions.UnavailableError) + assert str(firebase_error) == 'Explicit error message' + assert firebase_error.cause is error + assert firebase_error.http_response is resp + + def test_handle_platform_error(self): + payload = json.dumps({ + 'error': { + 'status': 'NOT_FOUND', + 'message': 'test error' + } + }) + resp, error = self._create_response(payload=payload) + firebase_error = _utils.handle_platform_error_from_requests(error) + assert isinstance(firebase_error, exceptions.NotFoundError) + assert str(firebase_error) == 'test error' + assert firebase_error.cause is error + assert firebase_error.http_response is resp + + def test_handle_platform_error_with_no_response(self): + error = requests.exceptions.RequestException('Test error') + firebase_error = _utils.handle_platform_error_from_requests(error) + assert isinstance(firebase_error, exceptions.UnknownError) + assert str(firebase_error) == 'Unknown error while making a remote service call: Test error' + assert firebase_error.cause is error + assert firebase_error.http_response is None + + def test_handle_platform_error_with_no_error_code(self): + resp, error = self._create_response(payload='no error code') + firebase_error = _utils.handle_platform_error_from_requests(error) + assert isinstance(firebase_error, exceptions.InternalError) + message = 'Unexpected HTTP response with status: 500; body: no error code' + assert str(firebase_error) == message + assert firebase_error.cause is error + assert firebase_error.http_response is resp + + def test_handle_platform_error_with_custom_handler(self): + payload = json.dumps({ + 'error': { + 'status': 'NOT_FOUND', + 'message': 'test error' + } + }) + resp, error = self._create_response(payload=payload) + invocations = [] + + def _custom_handler(error_dict, message, cause, http_response): + invocations.append((error_dict, message, cause, http_response)) + return exceptions.InvalidArgumentError('Custom message', cause, http_response) + + firebase_error = _utils.handle_platform_error_from_requests(error, _custom_handler) + + assert isinstance(firebase_error, exceptions.InvalidArgumentError) + assert str(firebase_error) == 'Custom message' + assert firebase_error.cause is error + assert firebase_error.http_response is resp + assert len(invocations) == 1 + args = invocations[0] + assert len(args) == 4 + assert args[0] == { + 'status': 'NOT_FOUND', + 'message': 'test error' + } + assert args[1] == 'test error' + assert args[2] is error + assert args[3] is resp + + def test_handle_platform_error_with_custom_handler_ignore(self): + payload = json.dumps({ + 'error': { + 'status': 'NOT_FOUND', + 'message': 'test error' + } + }) + resp, error = self._create_response(payload=payload) + invocations = [] + + def _custom_handler(error_dict, message, cause, http_response): + invocations.append((error_dict, message, cause, http_response)) + return None + + firebase_error = _utils.handle_platform_error_from_requests(error, _custom_handler) + + assert isinstance(firebase_error, exceptions.NotFoundError) + assert str(firebase_error) == 'test error' + assert firebase_error.cause is error + assert firebase_error.http_response is resp + assert len(invocations) == 1 + args = invocations[0] + assert len(args) == 4 + assert args[0] == { + 'status': 'NOT_FOUND', + 'message': 'test error' + } + assert args[1] == 'test error' + assert args[2] is error + assert args[3] is resp + + def _create_response(self, status=500, payload=None): + resp = models.Response() + resp.status_code = status + if payload: + resp.raw = six.BytesIO(payload.encode()) + exc = requests.exceptions.RequestException('Test error', response=resp) + return resp, exc + + +class TestGoogleApiClient(object): + + @pytest.mark.parametrize('error', [ + socket.timeout('Test error'), + socket.error('Read timed out') + ]) + def test_googleapicleint_timeout_error(self, error): + firebase_error = _utils.handle_googleapiclient_error(error) + assert isinstance(firebase_error, exceptions.DeadlineExceededError) + assert str(firebase_error) == 'Timed out while making an API call: {0}'.format(error) + assert firebase_error.cause is error + assert firebase_error.http_response is None + + def test_googleapiclient_connection_error(self): + error = httplib2.ServerNotFoundError('Test error') + firebase_error = _utils.handle_googleapiclient_error(error) + assert isinstance(firebase_error, exceptions.UnavailableError) + assert str(firebase_error) == 'Failed to establish a connection: Test error' + assert firebase_error.cause is error + assert firebase_error.http_response is None + + def test_unknown_transport_error(self): + error = socket.error('Test error') + firebase_error = _utils.handle_googleapiclient_error(error) + assert isinstance(firebase_error, exceptions.UnknownError) + assert str(firebase_error) == 'Unknown error while making a remote service call: Test error' + assert firebase_error.cause is error + assert firebase_error.http_response is None + + def test_http_response(self): + resp = httplib2.Response({'status': 500}) + error = errors.HttpError(resp, 'Body') + firebase_error = _utils.handle_googleapiclient_error(error) + assert isinstance(firebase_error, exceptions.InternalError) + assert str(firebase_error) == str(error) + assert firebase_error.cause is error + assert firebase_error.http_response.status_code == 500 + assert firebase_error.http_response.content.decode() == 'Body' + + def test_http_response_with_unknown_status(self): + resp = httplib2.Response({'status': 501}) + error = errors.HttpError(resp, 'Body') + firebase_error = _utils.handle_googleapiclient_error(error) + assert isinstance(firebase_error, exceptions.UnknownError) + assert str(firebase_error) == str(error) + assert firebase_error.cause is error + assert firebase_error.http_response.status_code == 501 + assert firebase_error.http_response.content.decode() == 'Body' + + def test_http_response_with_message(self): + resp = httplib2.Response({'status': 500}) + error = errors.HttpError(resp, 'Body') + firebase_error = _utils.handle_googleapiclient_error( + error, message='Explicit error message') + assert isinstance(firebase_error, exceptions.InternalError) + assert str(firebase_error) == 'Explicit error message' + assert firebase_error.cause is error + assert firebase_error.http_response.status_code == 500 + assert firebase_error.http_response.content.decode() == 'Body' + + def test_http_response_with_status(self): + resp = httplib2.Response({'status': 500}) + error = errors.HttpError(resp, 'Body') + firebase_error = _utils.handle_googleapiclient_error(error, code=503) + assert isinstance(firebase_error, exceptions.UnavailableError) + assert str(firebase_error) == str(error) + assert firebase_error.cause is error + assert firebase_error.http_response.status_code == 500 + assert firebase_error.http_response.content.decode() == 'Body' + + def test_http_response_with_message_and_status(self): + resp = httplib2.Response({'status': 500}) + error = errors.HttpError(resp, 'Body') + firebase_error = _utils.handle_googleapiclient_error( + error, message='Explicit error message', code=503) + assert isinstance(firebase_error, exceptions.UnavailableError) + assert str(firebase_error) == 'Explicit error message' + assert firebase_error.cause is error + assert firebase_error.http_response.status_code == 500 + assert firebase_error.http_response.content.decode() == 'Body' + + def test_handle_platform_error(self): + payload = json.dumps({ + 'error': { + 'status': 'NOT_FOUND', + 'message': 'test error' + } + }) + resp = httplib2.Response({'status': 500}) + error = errors.HttpError(resp, payload) + firebase_error = _utils.handle_platform_error_from_googleapiclient(error) + assert isinstance(firebase_error, exceptions.NotFoundError) + assert str(firebase_error) == 'test error' + assert firebase_error.cause is error + assert firebase_error.http_response.status_code == 500 + assert firebase_error.http_response.content.decode() == payload + + def test_handle_platform_error_with_no_response(self): + error = socket.error('Test error') + firebase_error = _utils.handle_platform_error_from_googleapiclient(error) + assert isinstance(firebase_error, exceptions.UnknownError) + assert str(firebase_error) == 'Unknown error while making a remote service call: Test error' + assert firebase_error.cause is error + assert firebase_error.http_response is None + + def test_handle_platform_error_with_no_error_code(self): + resp = httplib2.Response({'status': 500}) + error = errors.HttpError(resp, 'no error code') + firebase_error = _utils.handle_platform_error_from_googleapiclient(error) + assert isinstance(firebase_error, exceptions.InternalError) + message = 'Unexpected HTTP response with status: 500; body: no error code' + assert str(firebase_error) == message + assert firebase_error.cause is error + assert firebase_error.http_response.status_code == 500 + assert firebase_error.http_response.content.decode() == 'no error code' + + def test_handle_platform_error_with_custom_handler(self): + payload = json.dumps({ + 'error': { + 'status': 'NOT_FOUND', + 'message': 'test error' + } + }) + resp = httplib2.Response({'status': 500}) + error = errors.HttpError(resp, payload) + invocations = [] + + def _custom_handler(error_dict, message, cause, http_response): + invocations.append((error_dict, message, cause, http_response)) + return exceptions.InvalidArgumentError('Custom message', cause, http_response) + + firebase_error = _utils.handle_platform_error_from_googleapiclient(error, _custom_handler) + + assert isinstance(firebase_error, exceptions.InvalidArgumentError) + assert str(firebase_error) == 'Custom message' + assert firebase_error.cause is error + assert firebase_error.http_response.status_code == 500 + assert firebase_error.http_response.content.decode() == payload + assert len(invocations) == 1 + args = invocations[0] + assert len(args) == 4 + assert args[0] == { + 'status': 'NOT_FOUND', + 'message': 'test error' + } + assert args[1] == 'test error' + assert args[2] is error + assert args[3] is not None + + def test_handle_platform_error_with_custom_handler_ignore(self): + payload = json.dumps({ + 'error': { + 'status': 'NOT_FOUND', + 'message': 'test error' + } + }) + resp = httplib2.Response({'status': 500}) + error = errors.HttpError(resp, payload) + invocations = [] + + def _custom_handler(error_dict, message, cause, http_response): + invocations.append((error_dict, message, cause, http_response)) + return None + + firebase_error = _utils.handle_platform_error_from_googleapiclient(error, _custom_handler) + + assert isinstance(firebase_error, exceptions.NotFoundError) + assert str(firebase_error) == 'test error' + assert firebase_error.cause is error + assert firebase_error.http_response.status_code == 500 + assert firebase_error.http_response.content.decode() == payload + assert len(invocations) == 1 + args = invocations[0] + assert len(args) == 4 + assert args[0] == { + 'status': 'NOT_FOUND', + 'message': 'test error' + } + assert args[1] == 'test error' + assert args[2] is error + assert args[3] is not None diff --git a/tests/test_messaging.py b/tests/test_messaging.py index 8bb07869f..cf99c36ba 100644 --- a/tests/test_messaging.py +++ b/tests/test_messaging.py @@ -45,7 +45,6 @@ 'SENDER_ID_MISMATCH': messaging.SenderIdMismatchError, 'THIRD_PARTY_AUTH_ERROR': messaging.ThirdPartyAuthError, 'UNREGISTERED': messaging.UnregisteredError, - 'SOMETHING_NEW': exceptions.UnknownError, } @@ -1360,6 +1359,31 @@ def test_send_fcm_error_code(self, status, fcm_error_code, exc_type): body = {'message': messaging._MessagingService.JSON_ENCODER.default(msg)} assert json.loads(recorder[0].body.decode()) == body + @pytest.mark.parametrize('status', HTTP_ERROR_CODES) + def test_send_unknown_fcm_error_code(self, status): + payload = json.dumps({ + 'error': { + 'status': 'INVALID_ARGUMENT', + 'message': 'test error', + 'details': [ + { + '@type': 'type.googleapis.com/google.firebase.fcm.v1.FcmError', + 'errorCode': 'SOME_UNKNOWN_CODE', + }, + ], + } + }) + _, recorder = self._instrument_messaging_service(status=status, payload=payload) + msg = messaging.Message(topic='foo') + with pytest.raises(exceptions.InvalidArgumentError) as excinfo: + messaging.send(msg) + check_exception(excinfo.value, 'test error', status) + assert len(recorder) == 1 + assert recorder[0].method == 'POST' + assert recorder[0].url == self._get_url('explicit-project-id') + body = {'message': messaging._MessagingService.JSON_ENCODER.default(msg)} + assert json.loads(recorder[0].body.decode()) == body + class TestBatch(object): From c0cf6f7cc28058f82a71ffa97125af76b47b79e6 Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Thu, 13 Jun 2019 14:05:18 -0700 Subject: [PATCH 07/11] Updated public API docs --- firebase_admin/messaging.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/firebase_admin/messaging.py b/firebase_admin/messaging.py index 6a392b490..bf29743de 100644 --- a/firebase_admin/messaging.py +++ b/firebase_admin/messaging.py @@ -99,7 +99,7 @@ def send(message, dry_run=False, app=None): string: A message ID string that uniquely identifies the sent the message. Raises: - ApiCallError: If an error occurs while sending the message to the FCM service. + FirebaseError: If an error occurs while sending the message to the FCM service. ValueError: If the input arguments are invalid. """ return _get_messaging_service(app).send(message, dry_run) @@ -119,7 +119,7 @@ def send_all(messages, dry_run=False, app=None): BatchResponse: A ``messaging.BatchResponse`` instance. Raises: - ApiCallError: If an error occurs while sending the message to the FCM service. + FirebaseError: If an error occurs while sending the message to the FCM service. ValueError: If the input arguments are invalid. """ return _get_messaging_service(app).send_all(messages, dry_run) @@ -139,7 +139,7 @@ def send_multicast(multicast_message, dry_run=False, app=None): BatchResponse: A ``messaging.BatchResponse`` instance. Raises: - ApiCallError: If an error occurs while sending the message to the FCM service. + FirebaseError: If an error occurs while sending the message to the FCM service. ValueError: If the input arguments are invalid. """ if not isinstance(multicast_message, MulticastMessage): From 0ea0f82dda5b5a317d361dd7dffe39f6adcf30d7 Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Thu, 13 Jun 2019 14:28:40 -0700 Subject: [PATCH 08/11] Fixing test for python3 --- tests/test_exceptions.py | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index 447242811..ef55c98dc 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -225,8 +225,7 @@ def test_unknown_transport_error(self): assert firebase_error.http_response is None def test_http_response(self): - resp = httplib2.Response({'status': 500}) - error = errors.HttpError(resp, 'Body') + error = self._create_http_error() firebase_error = _utils.handle_googleapiclient_error(error) assert isinstance(firebase_error, exceptions.InternalError) assert str(firebase_error) == str(error) @@ -235,8 +234,7 @@ def test_http_response(self): assert firebase_error.http_response.content.decode() == 'Body' def test_http_response_with_unknown_status(self): - resp = httplib2.Response({'status': 501}) - error = errors.HttpError(resp, 'Body') + error = self._create_http_error(status=501) firebase_error = _utils.handle_googleapiclient_error(error) assert isinstance(firebase_error, exceptions.UnknownError) assert str(firebase_error) == str(error) @@ -245,8 +243,7 @@ def test_http_response_with_unknown_status(self): assert firebase_error.http_response.content.decode() == 'Body' def test_http_response_with_message(self): - resp = httplib2.Response({'status': 500}) - error = errors.HttpError(resp, 'Body') + error = self._create_http_error() firebase_error = _utils.handle_googleapiclient_error( error, message='Explicit error message') assert isinstance(firebase_error, exceptions.InternalError) @@ -256,8 +253,7 @@ def test_http_response_with_message(self): assert firebase_error.http_response.content.decode() == 'Body' def test_http_response_with_status(self): - resp = httplib2.Response({'status': 500}) - error = errors.HttpError(resp, 'Body') + error = self._create_http_error() firebase_error = _utils.handle_googleapiclient_error(error, code=503) assert isinstance(firebase_error, exceptions.UnavailableError) assert str(firebase_error) == str(error) @@ -266,8 +262,7 @@ def test_http_response_with_status(self): assert firebase_error.http_response.content.decode() == 'Body' def test_http_response_with_message_and_status(self): - resp = httplib2.Response({'status': 500}) - error = errors.HttpError(resp, 'Body') + error = self._create_http_error() firebase_error = _utils.handle_googleapiclient_error( error, message='Explicit error message', code=503) assert isinstance(firebase_error, exceptions.UnavailableError) @@ -283,8 +278,7 @@ def test_handle_platform_error(self): 'message': 'test error' } }) - resp = httplib2.Response({'status': 500}) - error = errors.HttpError(resp, payload) + error = self._create_http_error(payload=payload) firebase_error = _utils.handle_platform_error_from_googleapiclient(error) assert isinstance(firebase_error, exceptions.NotFoundError) assert str(firebase_error) == 'test error' @@ -301,8 +295,7 @@ def test_handle_platform_error_with_no_response(self): assert firebase_error.http_response is None def test_handle_platform_error_with_no_error_code(self): - resp = httplib2.Response({'status': 500}) - error = errors.HttpError(resp, 'no error code') + error = self._create_http_error(payload='no error code') firebase_error = _utils.handle_platform_error_from_googleapiclient(error) assert isinstance(firebase_error, exceptions.InternalError) message = 'Unexpected HTTP response with status: 500; body: no error code' @@ -318,8 +311,7 @@ def test_handle_platform_error_with_custom_handler(self): 'message': 'test error' } }) - resp = httplib2.Response({'status': 500}) - error = errors.HttpError(resp, payload) + error = self._create_http_error(payload=payload) invocations = [] def _custom_handler(error_dict, message, cause, http_response): @@ -351,8 +343,7 @@ def test_handle_platform_error_with_custom_handler_ignore(self): 'message': 'test error' } }) - resp = httplib2.Response({'status': 500}) - error = errors.HttpError(resp, payload) + error = self._create_http_error(payload=payload) invocations = [] def _custom_handler(error_dict, message, cause, http_response): @@ -376,3 +367,7 @@ def _custom_handler(error_dict, message, cause, http_response): assert args[1] == 'test error' assert args[2] is error assert args[3] is not None + + def _create_http_error(self, status=500, payload='Body'): + resp = httplib2.Response({'status': status}) + return errors.HttpError(resp, payload.encode()) From 130a42f8549266c473ae6fe2d6e751697e7123bc Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Fri, 14 Jun 2019 10:42:36 -0700 Subject: [PATCH 09/11] Cleaning up the error code lookup code --- firebase_admin/_utils.py | 45 +++++++++++++++++++--------------------- tests/test_exceptions.py | 16 +++++++------- 2 files changed, 29 insertions(+), 32 deletions(-) diff --git a/firebase_admin/_utils.py b/firebase_admin/_utils.py index e002aea29..805d6c647 100644 --- a/firebase_admin/_utils.py +++ b/firebase_admin/_utils.py @@ -27,15 +27,6 @@ _ERROR_CODE_TO_EXCEPTION_TYPE = { - 400: exceptions.InvalidArgumentError, - 401: exceptions.UnauthenticatedError, - 403: exceptions.PermissionDeniedError, - 404: exceptions.NotFoundError, - 409: exceptions.ConflictError, - 429: exceptions.ResourceExhaustedError, - 500: exceptions.InternalError, - 503: exceptions.UnavailableError, - exceptions.INVALID_ARGUMENT: exceptions.InvalidArgumentError, exceptions.FAILED_PRECONDITION: exceptions.FailedPreconditionError, exceptions.OUT_OF_RANGE: exceptions.OutOfRangeError, @@ -44,6 +35,7 @@ exceptions.NOT_FOUND: exceptions.NotFoundError, exceptions.ABORTED: exceptions.AbortedError, exceptions.ALREADY_EXISTS: exceptions.AlreadyExistsError, + exceptions.CONFLICT: exceptions.ConflictError, exceptions.RESOURCE_EXHAUSTED: exceptions.ResourceExhaustedError, exceptions.CANCELLED: exceptions.CancelledError, exceptions.DATA_LOSS: exceptions.DataLossError, @@ -91,7 +83,7 @@ def handle_platform_error_from_requests(error, handle_func=None): This can be used to handle errors returned by Google Cloud Platform (GCP) APIs. Args: - error: An error raised by the reqests module while making an HTTP call to a GCP API. + error: An error raised by the requests module while making an HTTP call to a GCP API. handle_func: A function that can be used to handle platform errors in a custom way. When specified, this function will be called with four arguments -- parsed error response, error message, source exception from requests and the HTTP response object. @@ -117,12 +109,12 @@ def handle_requests_error(error, message=None, code=None): """Constructs a ``FirebaseError`` from the given requests error. Args: - error: An error raised by the reqests module while making an HTTP call. + error: An error raised by the requests module while making an HTTP call. message: A message to be included in the resulting ``FirebaseError`` (optional). If not specified the string representation of the ``error`` argument is used as the message. - code: An HTTP status code or GCP error code that will be used to determine the resulting - error type (optional). If not specified the HTTP status code on the error response is - used. + code: A GCP error code that will be used to determine the resulting error type (optional). + If not specified the HTTP status code on the error response is used to determine a + suitable error code. Returns: FirebaseError: A ``FirebaseError`` that can be raised to the user code. @@ -141,11 +133,11 @@ def handle_requests_error(error, message=None, code=None): cause=error) if not code: - code = error.response.status_code + code = _http_status_to_error_code(error.response.status_code) if not message: message = str(error) - err_type = _lookup_error_type(code) + err_type = _error_code_to_exception_type(code) return err_type(message=message, cause=error, http_response=error.response) @@ -184,9 +176,9 @@ def handle_googleapiclient_error(error, message=None, code=None): error: An error raised by the googleapiclient module while making an HTTP call. message: A message to be included in the resulting ``FirebaseError`` (optional). If not specified the string representation of the ``error`` argument is used as the message. - code: An HTTP status code or GCP error code that will be used to determine the resulting - error type (optional). If not specified the HTTP status code on the error response is - used. + code: A GCP error code that will be used to determine the resulting error type (optional). + If not specified the HTTP status code on the error response is used to determine a + suitable error code. Returns: FirebaseError: A ``FirebaseError`` that can be raised to the user code. @@ -206,12 +198,12 @@ def handle_googleapiclient_error(error, message=None, code=None): cause=error) if not code: - code = error.resp.status + code = _http_status_to_error_code(error.resp.status) if not message: message = str(error) http_response = _http_response_from_googleapiclient_error(error) - err_type = _lookup_error_type(code) + err_type = _error_code_to_exception_type(code) return err_type(message=message, cause=error, http_response=http_response) @@ -223,8 +215,13 @@ def _http_response_from_googleapiclient_error(error): return resp -def _lookup_error_type(code): - """Maps an error code to an exception type.""" +def _http_status_to_error_code(status): + """Maps an HTTP status to a platform error code.""" + return _HTTP_STATUS_TO_ERROR_CODE.get(status, exceptions.UNKNOWN) + + +def _error_code_to_exception_type(code): + """Maps a platform error code to an exception type.""" return _ERROR_CODE_TO_EXCEPTION_TYPE.get(code, exceptions.UnknownError) @@ -250,7 +247,7 @@ def _parse_platform_error(content, status_code): error_dict = data.get('error', {}) code = error_dict.get('status') if not code: - code = _HTTP_STATUS_TO_ERROR_CODE.get(status_code, exceptions.UNKNOWN) + code = _http_status_to_error_code(status_code) msg = error_dict.get('message') if not msg: diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index ef55c98dc..32e6c6659 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -76,18 +76,18 @@ def test_http_response_with_message(self): assert firebase_error.cause is error assert firebase_error.http_response is resp - def test_http_response_with_status(self): + def test_http_response_with_code(self): resp, error = self._create_response() - firebase_error = _utils.handle_requests_error(error, code=503) + firebase_error = _utils.handle_requests_error(error, code=exceptions.UNAVAILABLE) assert isinstance(firebase_error, exceptions.UnavailableError) assert str(firebase_error) == 'Test error' assert firebase_error.cause is error assert firebase_error.http_response is resp - def test_http_response_with_message_and_status(self): + def test_http_response_with_message_and_code(self): resp, error = self._create_response() firebase_error = _utils.handle_requests_error( - error, message='Explicit error message', code=503) + error, message='Explicit error message', code=exceptions.UNAVAILABLE) assert isinstance(firebase_error, exceptions.UnavailableError) assert str(firebase_error) == 'Explicit error message' assert firebase_error.cause is error @@ -252,19 +252,19 @@ def test_http_response_with_message(self): assert firebase_error.http_response.status_code == 500 assert firebase_error.http_response.content.decode() == 'Body' - def test_http_response_with_status(self): + def test_http_response_with_code(self): error = self._create_http_error() - firebase_error = _utils.handle_googleapiclient_error(error, code=503) + firebase_error = _utils.handle_googleapiclient_error(error, code=exceptions.UNAVAILABLE) assert isinstance(firebase_error, exceptions.UnavailableError) assert str(firebase_error) == str(error) assert firebase_error.cause is error assert firebase_error.http_response.status_code == 500 assert firebase_error.http_response.content.decode() == 'Body' - def test_http_response_with_message_and_status(self): + def test_http_response_with_message_and_code(self): error = self._create_http_error() firebase_error = _utils.handle_googleapiclient_error( - error, message='Explicit error message', code=503) + error, message='Explicit error message', code=exceptions.UNAVAILABLE) assert isinstance(firebase_error, exceptions.UnavailableError) assert str(firebase_error) == 'Explicit error message' assert firebase_error.cause is error From de29a6b02a82d2ffcd0b7b34f4f21ee14c841584 Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Fri, 14 Jun 2019 11:54:10 -0700 Subject: [PATCH 10/11] Cleaning up the error parsing APIs --- firebase_admin/_utils.py | 47 +++++++------ firebase_admin/messaging.py | 23 +++++-- tests/test_exceptions.py | 131 ++++++++++++++---------------------- 3 files changed, 93 insertions(+), 108 deletions(-) diff --git a/firebase_admin/_utils.py b/firebase_admin/_utils.py index 805d6c647..089408fd4 100644 --- a/firebase_admin/_utils.py +++ b/firebase_admin/_utils.py @@ -97,24 +97,23 @@ def handle_platform_error_from_requests(error, handle_func=None): response = error.response content = response.content.decode() status_code = response.status_code - error_dict, code, message = _parse_platform_error(content, status_code) + error_dict, message = _parse_platform_error(content, status_code) exc = None if handle_func: - exc = handle_func(error_dict, message, error, error.response) + exc = handle_func(error, message, error_dict) - return exc if exc else handle_requests_error(error, message, code) + return exc if exc else handle_requests_error(error, message, error_dict) -def handle_requests_error(error, message=None, code=None): +def handle_requests_error(error, message=None, error_dict=None): """Constructs a ``FirebaseError`` from the given requests error. Args: error: An error raised by the requests module while making an HTTP call. message: A message to be included in the resulting ``FirebaseError`` (optional). If not specified the string representation of the ``error`` argument is used as the message. - code: A GCP error code that will be used to determine the resulting error type (optional). - If not specified the HTTP status code on the error response is used to determine a - suitable error code. + error_dict: A parsed error payload to extract error code from (optional). If not specified + parses the response payload available in the ``error``. Returns: FirebaseError: A ``FirebaseError`` that can be raised to the user code. @@ -132,6 +131,10 @@ def handle_requests_error(error, message=None, code=None): message='Unknown error while making a remote service call: {0}'.format(error), cause=error) + if error_dict is None: + error_dict = {} + + code = error_dict.get('status') if not code: code = _http_status_to_error_code(error.response.status_code) if not message: @@ -160,25 +163,26 @@ def handle_platform_error_from_googleapiclient(error, handle_func=None): content = error.content.decode() status_code = error.resp.status - error_dict, code, message = _parse_platform_error(content, status_code) + error_dict, message = _parse_platform_error(content, status_code) + http_response = _http_response_from_googleapiclient_error(error) exc = None if handle_func: - http_response = _http_response_from_googleapiclient_error(error) - exc = handle_func(error_dict, message, error, http_response) + exc = handle_func(error, message, error_dict, http_response) - return exc if exc else handle_googleapiclient_error(error, message, code) + return exc if exc else handle_googleapiclient_error(error, message, error_dict, http_response) -def handle_googleapiclient_error(error, message=None, code=None): +def handle_googleapiclient_error(error, message=None, error_dict=None, http_response=None): """Constructs a ``FirebaseError`` from the given googleapiclient error. Args: error: An error raised by the googleapiclient module while making an HTTP call. message: A message to be included in the resulting ``FirebaseError`` (optional). If not specified the string representation of the ``error`` argument is used as the message. - code: A GCP error code that will be used to determine the resulting error type (optional). - If not specified the HTTP status code on the error response is used to determine a - suitable error code. + error_dict: A parsed error payload to extract error code from (optional). If not specified + the error code will be determined from the HTTP status code of the response. + http_response: A requests HTTP response object to associate with the exception (optional). + If not specified, one will be created from the ``error``. Returns: FirebaseError: A ``FirebaseError`` that can be raised to the user code. @@ -197,12 +201,17 @@ def handle_googleapiclient_error(error, message=None, code=None): message='Unknown error while making a remote service call: {0}'.format(error), cause=error) + if error_dict is None: + error_dict = {} + + code = error_dict.get('status') if not code: code = _http_status_to_error_code(error.resp.status) if not message: message = str(error) + if not http_response: + http_response = _http_response_from_googleapiclient_error(error) - http_response = _http_response_from_googleapiclient_error(error) err_type = _error_code_to_exception_type(code) return err_type(message=message, cause=error, http_response=http_response) @@ -245,11 +254,7 @@ def _parse_platform_error(content, status_code): pass error_dict = data.get('error', {}) - code = error_dict.get('status') - if not code: - code = _http_status_to_error_code(status_code) - msg = error_dict.get('message') if not msg: msg = 'Unexpected HTTP response with status: {0}; body: {1}'.format(status_code, content) - return error_dict, code, msg + return error_dict, msg diff --git a/firebase_admin/messaging.py b/firebase_admin/messaging.py index bf29743de..d15cd5868 100644 --- a/firebase_admin/messaging.py +++ b/firebase_admin/messaging.py @@ -452,7 +452,7 @@ def _postproc(self, _, body): def _handle_fcm_error(self, error): """Handles errors received from the FCM API.""" return _utils.handle_platform_error_from_requests( - error, _MessagingService._build_fcm_error) + error, _MessagingService._build_fcm_error_requests) def _handle_iid_error(self, error): """Handles errors received from the Instance ID API.""" @@ -475,15 +475,28 @@ def _handle_iid_error(self, error): def _handle_batch_error(self, error): """Handles errors received from the googleapiclient while making batch requests.""" return _utils.handle_platform_error_from_googleapiclient( - error, _MessagingService._build_fcm_error) + error, _MessagingService._build_fcm_error_googleapiclient) @classmethod - def _build_fcm_error(cls, error_dict, message, cause, http_response): + def _build_fcm_error_requests(cls, error, message, error_dict): """Parses an error response from the FCM API and creates a FCM-specific exception if appropriate.""" + exc_type = cls._build_fcm_error(error_dict) + return exc_type(message, cause=error, http_response=error.response) if exc_type else None + + @classmethod + def _build_fcm_error_googleapiclient(cls, error, message, error_dict, http_response): + """Parses an error response from the FCM API and creates a FCM-specific exception if + appropriate.""" + exc_type = cls._build_fcm_error(error_dict) + return exc_type(message, cause=error, http_response=http_response) if exc_type else None + + @classmethod + def _build_fcm_error(cls, error_dict): + if not error_dict: + return None fcm_code = None for detail in error_dict.get('details', []): if detail.get('@type') == 'type.googleapis.com/google.firebase.fcm.v1.FcmError': fcm_code = detail.get('errorCode') - exc_type = _MessagingService.FCM_ERROR_TYPES.get(fcm_code) - return exc_type(message, cause=cause, http_response=http_response) if exc_type else None + return _MessagingService.FCM_ERROR_TYPES.get(fcm_code) diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index 32e6c6659..4095b7754 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -26,6 +26,22 @@ from firebase_admin import _utils +_NOT_FOUND_ERROR_DICT = { + 'status': 'NOT_FOUND', + 'message': 'test error' +} + + +_UNAVAILABLE_ERROR_DICT = { + 'status': exceptions.UNAVAILABLE, +} + + +_NOT_FOUND_PAYLOAD = json.dumps({ + 'error': _NOT_FOUND_ERROR_DICT, +}) + + class TestRequests(object): def test_timeout_error(self): @@ -78,7 +94,7 @@ def test_http_response_with_message(self): def test_http_response_with_code(self): resp, error = self._create_response() - firebase_error = _utils.handle_requests_error(error, code=exceptions.UNAVAILABLE) + firebase_error = _utils.handle_requests_error(error, error_dict=_UNAVAILABLE_ERROR_DICT) assert isinstance(firebase_error, exceptions.UnavailableError) assert str(firebase_error) == 'Test error' assert firebase_error.cause is error @@ -87,20 +103,14 @@ def test_http_response_with_code(self): def test_http_response_with_message_and_code(self): resp, error = self._create_response() firebase_error = _utils.handle_requests_error( - error, message='Explicit error message', code=exceptions.UNAVAILABLE) + error, message='Explicit error message', error_dict=_UNAVAILABLE_ERROR_DICT) assert isinstance(firebase_error, exceptions.UnavailableError) assert str(firebase_error) == 'Explicit error message' assert firebase_error.cause is error assert firebase_error.http_response is resp def test_handle_platform_error(self): - payload = json.dumps({ - 'error': { - 'status': 'NOT_FOUND', - 'message': 'test error' - } - }) - resp, error = self._create_response(payload=payload) + resp, error = self._create_response(payload=_NOT_FOUND_PAYLOAD) firebase_error = _utils.handle_platform_error_from_requests(error) assert isinstance(firebase_error, exceptions.NotFoundError) assert str(firebase_error) == 'test error' @@ -125,18 +135,12 @@ def test_handle_platform_error_with_no_error_code(self): assert firebase_error.http_response is resp def test_handle_platform_error_with_custom_handler(self): - payload = json.dumps({ - 'error': { - 'status': 'NOT_FOUND', - 'message': 'test error' - } - }) - resp, error = self._create_response(payload=payload) + resp, error = self._create_response(payload=_NOT_FOUND_PAYLOAD) invocations = [] - def _custom_handler(error_dict, message, cause, http_response): - invocations.append((error_dict, message, cause, http_response)) - return exceptions.InvalidArgumentError('Custom message', cause, http_response) + def _custom_handler(cause, message, error_dict): + invocations.append((cause, message, error_dict)) + return exceptions.InvalidArgumentError('Custom message', cause, cause.response) firebase_error = _utils.handle_platform_error_from_requests(error, _custom_handler) @@ -146,27 +150,17 @@ def _custom_handler(error_dict, message, cause, http_response): assert firebase_error.http_response is resp assert len(invocations) == 1 args = invocations[0] - assert len(args) == 4 - assert args[0] == { - 'status': 'NOT_FOUND', - 'message': 'test error' - } + assert len(args) == 3 + assert args[0] is error assert args[1] == 'test error' - assert args[2] is error - assert args[3] is resp + assert args[2] == _NOT_FOUND_ERROR_DICT def test_handle_platform_error_with_custom_handler_ignore(self): - payload = json.dumps({ - 'error': { - 'status': 'NOT_FOUND', - 'message': 'test error' - } - }) - resp, error = self._create_response(payload=payload) + resp, error = self._create_response(payload=_NOT_FOUND_PAYLOAD) invocations = [] - def _custom_handler(error_dict, message, cause, http_response): - invocations.append((error_dict, message, cause, http_response)) + def _custom_handler(cause, message, error_dict): + invocations.append((cause, message, error_dict)) return None firebase_error = _utils.handle_platform_error_from_requests(error, _custom_handler) @@ -177,14 +171,10 @@ def _custom_handler(error_dict, message, cause, http_response): assert firebase_error.http_response is resp assert len(invocations) == 1 args = invocations[0] - assert len(args) == 4 - assert args[0] == { - 'status': 'NOT_FOUND', - 'message': 'test error' - } + assert len(args) == 3 + assert args[0] is error assert args[1] == 'test error' - assert args[2] is error - assert args[3] is resp + assert args[2] == _NOT_FOUND_ERROR_DICT def _create_response(self, status=500, payload=None): resp = models.Response() @@ -254,7 +244,8 @@ def test_http_response_with_message(self): def test_http_response_with_code(self): error = self._create_http_error() - firebase_error = _utils.handle_googleapiclient_error(error, code=exceptions.UNAVAILABLE) + firebase_error = _utils.handle_googleapiclient_error( + error, error_dict=_UNAVAILABLE_ERROR_DICT) assert isinstance(firebase_error, exceptions.UnavailableError) assert str(firebase_error) == str(error) assert firebase_error.cause is error @@ -264,7 +255,7 @@ def test_http_response_with_code(self): def test_http_response_with_message_and_code(self): error = self._create_http_error() firebase_error = _utils.handle_googleapiclient_error( - error, message='Explicit error message', code=exceptions.UNAVAILABLE) + error, message='Explicit error message', error_dict=_UNAVAILABLE_ERROR_DICT) assert isinstance(firebase_error, exceptions.UnavailableError) assert str(firebase_error) == 'Explicit error message' assert firebase_error.cause is error @@ -272,19 +263,13 @@ def test_http_response_with_message_and_code(self): assert firebase_error.http_response.content.decode() == 'Body' def test_handle_platform_error(self): - payload = json.dumps({ - 'error': { - 'status': 'NOT_FOUND', - 'message': 'test error' - } - }) - error = self._create_http_error(payload=payload) + error = self._create_http_error(payload=_NOT_FOUND_PAYLOAD) firebase_error = _utils.handle_platform_error_from_googleapiclient(error) assert isinstance(firebase_error, exceptions.NotFoundError) assert str(firebase_error) == 'test error' assert firebase_error.cause is error assert firebase_error.http_response.status_code == 500 - assert firebase_error.http_response.content.decode() == payload + assert firebase_error.http_response.content.decode() == _NOT_FOUND_PAYLOAD def test_handle_platform_error_with_no_response(self): error = socket.error('Test error') @@ -305,17 +290,11 @@ def test_handle_platform_error_with_no_error_code(self): assert firebase_error.http_response.content.decode() == 'no error code' def test_handle_platform_error_with_custom_handler(self): - payload = json.dumps({ - 'error': { - 'status': 'NOT_FOUND', - 'message': 'test error' - } - }) - error = self._create_http_error(payload=payload) + error = self._create_http_error(payload=_NOT_FOUND_PAYLOAD) invocations = [] - def _custom_handler(error_dict, message, cause, http_response): - invocations.append((error_dict, message, cause, http_response)) + def _custom_handler(cause, message, error_dict, http_response): + invocations.append((cause, message, error_dict, http_response)) return exceptions.InvalidArgumentError('Custom message', cause, http_response) firebase_error = _utils.handle_platform_error_from_googleapiclient(error, _custom_handler) @@ -324,30 +303,21 @@ def _custom_handler(error_dict, message, cause, http_response): assert str(firebase_error) == 'Custom message' assert firebase_error.cause is error assert firebase_error.http_response.status_code == 500 - assert firebase_error.http_response.content.decode() == payload + assert firebase_error.http_response.content.decode() == _NOT_FOUND_PAYLOAD assert len(invocations) == 1 args = invocations[0] assert len(args) == 4 - assert args[0] == { - 'status': 'NOT_FOUND', - 'message': 'test error' - } + assert args[0] is error assert args[1] == 'test error' - assert args[2] is error + assert args[2] == _NOT_FOUND_ERROR_DICT assert args[3] is not None def test_handle_platform_error_with_custom_handler_ignore(self): - payload = json.dumps({ - 'error': { - 'status': 'NOT_FOUND', - 'message': 'test error' - } - }) - error = self._create_http_error(payload=payload) + error = self._create_http_error(payload=_NOT_FOUND_PAYLOAD) invocations = [] - def _custom_handler(error_dict, message, cause, http_response): - invocations.append((error_dict, message, cause, http_response)) + def _custom_handler(cause, message, error_dict, http_response): + invocations.append((cause, message, error_dict, http_response)) return None firebase_error = _utils.handle_platform_error_from_googleapiclient(error, _custom_handler) @@ -356,16 +326,13 @@ def _custom_handler(error_dict, message, cause, http_response): assert str(firebase_error) == 'test error' assert firebase_error.cause is error assert firebase_error.http_response.status_code == 500 - assert firebase_error.http_response.content.decode() == payload + assert firebase_error.http_response.content.decode() == _NOT_FOUND_PAYLOAD assert len(invocations) == 1 args = invocations[0] assert len(args) == 4 - assert args[0] == { - 'status': 'NOT_FOUND', - 'message': 'test error' - } + assert args[0] is error assert args[1] == 'test error' - assert args[2] is error + assert args[2] == _NOT_FOUND_ERROR_DICT assert args[3] is not None def _create_http_error(self, status=500, payload='Body'): From c536f5b225163689ae5b117eb14d5bf1c2d380df Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Fri, 14 Jun 2019 12:56:07 -0700 Subject: [PATCH 11/11] Cleaned up error parsing logic; Updated docs --- firebase_admin/_utils.py | 73 +++++++++++++++++++++++++++---------- firebase_admin/messaging.py | 1 + tests/test_exceptions.py | 13 ++----- 3 files changed, 58 insertions(+), 29 deletions(-) diff --git a/firebase_admin/_utils.py b/firebase_admin/_utils.py index 089408fd4..42b83809e 100644 --- a/firebase_admin/_utils.py +++ b/firebase_admin/_utils.py @@ -85,8 +85,8 @@ def handle_platform_error_from_requests(error, handle_func=None): Args: error: An error raised by the requests module while making an HTTP call to a GCP API. handle_func: A function that can be used to handle platform errors in a custom way. When - specified, this function will be called with four arguments -- parsed error response, - error message, source exception from requests and the HTTP response object. + specified, this function will be called with three arguments. It has the same + signature as ```_handle_func_requests``, but may return ``None``. Returns: FirebaseError: A ``FirebaseError`` that can be raised to the user code. @@ -102,18 +102,38 @@ def handle_platform_error_from_requests(error, handle_func=None): if handle_func: exc = handle_func(error, message, error_dict) - return exc if exc else handle_requests_error(error, message, error_dict) + return exc if exc else _handle_func_requests(error, message, error_dict) -def handle_requests_error(error, message=None, error_dict=None): +def _handle_func_requests(error, message, error_dict): + """Constructs a ``FirebaseError`` from the given GCP error. + + Args: + error: An error raised by the requests module while making an HTTP call. + message: A message to be included in the resulting ``FirebaseError``. + error_dict: Parsed GCP error response. + + Returns: + FirebaseError: A ``FirebaseError`` that can be raised to the user code or None. + """ + code = error_dict.get('status') + return handle_requests_error(error, message, code) + + +def handle_requests_error(error, message=None, code=None): """Constructs a ``FirebaseError`` from the given requests error. + This method is agnostic of the remote service that produced the error, whether it is a GCP + service or otherwise. Therefore, this method does not attempt to parse the error response in + any way. + Args: error: An error raised by the requests module while making an HTTP call. message: A message to be included in the resulting ``FirebaseError`` (optional). If not specified the string representation of the ``error`` argument is used as the message. - error_dict: A parsed error payload to extract error code from (optional). If not specified - parses the response payload available in the ``error``. + code: A GCP error code that will be used to determine the resulting error type (optional). + If not specified the HTTP status code on the error response is used to determine a + suitable error code. Returns: FirebaseError: A ``FirebaseError`` that can be raised to the user code. @@ -131,10 +151,6 @@ def handle_requests_error(error, message=None, error_dict=None): message='Unknown error while making a remote service call: {0}'.format(error), cause=error) - if error_dict is None: - error_dict = {} - - code = error_dict.get('status') if not code: code = _http_status_to_error_code(error.response.status_code) if not message: @@ -152,8 +168,8 @@ def handle_platform_error_from_googleapiclient(error, handle_func=None): Args: error: An error raised by the googleapiclient while making an HTTP call to a GCP API. handle_func: A function that can be used to handle platform errors in a custom way. When - specified, this function will be called with four arguments -- parsed error response, - error message, source exception from googleapiclient and a HTTP response object. + specified, this function will be called with three arguments. It has the same + signature as ```_handle_func_googleapiclient``, but may return ``None``. Returns: FirebaseError: A ``FirebaseError`` that can be raised to the user code. @@ -169,18 +185,39 @@ def handle_platform_error_from_googleapiclient(error, handle_func=None): if handle_func: exc = handle_func(error, message, error_dict, http_response) - return exc if exc else handle_googleapiclient_error(error, message, error_dict, http_response) + return exc if exc else _handle_func_googleapiclient(error, message, error_dict, http_response) -def handle_googleapiclient_error(error, message=None, error_dict=None, http_response=None): +def _handle_func_googleapiclient(error, message, error_dict, http_response): + """Constructs a ``FirebaseError`` from the given GCP error. + + Args: + error: An error raised by the googleapiclient module while making an HTTP call. + message: A message to be included in the resulting ``FirebaseError``. + error_dict: Parsed GCP error response. + http_response: A requests HTTP response object to associate with the exception. + + Returns: + FirebaseError: A ``FirebaseError`` that can be raised to the user code or None. + """ + code = error_dict.get('status') + return handle_googleapiclient_error(error, message, code, http_response) + + +def handle_googleapiclient_error(error, message=None, code=None, http_response=None): """Constructs a ``FirebaseError`` from the given googleapiclient error. + This method is agnostic of the remote service that produced the error, whether it is a GCP + service or otherwise. Therefore, this method does not attempt to parse the error response in + any way. + Args: error: An error raised by the googleapiclient module while making an HTTP call. message: A message to be included in the resulting ``FirebaseError`` (optional). If not specified the string representation of the ``error`` argument is used as the message. - error_dict: A parsed error payload to extract error code from (optional). If not specified - the error code will be determined from the HTTP status code of the response. + code: A GCP error code that will be used to determine the resulting error type (optional). + If not specified the HTTP status code on the error response is used to determine a + suitable error code. http_response: A requests HTTP response object to associate with the exception (optional). If not specified, one will be created from the ``error``. @@ -201,10 +238,6 @@ def handle_googleapiclient_error(error, message=None, error_dict=None, http_resp message='Unknown error while making a remote service call: {0}'.format(error), cause=error) - if error_dict is None: - error_dict = {} - - code = error_dict.get('status') if not code: code = _http_status_to_error_code(error.resp.status) if not message: diff --git a/firebase_admin/messaging.py b/firebase_admin/messaging.py index d15cd5868..63cbbf4be 100644 --- a/firebase_admin/messaging.py +++ b/firebase_admin/messaging.py @@ -499,4 +499,5 @@ def _build_fcm_error(cls, error_dict): for detail in error_dict.get('details', []): if detail.get('@type') == 'type.googleapis.com/google.firebase.fcm.v1.FcmError': fcm_code = detail.get('errorCode') + break return _MessagingService.FCM_ERROR_TYPES.get(fcm_code) diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index 4095b7754..98d9ce5e9 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -32,11 +32,6 @@ } -_UNAVAILABLE_ERROR_DICT = { - 'status': exceptions.UNAVAILABLE, -} - - _NOT_FOUND_PAYLOAD = json.dumps({ 'error': _NOT_FOUND_ERROR_DICT, }) @@ -94,7 +89,7 @@ def test_http_response_with_message(self): def test_http_response_with_code(self): resp, error = self._create_response() - firebase_error = _utils.handle_requests_error(error, error_dict=_UNAVAILABLE_ERROR_DICT) + firebase_error = _utils.handle_requests_error(error, code=exceptions.UNAVAILABLE) assert isinstance(firebase_error, exceptions.UnavailableError) assert str(firebase_error) == 'Test error' assert firebase_error.cause is error @@ -103,7 +98,7 @@ def test_http_response_with_code(self): def test_http_response_with_message_and_code(self): resp, error = self._create_response() firebase_error = _utils.handle_requests_error( - error, message='Explicit error message', error_dict=_UNAVAILABLE_ERROR_DICT) + error, message='Explicit error message', code=exceptions.UNAVAILABLE) assert isinstance(firebase_error, exceptions.UnavailableError) assert str(firebase_error) == 'Explicit error message' assert firebase_error.cause is error @@ -245,7 +240,7 @@ def test_http_response_with_message(self): def test_http_response_with_code(self): error = self._create_http_error() firebase_error = _utils.handle_googleapiclient_error( - error, error_dict=_UNAVAILABLE_ERROR_DICT) + error, code=exceptions.UNAVAILABLE) assert isinstance(firebase_error, exceptions.UnavailableError) assert str(firebase_error) == str(error) assert firebase_error.cause is error @@ -255,7 +250,7 @@ def test_http_response_with_code(self): def test_http_response_with_message_and_code(self): error = self._create_http_error() firebase_error = _utils.handle_googleapiclient_error( - error, message='Explicit error message', error_dict=_UNAVAILABLE_ERROR_DICT) + error, message='Explicit error message', code=exceptions.UNAVAILABLE) assert isinstance(firebase_error, exceptions.UnavailableError) assert str(firebase_error) == 'Explicit error message' assert firebase_error.cause is error