Repository: auth0/auth0-python
Pull request #98 (2018-04-18 18:54:37)

Raise Auth0Error for bad status code
I copied the logic from authentication/base.py for _process_response into management/rest.py.  This way any 400 or greater status codes will result in an Auth0Error.

The current functionality, which is based on 'error_code' being present in the response is not good enough because not all Auth0 API error response include 'error_code' and thus bad response get through when the expectation is a valid object.  For example, if I call auth0.users.create() and get a 429 response, the API response does not have an error_code and my code expects to receive a valid user profile but instead receives a `{"error": "Too Many Requests", "status_code": 429}`.

This change will make consuming the auth0-python sdk more consistent as all error responses will result in an Auth0Error instead of some resulting in an Auth0Error and other returning an error object when an Auth0 object is expected.


Patch:
diff --git a/auth0/v3/authentication/base.py b/auth0/v3/authentication/base.py
index 778912c..7f9eb3d 100644
--- a/auth0/v3/authentication/base.py
+++ b/auth0/v3/authentication/base.py
@@ -3,6 +3,9 @@ import requests
 from ..exceptions import Auth0Error
 
 
+UNKNOWN_ERROR = 'a0.sdk.internal.unknown'
+
+
 class AuthenticationBase(object):
 
     def post(self, url, data=None, headers=None):
@@ -14,13 +17,75 @@ class AuthenticationBase(object):
         return requests.get(url=url, params=params, headers=headers).text
 
     def _process_response(self, response):
+        return self._parse(response).content()
+
+    def _parse(self, response):
+        if not response.text:
+            return EmptyResponse(response.status_code)
         try:
-            text = json.loads(response.text) if response.text else {}
+            return JsonResponse(response)
         except ValueError:
-            return response.text
+            return PlainResponse(response)
+
+
+class Response(object):
+    def __init__(self, status_code, content):
+        self._status_code = status_code
+        self._content = content
+
+    def content(self):
+        if self._is_error():
+            raise Auth0Error(status_code=self._status_code,
+                             error_code=self._error_code(),
+                             message=self._error_message())
+        else:
+            return self._content
+
+    def _is_error(self):
+        return self._status_code is None or self._status_code >= 400
+
+    # Adding these methods to force implementation in subclasses because they are references in this parent class
+    def _error_code(self):
+        raise NotImplementedError
+
+    def _error_message(self):
+        raise NotImplementedError
+
+
+class JsonResponse(Response):
+    def __init__(self, response):
+        content = json.loads(response.text)
+        super(JsonResponse, self).__init__(response.status_code, content)
+
+    def _error_code(self):
+        if 'error' in self._content:
+            return self._content.get('error')
+        elif 'code' in self._content:
+            return self._content.get('code')
         else:
-            if response.status_code is None or response.status_code >= 400:
-                raise Auth0Error(status_code=response.status_code,
-                                 error_code=text.get('error', ''),
-                                 message=text.get('error_description', ''))
-        return text
+            return UNKNOWN_ERROR
+
+    def _error_message(self):
+        return self._content.get('error_description', '')
+
+
+class PlainResponse(Response):
+    def __init__(self, response):
+        super(PlainResponse, self).__init__(response.status_code, response.text)
+
+    def _error_code(self):
+        return UNKNOWN_ERROR
+
+    def _error_message(self):
+        return self._content
+
+
+class EmptyResponse(Response):
+    def __init__(self, status_code):
+        super(EmptyResponse, self).__init__(status_code, '')
+
+    def _error_code(self):
+        return UNKNOWN_ERROR
+
+    def _error_message(self):
+        return ''
diff --git a/auth0/v3/management/rest.py b/auth0/v3/management/rest.py
index eed23fa..9ff7626 100644
--- a/auth0/v3/management/rest.py
+++ b/auth0/v3/management/rest.py
@@ -5,6 +5,9 @@ import requests
 from ..exceptions import Auth0Error
 
 
