Merge branch 'feature/handle-non-json-errors' into develop

sevenbridges/decorators.py

@@ -13,7 +13,7 @@
 from sevenbridges.errors import (
     BadRequest, Unauthorized, Forbidden, NotFound, MethodNotAllowed,
     RequestTimeout, Conflict, TooManyRequests, SbgError, ServerError,
-    ServiceUnavailable
+    ServiceUnavailable, NonJSONResponseError
 )
 
 logger = logging.getLogger(__name__)
@@ -67,12 +67,14 @@ def check_for_error(func):
     def wrapper(*args, **kwargs):
         try:
             response = func(*args, **kwargs)
+        except requests.RequestException as e:
+            raise SbgError(message=six.text_type(e))
+        try:
             status_code = response.status_code
             if status_code in range(200, 204):
                 return response
             if status_code == 204:
                 return
-            data = response.json()
             e = {
                 400: BadRequest,
                 401: Unauthorized,
@@ -85,6 +87,7 @@ def wrapper(*args, **kwargs):
                 500: ServerError,
                 503: ServiceUnavailable,
             }.get(status_code, SbgError)()
+            data = response.json()
             if 'message' in data:
                 e.message = data['message']
             if 'code' in data:
@@ -94,14 +97,12 @@ def wrapper(*args, **kwargs):
             if 'more_info' in data:
                 e.more_info = data['more_info']
             raise e
-        except requests.RequestException as e:
-            raise SbgError(message=six.text_type(e))
         except JSONDecodeError:
             raise_from(
-                ServiceUnavailable(message=six.text_type(
-                    'Server response format is not JSON, '
-                    'service might be unavailable.'
-                )), None
+                NonJSONResponseError(
+                    status=response.status_code,
+                    message=six.text_type(response.text)
+                ), None
             )
         except ValueError as e:
             raise SbgError(message=six.text_type(e))

sevenbridges/errors.py

@@ -32,6 +32,13 @@ def __init__(self):
         )
 
 
+class NonJSONResponseError(SbgError):
+    def __init__(self, status, code=None, message=None, more_info=None):
+        super(NonJSONResponseError, self).__init__(
+            code=code, status=status, message=message, more_info=more_info
+        )
+
+
 class ReadOnlyPropertyError(SbgError):
     def __init__(self, message):
         super(ReadOnlyPropertyError, self).__init__(

sevenbridges/meta/resource.py

@@ -1,9 +1,13 @@
 import copy
 import logging
+try:
+    from json.decoder import JSONDecodeError
+except ImportError:
+    JSONDecodeError = ValueError
 
 import six
 
-from sevenbridges.errors import SbgError
+from sevenbridges.errors import SbgError, NonJSONResponseError
 from sevenbridges.meta.fields import Field
 from sevenbridges.meta.data import DataContainer
 from sevenbridges.meta.transformer import Transform
@@ -144,7 +148,15 @@ def _query(cls, **kwargs):
         extra = {'resource': cls.__name__, 'query': kwargs}
         logger.info('Querying {} resource'.format(cls), extra=extra)
         response = api.get(url=url, params=kwargs)
-        data = response.json()
+        try:
+            data = response.json()
+        except JSONDecodeError:
+            six.raise_from(
+                NonJSONResponseError(
+                    status=response.status_code,
+                    message=six.text_type(response.text)
+                ), None
+            )
         total = response.headers['x-total-matching-query']
 
         items = [cls(api=api, **item) for item in data['items']]

tests/providers.py

@@ -551,6 +551,17 @@ def apps_exist(self, visibility, num_of_apps):
         self.request_mocker.get(href, json=response, headers={
             'x-total-matching-query': str(num_of_apps)})
 
+    def app_exist_non_json(self, status_code):
+        url = '/apps'
+        href = self.base_url + url
+
+        response = generator.user_name()
+        self.request_mocker.get(
+            href,
+            status_code=status_code,
+            text=response
+        )
+
     def app_with_revision_exists(self, **kwargs):
         app = self.default_app()
         app.update(kwargs)

tests/test_decorators.py

@@ -0,0 +1,16 @@
+import faker
+import pytest
+
+from sevenbridges.errors import NonJSONResponseError
+
+generator = faker.Factory.create()
+
+
+@pytest.mark.parametrize("status_code", [200, 500])
+def test_non_json_response(api, given, status_code):
+    # preconditions
+    given.app.app_exist_non_json(status_code=status_code)
+
+    # action
+    with pytest.raises(NonJSONResponseError):
+        api.apps.query(visibility="private")