Merge pull request #19 from stax-labs/feat/handle-http-error

ZXYmania · web-flow · commit feecb9a4a527 · 2020-05-08T10:10:02.000+10:00
feat(api) handle http errors
diff --git a/staxapp/api.py b/staxapp/api.py
@@ -23,8 +23,6 @@ def handle_api_response(response):
         try:
             response.raise_for_status()
         except requests.exceptions.HTTPError as e:
-            logging.error(f"{response.status_code}: {response.json()}")
-
             raise ApiException(str(e), response)
 
     @classmethod
diff --git a/staxapp/exceptions.py b/staxapp/exceptions.py
@@ -1,14 +1,22 @@
+import logging
+from json.decoder import JSONDecodeError
+
+
 class ApiException(Exception):
     def __init__(self, message, response, detail=""):
-        prefix = f"Api Exception: {response.status_code} - "
-        if detail:
-            prefix = f"{prefix}{detail} "
-        if response.json().get("Error"):
-            print("Some error")
-            self.message = f"{prefix}{response.json()['Error']}"
-        else:
-            print("No Error")
-            self.message = f"{prefix}{message}"
+        try:
+            if response.json().get("Error"):
+                logging.error(f"{response.status_code}: {response.json()}")
+                self.message = f"Api Exception: {response.status_code} -{detail} {response.json()['Error']}"
+            else:
+                logging.error(f"{response.status_code}: {response.json()}")
+                self.message = f"Api Exception:{detail} {message}"
+        except JSONDecodeError:
+            if response.content:
+                logging.error(f"{response.status_code}: {response.content}")
+            else:
+                logging.error(f"{response.status_code}: {message}")
+            self.message = f"Api Exception:{detail} {message}"
 
     def __str__(self):
         return self.message
diff --git a/tests/test_api.py b/tests/test_api.py
@@ -161,7 +161,7 @@ def testFailedPut(self):
         )
         payload = {"Unit": "Test"}
         with self.assertRaises(ApiException):
-            response = self.Api.put("/test/put/400", payload)
+            self.Api.put("/test/put/400", payload)
 
     @responses.activate
     def testDelete(self):
@@ -194,6 +194,41 @@ def testFailedDelete(self):
         with self.assertRaises(ApiException):
             self.Api.delete("/test/delete/exception")
 
+    @responses.activate
+    def testFailedApiException(self):
+        """
+        Test all paths of ApiException
+        """
+        # Test Exception with no error in response
+        response_dict = {}
+        responses.add(
+            responses.GET,
+            f"{Config.api_base_url()}/test/no/error",
+            json=response_dict,
+            status=500,
+        )
+        with self.assertRaises(ApiException):
+            self.Api.get("/test/no/error")
+        
+		# Test an exception which has no json in response
+        responses.add(
+            responses.GET,
+            f"{Config.api_base_url()}/test/invalid/json",
+            body='<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2 Final//EN">\n<title>405 Method Not Allowed</title>\n<h1>Method Not Allowed</h1>\n<p>The method is not allowed for the requested URL.</p>\n',
+            status=405,
+        )
+        with self.assertRaises(ApiException):
+            self.Api.get("/test/invalid/json")
+		
+        # Test an exception with no content
+        responses.add(
+            responses.GET,
+            f"{Config.api_base_url()}/test/no/content",
+            status=500,
+        )
+        with self.assertRaises(ApiException):
+            self.Api.get("/test/no/content")
+        
 
 if __name__ == "__main__":
     unittest.main()