+UNKNOWN_ERROR = 'a0.sdk.internal.unknown'
+
+
 class RestClient(object):
 
     """Provides simple methods for handling all RESTful api endpoints. """
@@ -103,10 +106,71 @@ class RestClient(object):
         return self._process_response(response)
 
     def _process_response(self, response):
-        text = json.loads(response.text) if response.text else {}
+        return self._parse(response).content()
+
+    def _parse(self, response):
+        if not response.text:
+            return EmptyResponse(response.status_code)
+        try:
+            return JsonResponse(response)
+        except ValueError:
+            return PlainResponse(response)
+
+class Response(object):
+    def __init__(self, status_code, content):
+        self._status_code = status_code
+        self._content = content
+
+    def content(self):
+        if self._is_error():
+            raise Auth0Error(status_code=self._status_code,
+                             error_code=self._error_code(),
+                             message=self._error_message())
+        else:
+            return self._content
+
+    def _is_error(self):
+        return self._status_code is None or self._status_code >= 400
+
+    # Adding these methods to force implementation in subclasses because they are references in this parent class
+    def _error_code(self):
+        raise NotImplementedError
+
+    def _error_message(self):
+        raise NotImplementedError
+
+class JsonResponse(Response):
+    def __init__(self, response):
+        content = json.loads(response.text)
+        super(JsonResponse, self).__init__(response.status_code, content)
+
+    def _error_code(self):
+        if 'errorCode' in self._content:
+            return self._content.get('errorCode')
+        elif 'error' in self._content:
+            return self._content.get('error')
+        else:
+            return UNKNOWN_ERROR
+
+    def _error_message(self):
+        return self._content.get('error', self._content.get('message', ''))
+
+class PlainResponse(Response):
+    def __init__(self, response):
+        super(PlainResponse, self).__init__(response.status_code, response.text)
+
+    def _error_code(self):
+        return UNKNOWN_ERROR
+
+    def _error_message(self):
+        return self._content
+
+class EmptyResponse(Response):
+    def __init__(self, status_code):
+        super(EmptyResponse, self).__init__(status_code, '')
+
+    def _error_code(self):
+        return UNKNOWN_ERROR
 
-        if isinstance(text, dict) and 'errorCode' in text:
-            raise Auth0Error(status_code=text['statusCode'],
-                             error_code=text['errorCode'],
-                             message=text['message'])
-        return text
+    def _error_message(self):
+        return ''


Test patch:
diff --git a/auth0/v3/test/authentication/test_base.py b/auth0/v3/test/authentication/test_base.py
index d6539c8..03187f1 100644
--- a/auth0/v3/test/authentication/test_base.py
+++ b/auth0/v3/test/authentication/test_base.py
@@ -35,3 +35,65 @@ class TestBase(unittest.TestCase):
             self.assertEqual(context.exception.status_code, error_status)
             self.assertEqual(context.exception.error_code, 'e0')
             self.assertEqual(context.exception.message, 'desc')
