Manage additional 4xx HTTP status code #186

cp-bagerard · cp-bagerard · commit 53bb888ce99a · 2018-01-12T11:51:03.000+01:00
diff --git a/consul/base.py b/consul/base.py
@@ -36,6 +36,11 @@ class BadRequest(ConsulException):
     pass
 
 
+class ClientError(ConsulException):
+    """Encapsulates 4xx Http error code"""
+    pass
+
+
 #
 # Convenience to define checks
 
@@ -158,24 +163,28 @@ def _compat(
 
 class CB(object):
     @classmethod
-    def __status(klass, response, allow_404=True):
+    def _status(klass, response, allow_404=True):
         # status checking
-        if response.code >= 500 and response.code < 600:
+        if 400 <= response.code < 500:
+            if response.code == 400:
+                raise BadRequest('%d %s' % (response.code, response.body))
+            elif response.code == 401:
+                raise ACLDisabled(response.body)
+            elif response.code == 403:
+                raise ACLPermissionDenied(response.body)
+            elif response.code == 404:
+                if not allow_404:
+                    raise NotFound(response.body)
+            else:
+                raise ClientError("%d %s" % (response.code, response.body))
+        elif 500 <= response.code < 600:
             raise ConsulException("%d %s" % (response.code, response.body))
-        if response.code == 400:
-            raise BadRequest('%d %s' % (response.code, response.body))
-        if response.code == 401:
-            raise ACLDisabled(response.body)
-        if response.code == 403:
-            raise ACLPermissionDenied(response.body)
-        if response.code == 404 and not allow_404:
-            raise NotFound(response.body)
 
     @classmethod
     def bool(klass):
         # returns True on successful response
         def cb(response):
-            CB.__status(response)
+            CB._status(response)
             return response.code == 200
         return cb
 
@@ -204,7 +213,7 @@ def json(
         *is_id* only the 'ID' field of the json object will be returned.
         """
         def cb(response):
-            CB.__status(response, allow_404=allow_404)
+            CB._status(response, allow_404=allow_404)
             if response.code == 404:
                 return response.headers['X-Consul-Index'], None
 
diff --git a/tests/test_base.py b/tests/test_base.py
@@ -5,6 +5,9 @@
 import consul.base
 
 
+CB = consul.base.CB
+Response = consul.base.Response
+
 Request = collections.namedtuple(
     'Request', ['method', 'path', 'params', 'data'])
 
@@ -77,6 +80,48 @@ def test_implicit(self):
             assert r(consistency='stale').params == {'stale': '1'}
 
 
+class TestCB(object):
+
+    def test_status_200_passes(self):
+        response = consul.base.Response(200, None, None)
+        CB._status(response)
+
+    @pytest.mark.parametrize(
+        'response, expected_exception',
+        [
+            (Response(400, None, None), consul.base.BadRequest),
+            (Response(401, None, None), consul.base.ACLDisabled),
+            (Response(403, None, None), consul.base.ACLPermissionDenied),
+        ])
+    def test_status_4xx_raises_error(self, response, expected_exception):
+        with pytest.raises(expected_exception):
+            CB._status(response)
+
+    def test_status_404_allow_404(self):
+        response = Response(404, None, None)
+        CB._status(response, allow_404=True)
+
+    def test_status_404_dont_allow_404(self):
+        response = Response(404, None, None)
+        with pytest.raises(consul.base.NotFound):
+            CB._status(response, allow_404=False)
+
+    def test_status_405_raises_generic_ClientError(self):
+        response = Response(405, None, None)
+        with pytest.raises(consul.base.ClientError):
+            CB._status(response)
+
+    @pytest.mark.parametrize(
+        'response',
+        [
+            Response(500, None, None),
+            Response(599, None, None),
+        ])
+    def test_status_5xx_raises_error(self, response):
+        with pytest.raises(consul.base.ConsulException):
+            CB._status(response)
+
+
 class TestChecks(object):
     """
     Check constructor helpers return valid check configurations.
