Migrated remaining messaging APIs to new error types (#298)

* Migrated FCM send APIs to the new error handling regime

* Moved error parsing logic to _utils

* Refactored OP error handling code

* Fixing a broken test

* Added utils for handling googleapiclient errors

* Added tests for new error handling logic

* Updated public API docs

* Fixing test for python3

* Cleaning up the error code lookup code

* Cleaning up the error parsing APIs

* Cleaned up error parsing logic; Updated docs

* Migrated the FCM IID APIs to the new error types

Author: hiranya911

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
 
 
@@ -313,22 +301,13 @@ class _MessagingService(object):
     IID_HEADERS = {'access_token_auth': 'true'}
     JSON_ENCODER = _messaging_utils.MessageEncoder()
 
-    INTERNAL_ERROR = 'internal-error'
-    UNKNOWN_ERROR = 'unknown-error'
     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',
-        401: 'authentication-error',
-        403: 'authentication-error',
-        500: INTERNAL_ERROR,
-        503: 'server-unavailable',
-    }
 
     def __init__(self, app):
         project_id = app.project_id
@@ -431,10 +410,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 +432,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 +443,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."""

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')