+
+    @mock.patch('requests.post')
+    def test_post_error_with_code_property(self, mock_post):
+        ab = AuthenticationBase()
+
+        for error_status in [400, 500, None]:
+            mock_post.return_value.status_code = error_status
+            mock_post.return_value.text = '{"code": "e0",' \
+                                          '"error_description": "desc"}'
+
+            with self.assertRaises(Auth0Error) as context:
+                data = ab.post('the-url', data={'a': 'b'}, headers={'c': 'd'})
+
+            self.assertEqual(context.exception.status_code, error_status)
+            self.assertEqual(context.exception.error_code, 'e0')
+            self.assertEqual(context.exception.message, 'desc')
+
+    @mock.patch('requests.post')
+    def test_post_error_with_no_error_code(self, mock_post):
+        ab = AuthenticationBase()
+
+        for error_status in [400, 500, None]:
+            mock_post.return_value.status_code = error_status
+            mock_post.return_value.text = '{"error_description": "desc"}'
+
+            with self.assertRaises(Auth0Error) as context:
+                data = ab.post('the-url', data={'a': 'b'}, headers={'c': 'd'})
+
+            self.assertEqual(context.exception.status_code, error_status)
+            self.assertEqual(context.exception.error_code, 'a0.sdk.internal.unknown')
+            self.assertEqual(context.exception.message, 'desc')
+
+    @mock.patch('requests.post')
+    def test_post_error_with_text_response(self, mock_post):
+        ab = AuthenticationBase()
+
+        for error_status in [400, 500, None]:
+            mock_post.return_value.status_code = error_status
+            mock_post.return_value.text = 'there has been a terrible error'
+
+            with self.assertRaises(Auth0Error) as context:
+                data = ab.post('the-url', data={'a': 'b'}, headers={'c': 'd'})
+
+            self.assertEqual(context.exception.status_code, error_status)
+            self.assertEqual(context.exception.error_code, 'a0.sdk.internal.unknown')
+            self.assertEqual(context.exception.message,
+                             'there has been a terrible error')
+
+    @mock.patch('requests.post')
+    def test_post_error_with_no_response_text(self, mock_post):
+        ab = AuthenticationBase()
+
+        for error_status in [400, 500, None]:
+            mock_post.return_value.status_code = error_status
+            mock_post.return_value.text = None
+
+            with self.assertRaises(Auth0Error) as context:
+                data = ab.post('the-url', data={'a': 'b'}, headers={'c': 'd'})
+
+            self.assertEqual(context.exception.status_code, error_status)
+            self.assertEqual(context.exception.error_code, 'a0.sdk.internal.unknown')
+            self.assertEqual(context.exception.message, '')
diff --git a/auth0/v3/test/management/test_rest.py b/auth0/v3/test/management/test_rest.py
index b173b3d..6a7418b 100644
--- a/auth0/v3/test/management/test_rest.py
+++ b/auth0/v3/test/management/test_rest.py
@@ -13,6 +13,7 @@ class TestRest(unittest.TestCase):
         headers = {'Authorization': 'Bearer a-token'}
 
         mock_get.return_value.text = '["a", "b"]'
+        mock_get.return_value.status_code = 200
 
         response = rc.get('the-url')
         mock_get.assert_called_with('the-url', params={}, headers=headers)
@@ -27,7 +28,7 @@ class TestRest(unittest.TestCase):
 
         mock_get.return_value.text = ''
         response = rc.get('the/url')
-        self.assertEqual(response, {})
+        self.assertEqual(response, '')
 
     @mock.patch('requests.get')
     def test_get_errors(self, mock_get):
@@ -37,6 +38,7 @@ class TestRest(unittest.TestCase):
         mock_get.return_value.text = '{"statusCode": 999,' \
                                      ' "errorCode": "code",' \
                                      ' "message": "message"}'
+        mock_get.return_value.status_code = 999
 
         with self.assertRaises(Auth0Error) as context:
             rc.get('the/url')
@@ -55,6 +57,7 @@ class TestRest(unittest.TestCase):
 
         data = {'some': 'data'}
 
+        mock_post.return_value.status_code = 200
         response = rc.post('the/url', data=data)
         mock_post.assert_called_with('the/url', data=json.dumps(data),
                                      headers=headers)
@@ -68,6 +71,7 @@ class TestRest(unittest.TestCase):
         mock_post.return_value.text = '{"statusCode": 999,' \
                                       ' "errorCode": "code",' \
                                       ' "message": "message"}'
+        mock_post.return_value.status_code = 999
 
         with self.assertRaises(Auth0Error) as context:
             rc.post('the-url')
@@ -76,6 +80,68 @@ class TestRest(unittest.TestCase):
         self.assertEqual(context.exception.error_code, 'code')
         self.assertEqual(context.exception.message, 'message')
 
