From 3218695b30ded412b354ee70a3362cfa8a72287f Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Wed, 12 Jun 2019 16:48:21 -0700 Subject: [PATCH 01/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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 From 093d705e624f07ff67c6de8dfaf2e16c4922988a Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Thu, 20 Jun 2019 14:31:35 -0700 Subject: [PATCH 12/18] Migrated the FCM IID APIs to the new error types --- firebase_admin/messaging.py | 47 +++++++++++-------------------------- tests/test_messaging.py | 38 ++++++++++-------------------- 2 files changed, 27 insertions(+), 58 deletions(-) diff --git a/firebase_admin/messaging.py b/firebase_admin/messaging.py index 63cbbf4be..37ea3db1d 100644 --- a/firebase_admin/messaging.py +++ b/firebase_admin/messaging.py @@ -36,7 +36,6 @@ 'AndroidNotification', 'APNSConfig', 'APNSPayload', - 'ApiCallError', 'Aps', 'ApsAlert', 'BatchResponse', @@ -45,8 +44,12 @@ 'Message', 'MulticastMessage', 'Notification', + 'QuotaExceededError', + 'SenderIdMismatchError', 'SendResponse', + 'ThirdPartyAuthError', 'TopicManagementResponse', + 'UnregisteredError', 'WebpushConfig', 'WebpushFcmOptions', 'WebpushNotification', @@ -167,7 +170,7 @@ def subscribe_to_topic(tokens, topic, app=None): TopicManagementResponse: A ``TopicManagementResponse`` instance. Raises: - ApiCallError: If an error occurs while communicating with instance ID service. + FirebaseError: If an error occurs while communicating with instance ID service. ValueError: If the input arguments are invalid. """ return _get_messaging_service(app).make_topic_management_request( @@ -186,7 +189,7 @@ def unsubscribe_from_topic(tokens, topic, app=None): TopicManagementResponse: A ``TopicManagementResponse`` instance. Raises: - ApiCallError: If an error occurs while communicating with instance ID service. + FirebaseError: If an error occurs while communicating with instance ID service. ValueError: If the input arguments are invalid. """ return _get_messaging_service(app).make_topic_management_request( @@ -243,21 +246,6 @@ def errors(self): return self._errors -class ApiCallError(Exception): - """Represents an Exception encountered while invoking the FCM API. - - Attributes: - code: A string error code. - message: A error message string. - detail: Original low-level exception. - """ - - def __init__(self, code, message, detail=None): - Exception.__init__(self, message) - self.code = code - self.detail = detail - - class BatchResponse(object): """The response received from a batch request to the FCM API.""" @@ -300,7 +288,7 @@ def success(self): @property def exception(self): - """An ApiCallError if an error occurs while sending the message to the FCM service.""" + """A FirebaseError if an error occurs while sending the message to the FCM service.""" return self._exception @@ -322,13 +310,6 @@ class _MessagingService(object): 'THIRD_PARTY_AUTH_ERROR': ThirdPartyAuthError, 'UNREGISTERED': UnregisteredError, } - IID_ERROR_CODES = { - 400: 'invalid-argument', - 401: 'authentication-error', - 403: 'authentication-error', - 500: INTERNAL_ERROR, - 503: 'server-unavailable', - } def __init__(self, app): project_id = app.project_id @@ -431,10 +412,7 @@ def make_topic_management_request(self, tokens, topic, operation): timeout=self._timeout ) except requests.exceptions.RequestException as error: - if error.response is not None: - self._handle_iid_error(error) - else: - raise ApiCallError(self.INTERNAL_ERROR, 'Failed to call instance ID API.', error) + raise self._handle_iid_error(error) else: return TopicManagementResponse(resp) @@ -456,6 +434,9 @@ def _handle_fcm_error(self, error): def _handle_iid_error(self, error): """Handles errors received from the Instance ID API.""" + if error.response is None: + raise _utils.handle_requests_error(error) + data = {} try: parsed_body = error.response.json() @@ -464,13 +445,13 @@ def _handle_iid_error(self, error): except ValueError: pass - code = _MessagingService.IID_ERROR_CODES.get( - error.response.status_code, _MessagingService.UNKNOWN_ERROR) + # IID error response format: {"error": "some error message"} msg = data.get('error') if not msg: msg = 'Unexpected HTTP response with status: {0}; body: {1}'.format( error.response.status_code, error.response.content.decode()) - raise ApiCallError(code, msg, error) + + return _utils.handle_requests_error(error, msg) def _handle_batch_error(self, error): """Handles errors received from the googleapiclient while making batch requests.""" diff --git a/tests/test_messaging.py b/tests/test_messaging.py index cf99c36ba..421556da3 100644 --- a/tests/test_messaging.py +++ b/tests/test_messaging.py @@ -32,9 +32,9 @@ 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] # TODO(hkj): Remove this when IID tests are updated. HTTP_ERROR_CODES = { 400: exceptions.InvalidArgumentError, + 403: exceptions.PermissionDeniedError, 404: exceptions.NotFoundError, 500: exceptions.InternalError, 503: exceptions.UnavailableError, @@ -1859,30 +1859,24 @@ def test_subscribe_to_topic(self, args): assert recorder[0].url == self._get_url('iid/v1:batchAdd') assert json.loads(recorder[0].body.decode()) == args[2] - @pytest.mark.parametrize('status', HTTP_ERRORS) - def test_subscribe_to_topic_error(self, status): + @pytest.mark.parametrize('status, exc_type', HTTP_ERROR_CODES.items()) + def test_subscribe_to_topic_error(self, status, exc_type): _, recorder = self._instrument_iid_service( status=status, payload=self._DEFAULT_ERROR_RESPONSE) - with pytest.raises(messaging.ApiCallError) as excinfo: + with pytest.raises(exc_type) as excinfo: messaging.subscribe_to_topic('foo', 'test-topic') assert str(excinfo.value) == 'error_reason' - code = messaging._MessagingService.IID_ERROR_CODES.get( - status, messaging._MessagingService.UNKNOWN_ERROR) - assert excinfo.value.code == code assert len(recorder) == 1 assert recorder[0].method == 'POST' assert recorder[0].url == self._get_url('iid/v1:batchAdd') - @pytest.mark.parametrize('status', HTTP_ERRORS) - def test_subscribe_to_topic_non_json_error(self, status): + @pytest.mark.parametrize('status, exc_type', HTTP_ERROR_CODES.items()) + def test_subscribe_to_topic_non_json_error(self, status, exc_type): _, recorder = self._instrument_iid_service(status=status, payload='not json') - with pytest.raises(messaging.ApiCallError) as excinfo: + with pytest.raises(exc_type) as excinfo: messaging.subscribe_to_topic('foo', 'test-topic') reason = 'Unexpected HTTP response with status: {0}; body: not json'.format(status) - code = messaging._MessagingService.IID_ERROR_CODES.get( - status, messaging._MessagingService.UNKNOWN_ERROR) assert str(excinfo.value) == reason - assert excinfo.value.code == code assert len(recorder) == 1 assert recorder[0].method == 'POST' assert recorder[0].url == self._get_url('iid/v1:batchAdd') @@ -1897,30 +1891,24 @@ def test_unsubscribe_from_topic(self, args): assert recorder[0].url == self._get_url('iid/v1:batchRemove') assert json.loads(recorder[0].body.decode()) == args[2] - @pytest.mark.parametrize('status', HTTP_ERRORS) - def test_unsubscribe_from_topic_error(self, status): + @pytest.mark.parametrize('status, exc_type', HTTP_ERROR_CODES.items()) + def test_unsubscribe_from_topic_error(self, status, exc_type): _, recorder = self._instrument_iid_service( status=status, payload=self._DEFAULT_ERROR_RESPONSE) - with pytest.raises(messaging.ApiCallError) as excinfo: + with pytest.raises(exc_type) as excinfo: messaging.unsubscribe_from_topic('foo', 'test-topic') assert str(excinfo.value) == 'error_reason' - code = messaging._MessagingService.IID_ERROR_CODES.get( - status, messaging._MessagingService.UNKNOWN_ERROR) - assert excinfo.value.code == code assert len(recorder) == 1 assert recorder[0].method == 'POST' assert recorder[0].url == self._get_url('iid/v1:batchRemove') - @pytest.mark.parametrize('status', HTTP_ERRORS) - def test_unsubscribe_from_topic_non_json_error(self, status): + @pytest.mark.parametrize('status, exc_type', HTTP_ERROR_CODES.items()) + def test_unsubscribe_from_topic_non_json_error(self, status, exc_type): _, recorder = self._instrument_iid_service(status=status, payload='not json') - with pytest.raises(messaging.ApiCallError) as excinfo: + with pytest.raises(exc_type) as excinfo: messaging.unsubscribe_from_topic('foo', 'test-topic') reason = 'Unexpected HTTP response with status: {0}; body: not json'.format(status) - code = messaging._MessagingService.IID_ERROR_CODES.get( - status, messaging._MessagingService.UNKNOWN_ERROR) assert str(excinfo.value) == reason - assert excinfo.value.code == code assert len(recorder) == 1 assert recorder[0].method == 'POST' assert recorder[0].url == self._get_url('iid/v1:batchRemove') From bfa49999549bb3c83e83cdb5e53f82c5845000b8 Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Thu, 20 Jun 2019 14:59:53 -0700 Subject: [PATCH 13/18] Migrated custom token API to new error types --- firebase_admin/_token_gen.py | 16 ++++++++++++---- firebase_admin/auth.py | 9 ++++----- tests/test_token_gen.py | 15 +++++++++------ 3 files changed, 25 insertions(+), 15 deletions(-) diff --git a/firebase_admin/_token_gen.py b/firebase_admin/_token_gen.py index e2eaa5715..0fcb1d0c7 100644 --- a/firebase_admin/_token_gen.py +++ b/firebase_admin/_token_gen.py @@ -21,13 +21,15 @@ import requests import six from google.auth import credentials -from google.auth import exceptions from google.auth import iam from google.auth import jwt from google.auth import transport +import google.auth.exceptions import google.oauth2.id_token import google.oauth2.service_account +from firebase_admin import exceptions + # ID token constants ID_TOKEN_ISSUER_PREFIX = 'https://securetoken.google.com/' @@ -53,7 +55,6 @@ # Error codes COOKIE_CREATE_ERROR = 'COOKIE_CREATE_ERROR' -TOKEN_SIGN_ERROR = 'TOKEN_SIGN_ERROR' class ApiCallError(Exception): @@ -177,9 +178,9 @@ def create_custom_token(self, uid, developer_claims=None): payload['claims'] = developer_claims try: return jwt.encode(signing_provider.signer, payload) - except exceptions.TransportError as error: + except google.auth.exceptions.TransportError as error: msg = 'Failed to sign custom token. {0}'.format(error) - raise ApiCallError(TOKEN_SIGN_ERROR, msg, error) + raise TokenSignError(msg, error) def create_session_cookie(self, id_token, expires_in): @@ -339,3 +340,10 @@ def verify(self, token, request): certs_url=self.cert_url) verified_claims['uid'] = verified_claims['sub'] return verified_claims + + +class TokenSignError(exceptions.UnknownError): + """Unexpected error while signing a Firebase custom token.""" + + def __init__(self, message, cause): + exceptions.UnknownError.__init__(self, message, cause) diff --git a/firebase_admin/auth.py b/firebase_admin/auth.py index 0800d7c1e..5e168b2fe 100644 --- a/firebase_admin/auth.py +++ b/firebase_admin/auth.py @@ -42,6 +42,7 @@ 'ExportedUserRecord', 'ImportUserRecord', 'ListUsersPage', + 'TokenSignError', 'UserImportHash', 'UserImportResult', 'UserInfo', @@ -75,6 +76,7 @@ ListUsersPage = _user_mgt.ListUsersPage UserImportHash = _user_import.UserImportHash ImportUserRecord = _user_import.ImportUserRecord +TokenSignError = _token_gen.TokenSignError UserImportResult = _user_import.UserImportResult UserInfo = _user_mgt.UserInfo UserMetadata = _user_mgt.UserMetadata @@ -115,13 +117,10 @@ def create_custom_token(uid, developer_claims=None, app=None): Raises: ValueError: If input parameters are invalid. - AuthError: If an error occurs while creating the token using the remote IAM service. + TokenSignError: If an error occurs while signing the token using the remote IAM service. """ token_generator = _get_auth_service(app).token_generator - try: - return token_generator.create_custom_token(uid, developer_claims) - except _token_gen.ApiCallError as error: - raise AuthError(error.code, str(error), error.detail) + return token_generator.create_custom_token(uid, developer_claims) def verify_id_token(id_token, app=None, check_revoked=False): diff --git a/tests/test_token_gen.py b/tests/test_token_gen.py index 412ba3d0e..3a25640aa 100644 --- a/tests/test_token_gen.py +++ b/tests/test_token_gen.py @@ -21,8 +21,8 @@ import time from google.auth import crypt -from google.auth import exceptions from google.auth import jwt +import google.auth.exceptions import google.oauth2.id_token import pytest from pytest_localserver import plugin @@ -31,6 +31,7 @@ import firebase_admin from firebase_admin import auth from firebase_admin import credentials +from firebase_admin import exceptions from firebase_admin import _token_gen from tests import testutils @@ -219,10 +220,12 @@ def test_sign_with_iam_error(self): try: iam_resp = '{"error": {"code": 403, "message": "test error"}}' _overwrite_iam_request(app, testutils.MockRequest(403, iam_resp)) - with pytest.raises(auth.AuthError) as excinfo: + with pytest.raises(auth.TokenSignError) as excinfo: auth.create_custom_token(MOCK_UID, app=app) - assert excinfo.value.code == _token_gen.TOKEN_SIGN_ERROR - assert iam_resp in str(excinfo.value) + error = excinfo.value + assert error.code == exceptions.UNKNOWN + assert iam_resp in str(error) + assert isinstance(error.cause, google.auth.exceptions.TransportError) finally: firebase_admin.delete_app(app) @@ -421,7 +424,7 @@ def test_custom_token(self, auth_app): def test_certificate_request_failure(self, user_mgt_app): _overwrite_cert_request(user_mgt_app, testutils.MockRequest(404, 'not found')) - with pytest.raises(exceptions.TransportError): + with pytest.raises(google.auth.exceptions.TransportError): auth.verify_id_token(TEST_ID_TOKEN, app=user_mgt_app) @@ -521,7 +524,7 @@ def test_custom_token(self, auth_app): def test_certificate_request_failure(self, user_mgt_app): _overwrite_cert_request(user_mgt_app, testutils.MockRequest(404, 'not found')) - with pytest.raises(exceptions.TransportError): + with pytest.raises(google.auth.exceptions.TransportError): auth.verify_session_cookie(TEST_SESSION_COOKIE, app=user_mgt_app) From 21ca962f3b69c3ce16d6fa60813782c64aa40824 Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Thu, 20 Jun 2019 16:20:42 -0700 Subject: [PATCH 14/18] Migrated create cookie API to new error types --- firebase_admin/_auth_utils.py | 51 ++++++++++++++++++++++++++++++++++ firebase_admin/_http_client.py | 4 +++ firebase_admin/_token_gen.py | 31 +++++---------------- firebase_admin/auth.py | 10 +++---- integration/test_auth.py | 5 ++++ tests/test_token_gen.py | 12 ++++---- 6 files changed, 78 insertions(+), 35 deletions(-) diff --git a/firebase_admin/_auth_utils.py b/firebase_admin/_auth_utils.py index b6788355c..c12cf73a8 100644 --- a/firebase_admin/_auth_utils.py +++ b/firebase_admin/_auth_utils.py @@ -20,6 +20,9 @@ import six from six.moves import urllib +from firebase_admin import exceptions +from firebase_admin import _utils + MAX_CLAIMS_PAYLOAD_SIZE = 1000 RESERVED_CLAIMS = set([ @@ -188,3 +191,51 @@ def validate_action_type(action_type): raise ValueError('Invalid action type provided action_type: {0}. \ Valid values are {1}'.format(action_type, ', '.join(VALID_EMAIL_ACTION_TYPES))) return action_type + + +def handle_auth_backend_error(error): + if error.response is None: + raise _utils.handle_requests_error(error) + + error_dict = {} + try: + parsed_body = error.response.json() + if isinstance(parsed_body, dict): + error_dict = parsed_body.get('error') + except ValueError: + pass + + if not error_dict: + raise _utils.handle_requests_error(error) + + code = error_dict.get('message', '') + sep = code.find(':') + custom_message = None + if sep != -1: + code = code[:sep] + custom_message = code[sep + 1:].strip() + + msg = custom_message if custom_message else code + exc_type = _CODE_TO_EXC_TYPE.get(code) + if exc_type: + return exc_type(msg, cause=error, http_response=error.response) + + return _utils.handle_requests_error(error, message=msg) + + +class InvalidIdTokenError(exceptions.InvalidArgumentError): + """The provided ID token is not a valid Firebase ID token.""" + + def __init__(self, message, cause, http_response=None): + exceptions.InvalidArgumentError.__init__(self, message, cause, http_response) + + +class UnexpectedResponseError(exceptions.UnknownError): + + def __init__(self, message, cause=None, http_response=None): + exceptions.UnknownError.__init__(self, message, cause, http_response) + + +_CODE_TO_EXC_TYPE = { + 'INVALID_ID_TOKEN': InvalidIdTokenError, +} diff --git a/firebase_admin/_http_client.py b/firebase_admin/_http_client.py index 73028f833..eb8c4027a 100644 --- a/firebase_admin/_http_client.py +++ b/firebase_admin/_http_client.py @@ -109,6 +109,10 @@ def headers(self, method, url, **kwargs): resp = self.request(method, url, **kwargs) return resp.headers + def body_and_response(self, method, url, **kwargs): + resp = self.request(method, url, **kwargs) + return self.parse_body(resp), resp + def body(self, method, url, **kwargs): resp = self.request(method, url, **kwargs) return self.parse_body(resp) diff --git a/firebase_admin/_token_gen.py b/firebase_admin/_token_gen.py index 0fcb1d0c7..d0ba83d7f 100644 --- a/firebase_admin/_token_gen.py +++ b/firebase_admin/_token_gen.py @@ -29,6 +29,7 @@ import google.oauth2.service_account from firebase_admin import exceptions +from firebase_admin import _auth_utils # ID token constants @@ -53,18 +54,6 @@ METADATA_SERVICE_URL = ('http://metadata/computeMetadata/v1/instance/service-accounts/' 'default/email') -# Error codes -COOKIE_CREATE_ERROR = 'COOKIE_CREATE_ERROR' - - -class ApiCallError(Exception): - """Represents an Exception encountered while invoking the ID toolkit API.""" - - def __init__(self, code, message, error=None): - Exception.__init__(self, message) - self.code = code - self.detail = error - class _SigningProvider(object): """Stores a reference to a google.auth.crypto.Signer.""" @@ -207,20 +196,14 @@ def create_session_cookie(self, id_token, expires_in): 'validDuration': expires_in, } try: - response = self.client.body('post', ':createSessionCookie', json=payload) + body, http_resp = self.client.body_and_response('post', ':createSessionCookie', json=payload) except requests.exceptions.RequestException as error: - self._handle_http_error(COOKIE_CREATE_ERROR, 'Failed to create session cookie', error) - else: - if not response or not response.get('sessionCookie'): - raise ApiCallError(COOKIE_CREATE_ERROR, 'Failed to create session cookie.') - return response.get('sessionCookie') - - def _handle_http_error(self, code, msg, error): - if error.response is not None: - msg += '\nServer response: {0}'.format(error.response.content.decode()) + raise _auth_utils.handle_auth_backend_error(error) else: - msg += '\nReason: {0}'.format(error) - raise ApiCallError(code, msg, error) + if not body or not body.get('sessionCookie'): + raise _auth_utils.UnexpectedResponseError( + 'Failed to create session cookie.', http_response=http_resp) + return body.get('sessionCookie') class TokenVerifier(object): diff --git a/firebase_admin/auth.py b/firebase_admin/auth.py index 5e168b2fe..cd9a882e6 100644 --- a/firebase_admin/auth.py +++ b/firebase_admin/auth.py @@ -22,6 +22,7 @@ import time import firebase_admin +from firebase_admin import _auth_utils from firebase_admin import _http_client from firebase_admin import _token_gen from firebase_admin import _user_import @@ -76,7 +77,9 @@ ListUsersPage = _user_mgt.ListUsersPage UserImportHash = _user_import.UserImportHash ImportUserRecord = _user_import.ImportUserRecord +InvalidIdTokenError = _auth_utils.InvalidIdTokenError TokenSignError = _token_gen.TokenSignError +UnexpectedResponseError = _auth_utils.UnexpectedResponseError UserImportResult = _user_import.UserImportResult UserInfo = _user_mgt.UserInfo UserMetadata = _user_mgt.UserMetadata @@ -169,13 +172,10 @@ def create_session_cookie(id_token, expires_in, app=None): Raises: ValueError: If input parameters are invalid. - AuthError: If an error occurs while creating the cookie. + FirebaseError: If an error occurs while creating the cookie. """ token_generator = _get_auth_service(app).token_generator - try: - return token_generator.create_session_cookie(id_token, expires_in) - except _token_gen.ApiCallError as error: - raise AuthError(error.code, str(error), error.detail) + return token_generator.create_session_cookie(id_token, expires_in) def verify_session_cookie(session_cookie, check_revoked=False, app=None): diff --git a/integration/test_auth.py b/integration/test_auth.py index 53577b827..a2905d881 100644 --- a/integration/test_auth.py +++ b/integration/test_auth.py @@ -129,6 +129,11 @@ def test_session_cookies(api_key): estimated_exp = int(time.time() + expires_in.total_seconds()) assert abs(claims['exp'] - estimated_exp) < 5 +def test_session_cookie_error(): + expires_in = datetime.timedelta(days=1) + with pytest.raises(auth.InvalidIdTokenError): + auth.create_session_cookie('not.a.token', expires_in=expires_in) + def test_get_non_existing_user(): with pytest.raises(auth.AuthError) as excinfo: auth.get_user('non.existing') diff --git a/tests/test_token_gen.py b/tests/test_token_gen.py index 3a25640aa..6882fa040 100644 --- a/tests/test_token_gen.py +++ b/tests/test_token_gen.py @@ -301,17 +301,17 @@ def test_valid_args(self, user_mgt_app, expires_in): assert request == {'idToken' : 'id_token', 'validDuration': 3600} def test_error(self, user_mgt_app): - _instrument_user_manager(user_mgt_app, 500, '{"error":"test"}') - with pytest.raises(auth.AuthError) as excinfo: + _instrument_user_manager(user_mgt_app, 500, '{"error":{"message": "INVALID_ID_TOKEN"}}') + with pytest.raises(auth.InvalidIdTokenError) as excinfo: auth.create_session_cookie('id_token', expires_in=3600, app=user_mgt_app) - assert excinfo.value.code == _token_gen.COOKIE_CREATE_ERROR - assert '{"error":"test"}' in str(excinfo.value) + assert excinfo.value.code == exceptions.INVALID_ARGUMENT + assert str(excinfo.value) == 'INVALID_ID_TOKEN' def test_unexpected_response(self, user_mgt_app): _instrument_user_manager(user_mgt_app, 200, '{}') - with pytest.raises(auth.AuthError) as excinfo: + with pytest.raises(auth.UnexpectedResponseError) as excinfo: auth.create_session_cookie('id_token', expires_in=3600, app=user_mgt_app) - assert excinfo.value.code == _token_gen.COOKIE_CREATE_ERROR + assert excinfo.value.code == exceptions.UNKNOWN assert 'Failed to create session cookie' in str(excinfo.value) From 675adb05a112e9d2557972ea1f3da5481d8f66b7 Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Fri, 21 Jun 2019 11:40:27 -0700 Subject: [PATCH 15/18] Improved error message computation --- firebase_admin/_auth_utils.py | 18 ++++++++++++------ firebase_admin/_token_gen.py | 3 ++- tests/test_token_gen.py | 15 ++++++++++++++- 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/firebase_admin/_auth_utils.py b/firebase_admin/_auth_utils.py index c12cf73a8..ef8a96b94 100644 --- a/firebase_admin/_auth_utils.py +++ b/firebase_admin/_auth_utils.py @@ -194,6 +194,7 @@ def validate_action_type(action_type): def handle_auth_backend_error(error): + """Converts a requests error received from the Firebase Auth service into a FirebaseError.""" if error.response is None: raise _utils.handle_requests_error(error) @@ -208,24 +209,29 @@ def handle_auth_backend_error(error): if not error_dict: raise _utils.handle_requests_error(error) - code = error_dict.get('message', '') - sep = code.find(':') + # Auth error response format: {"error": {"message": "AUTH_ERROR_CODE: Optional text"}} + code = error_dict.get('message', '_NONE_') + separator = code.find(':') custom_message = None - if sep != -1: - code = code[:sep] - custom_message = code[sep + 1:].strip() + if separator != -1: + custom_message = code[separator + 1:].strip() + code = code[:separator] - msg = custom_message if custom_message else code + ext = ' {0}'.format(custom_message) if custom_message else '' exc_type = _CODE_TO_EXC_TYPE.get(code) if exc_type: + msg = '{0} ({1}).{2}'.format(exc_type.message, code, ext) return exc_type(msg, cause=error, http_response=error.response) + msg = 'Unexpected error code: {0}.{1}'.format(code, ext) return _utils.handle_requests_error(error, message=msg) class InvalidIdTokenError(exceptions.InvalidArgumentError): """The provided ID token is not a valid Firebase ID token.""" + message = 'The provided ID token is invalid' + def __init__(self, message, cause, http_response=None): exceptions.InvalidArgumentError.__init__(self, message, cause, http_response) diff --git a/firebase_admin/_token_gen.py b/firebase_admin/_token_gen.py index d0ba83d7f..0ea34f77c 100644 --- a/firebase_admin/_token_gen.py +++ b/firebase_admin/_token_gen.py @@ -196,7 +196,8 @@ def create_session_cookie(self, id_token, expires_in): 'validDuration': expires_in, } try: - body, http_resp = self.client.body_and_response('post', ':createSessionCookie', json=payload) + body, http_resp = self.client.body_and_response( + 'post', ':createSessionCookie', json=payload) except requests.exceptions.RequestException as error: raise _auth_utils.handle_auth_backend_error(error) else: diff --git a/tests/test_token_gen.py b/tests/test_token_gen.py index 6882fa040..42171b580 100644 --- a/tests/test_token_gen.py +++ b/tests/test_token_gen.py @@ -305,7 +305,20 @@ def test_error(self, user_mgt_app): with pytest.raises(auth.InvalidIdTokenError) as excinfo: auth.create_session_cookie('id_token', expires_in=3600, app=user_mgt_app) assert excinfo.value.code == exceptions.INVALID_ARGUMENT - assert str(excinfo.value) == 'INVALID_ID_TOKEN' + assert str(excinfo.value) == 'The provided ID token is invalid (INVALID_ID_TOKEN).' + + def test_error_with_details(self, user_mgt_app): + _instrument_user_manager(user_mgt_app, 500, '{"error":{"message": "INVALID_ID_TOKEN: More details."}}') + with pytest.raises(auth.InvalidIdTokenError) as excinfo: + auth.create_session_cookie('id_token', expires_in=3600, app=user_mgt_app) + assert excinfo.value.code == exceptions.INVALID_ARGUMENT + assert str(excinfo.value) == 'The provided ID token is invalid (INVALID_ID_TOKEN). More details.' + + def test_unknown_error(self, user_mgt_app): + _instrument_user_manager(user_mgt_app, 500, '{"error":{"message": "SOMETHING_UNUSUAL"}}') + with pytest.raises(exceptions.InternalError) as excinfo: + auth.create_session_cookie('id_token', expires_in=3600, app=user_mgt_app) + assert str(excinfo.value) == 'Unexpected error code: SOMETHING_UNUSUAL.' def test_unexpected_response(self, user_mgt_app): _instrument_user_manager(user_mgt_app, 200, '{}') From 2bcd52517cd6ac9e5c2755c9a70b1e2fb5c1585a Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Fri, 21 Jun 2019 14:16:11 -0700 Subject: [PATCH 16/18] Refactored the shared error handling code --- firebase_admin/_auth_utils.py | 50 ++++++++++++++++++++++------------- tests/test_token_gen.py | 10 +++++-- 2 files changed, 40 insertions(+), 20 deletions(-) diff --git a/firebase_admin/_auth_utils.py b/firebase_admin/_auth_utils.py index ef8a96b94..be0efeb81 100644 --- a/firebase_admin/_auth_utils.py +++ b/firebase_admin/_auth_utils.py @@ -198,45 +198,59 @@ def handle_auth_backend_error(error): if error.response is None: raise _utils.handle_requests_error(error) + code, custom_message = _parse_error_body(error.response) + if not code: + msg = 'Unexpected error response: {0}'.format(error.response.content.decode()) + raise _utils.handle_requests_error(error, message=msg) + + exc_type = _CODE_TO_EXC_TYPE.get(code) + msg = _build_error_message(code, exc_type, custom_message) + if not exc_type: + return _utils.handle_requests_error(error, message=msg) + + return exc_type(msg, cause=error, http_response=error.response) + + +def _parse_error_body(response): + """Parses the given error response to extract Auth error code and message.""" error_dict = {} try: - parsed_body = error.response.json() + parsed_body = response.json() if isinstance(parsed_body, dict): - error_dict = parsed_body.get('error') + error_dict = parsed_body.get('error', {}) except ValueError: pass - if not error_dict: - raise _utils.handle_requests_error(error) - # Auth error response format: {"error": {"message": "AUTH_ERROR_CODE: Optional text"}} - code = error_dict.get('message', '_NONE_') - separator = code.find(':') + code = error_dict.get('message') custom_message = None - if separator != -1: - custom_message = code[separator + 1:].strip() - code = code[:separator] + if code: + separator = code.find(':') + if separator != -1: + custom_message = code[separator + 1:].strip() + code = code[:separator] - ext = ' {0}'.format(custom_message) if custom_message else '' - exc_type = _CODE_TO_EXC_TYPE.get(code) - if exc_type: - msg = '{0} ({1}).{2}'.format(exc_type.message, code, ext) - return exc_type(msg, cause=error, http_response=error.response) + return code, custom_message - msg = 'Unexpected error code: {0}.{1}'.format(code, ext) - return _utils.handle_requests_error(error, message=msg) + +def _build_error_message(code, exc_type, custom_message): + preamble = exc_type.default_message if ( + exc_type and hasattr(exc_type, 'default_message')) else 'Error while calling Auth service' + ext = ' {0}'.format(custom_message) if custom_message else '' + return '{0} ({1}).{2}'.format(preamble, code, ext) class InvalidIdTokenError(exceptions.InvalidArgumentError): """The provided ID token is not a valid Firebase ID token.""" - message = 'The provided ID token is invalid' + default_message = 'The provided ID token is invalid' def __init__(self, message, cause, http_response=None): exceptions.InvalidArgumentError.__init__(self, message, cause, http_response) class UnexpectedResponseError(exceptions.UnknownError): + """Backend service responded with an unexpected or malformed response.""" def __init__(self, message, cause=None, http_response=None): exceptions.UnknownError.__init__(self, message, cause, http_response) diff --git a/tests/test_token_gen.py b/tests/test_token_gen.py index 42171b580..8574661a4 100644 --- a/tests/test_token_gen.py +++ b/tests/test_token_gen.py @@ -314,11 +314,17 @@ def test_error_with_details(self, user_mgt_app): assert excinfo.value.code == exceptions.INVALID_ARGUMENT assert str(excinfo.value) == 'The provided ID token is invalid (INVALID_ID_TOKEN). More details.' - def test_unknown_error(self, user_mgt_app): + def test_unexpected_error_code(self, user_mgt_app): _instrument_user_manager(user_mgt_app, 500, '{"error":{"message": "SOMETHING_UNUSUAL"}}') with pytest.raises(exceptions.InternalError) as excinfo: auth.create_session_cookie('id_token', expires_in=3600, app=user_mgt_app) - assert str(excinfo.value) == 'Unexpected error code: SOMETHING_UNUSUAL.' + assert str(excinfo.value) == 'Error while calling Auth service (SOMETHING_UNUSUAL).' + + def test_unexpected_error_response(self, user_mgt_app): + _instrument_user_manager(user_mgt_app, 500, '{}') + with pytest.raises(exceptions.InternalError) as excinfo: + auth.create_session_cookie('id_token', expires_in=3600, app=user_mgt_app) + assert str(excinfo.value) == 'Unexpected error response: {}' def test_unexpected_response(self, user_mgt_app): _instrument_user_manager(user_mgt_app, 200, '{}') From 1ca0872e4ff19f8e838b9006cb9491fd427bafc4 Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Thu, 4 Jul 2019 22:21:54 -0700 Subject: [PATCH 17/18] Fixing lint errors --- firebase_admin/_auth_utils.py | 42 +++++++++++++++++------------------ tests/test_token_gen.py | 6 +++-- 2 files changed, 25 insertions(+), 23 deletions(-) diff --git a/firebase_admin/_auth_utils.py b/firebase_admin/_auth_utils.py index be0efeb81..d96d97785 100644 --- a/firebase_admin/_auth_utils.py +++ b/firebase_admin/_auth_utils.py @@ -193,6 +193,27 @@ def validate_action_type(action_type): return action_type +class InvalidIdTokenError(exceptions.InvalidArgumentError): + """The provided ID token is not a valid Firebase ID token.""" + + default_message = 'The provided ID token is invalid' + + def __init__(self, message, cause, http_response=None): + exceptions.InvalidArgumentError.__init__(self, message, cause, http_response) + + +class UnexpectedResponseError(exceptions.UnknownError): + """Backend service responded with an unexpected or malformed response.""" + + def __init__(self, message, cause=None, http_response=None): + exceptions.UnknownError.__init__(self, message, cause, http_response) + + +_CODE_TO_EXC_TYPE = { + 'INVALID_ID_TOKEN': InvalidIdTokenError, +} + + def handle_auth_backend_error(error): """Converts a requests error received from the Firebase Auth service into a FirebaseError.""" if error.response is None: @@ -238,24 +259,3 @@ def _build_error_message(code, exc_type, custom_message): exc_type and hasattr(exc_type, 'default_message')) else 'Error while calling Auth service' ext = ' {0}'.format(custom_message) if custom_message else '' return '{0} ({1}).{2}'.format(preamble, code, ext) - - -class InvalidIdTokenError(exceptions.InvalidArgumentError): - """The provided ID token is not a valid Firebase ID token.""" - - default_message = 'The provided ID token is invalid' - - def __init__(self, message, cause, http_response=None): - exceptions.InvalidArgumentError.__init__(self, message, cause, http_response) - - -class UnexpectedResponseError(exceptions.UnknownError): - """Backend service responded with an unexpected or malformed response.""" - - def __init__(self, message, cause=None, http_response=None): - exceptions.UnknownError.__init__(self, message, cause, http_response) - - -_CODE_TO_EXC_TYPE = { - 'INVALID_ID_TOKEN': InvalidIdTokenError, -} diff --git a/tests/test_token_gen.py b/tests/test_token_gen.py index 8574661a4..baf8d9515 100644 --- a/tests/test_token_gen.py +++ b/tests/test_token_gen.py @@ -308,11 +308,13 @@ def test_error(self, user_mgt_app): assert str(excinfo.value) == 'The provided ID token is invalid (INVALID_ID_TOKEN).' def test_error_with_details(self, user_mgt_app): - _instrument_user_manager(user_mgt_app, 500, '{"error":{"message": "INVALID_ID_TOKEN: More details."}}') + _instrument_user_manager( + user_mgt_app, 500, '{"error":{"message": "INVALID_ID_TOKEN: More details."}}') with pytest.raises(auth.InvalidIdTokenError) as excinfo: auth.create_session_cookie('id_token', expires_in=3600, app=user_mgt_app) assert excinfo.value.code == exceptions.INVALID_ARGUMENT - assert str(excinfo.value) == 'The provided ID token is invalid (INVALID_ID_TOKEN). More details.' + expected = 'The provided ID token is invalid (INVALID_ID_TOKEN). More details.' + assert str(excinfo.value) == expected def test_unexpected_error_code(self, user_mgt_app): _instrument_user_manager(user_mgt_app, 500, '{"error":{"message": "SOMETHING_UNUSUAL"}}') From a19a66ada53b63277bf86ccab97c9892d5be1f76 Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Thu, 18 Jul 2019 15:12:31 -0700 Subject: [PATCH 18/18] Renamed variable for clarity --- firebase_admin/_auth_utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/firebase_admin/_auth_utils.py b/firebase_admin/_auth_utils.py index d96d97785..d9b8c66e0 100644 --- a/firebase_admin/_auth_utils.py +++ b/firebase_admin/_auth_utils.py @@ -255,7 +255,7 @@ def _parse_error_body(response): def _build_error_message(code, exc_type, custom_message): - preamble = exc_type.default_message if ( + default_message = exc_type.default_message if ( exc_type and hasattr(exc_type, 'default_message')) else 'Error while calling Auth service' ext = ' {0}'.format(custom_message) if custom_message else '' - return '{0} ({1}).{2}'.format(preamble, code, ext) + return '{0} ({1}).{2}'.format(default_message, code, ext)