+    @mock.patch('requests.post')
+    def test_post_error_with_code_property(self, mock_post):
+        rc = RestClient(jwt='a-token', telemetry=False)
+
+        for error_status in [400, 500, None]:
+            mock_post.return_value.status_code = error_status
+            mock_post.return_value.text = '{"errorCode": "e0",' \
+                                          '"message": "desc"}'
+
+            with self.assertRaises(Auth0Error) as context:
+                rc.post('the-url')
+
+            self.assertEqual(context.exception.status_code, error_status)
+            self.assertEqual(context.exception.error_code, 'e0')
+            self.assertEqual(context.exception.message, 'desc')
+
+    @mock.patch('requests.post')
+    def test_post_error_with_no_error_code(self, mock_post):
+        rc = RestClient(jwt='a-token', telemetry=False)
+
+        for error_status in [400, 500, None]:
+            mock_post.return_value.status_code = error_status
+            mock_post.return_value.text = '{"message": "desc"}'
+
+            with self.assertRaises(Auth0Error) as context:
+                rc.post('the-url')
+
+            self.assertEqual(context.exception.status_code, error_status)
+            self.assertEqual(context.exception.error_code, 'a0.sdk.internal.unknown')
+            self.assertEqual(context.exception.message, 'desc')
+
+    @mock.patch('requests.post')
+    def test_post_error_with_text_response(self, mock_post):
+        rc = RestClient(jwt='a-token', telemetry=False)
+
+        for error_status in [400, 500, None]:
+            mock_post.return_value.status_code = error_status
+            mock_post.return_value.text = 'there has been a terrible error'
+
+            with self.assertRaises(Auth0Error) as context:
+                rc.post('the-url')
+
+            self.assertEqual(context.exception.status_code, error_status)
+            self.assertEqual(context.exception.error_code, 'a0.sdk.internal.unknown')
+            self.assertEqual(context.exception.message,
+                             'there has been a terrible error')
+
+    @mock.patch('requests.post')
+    def test_post_error_with_no_response_text(self, mock_post):
+        rc = RestClient(jwt='a-token', telemetry=False)
+
+        for error_status in [400, 500, None]:
+            mock_post.return_value.status_code = error_status
+            mock_post.return_value.text = None
+
+            with self.assertRaises(Auth0Error) as context:
+                rc.post('the-url')
+
+            self.assertEqual(context.exception.status_code, error_status)
+            self.assertEqual(context.exception.error_code, 'a0.sdk.internal.unknown')
+            self.assertEqual(context.exception.message, '')
+
     @mock.patch('requests.patch')
     def test_patch(self, mock_patch):
         rc = RestClient(jwt='a-token', telemetry=False)
@@ -83,6 +149,7 @@ class TestRest(unittest.TestCase):
                    'Content-Type': 'application/json'}
 
         mock_patch.return_value.text = '["a", "b"]'
+        mock_patch.return_value.status_code = 200
 
         data = {'some': 'data'}
 
@@ -99,6 +166,7 @@ class TestRest(unittest.TestCase):
         mock_patch.return_value.text = '{"statusCode": 999,' \
                                        ' "errorCode": "code",' \
                                        ' "message": "message"}'
+        mock_patch.return_value.status_code = 999
 
         with self.assertRaises(Auth0Error) as context:
             rc.patch(url='the/url')
@@ -113,6 +181,7 @@ class TestRest(unittest.TestCase):
         headers = {'Authorization': 'Bearer a-token'}
 
         mock_delete.return_value.text = '["a", "b"]'
+        mock_delete.return_value.status_code = 200
 
         response = rc.delete(url='the-url/ID')
         mock_delete.assert_called_with('the-url/ID', headers=headers, params={})
@@ -126,6 +195,7 @@ class TestRest(unittest.TestCase):
         mock_delete.return_value.text = '{"statusCode": 999,' \
                                         ' "errorCode": "code",' \
                                         ' "message": "message"}'
+        mock_delete.return_value.status_code = 999
 
         with self.assertRaises(Auth0Error) as context:
             rc.delete(url='the-url')