fix: Improve support for error_details (googleapis#1126)

parthea · web-flow · commit e6a1da3542e2 · 2020-12-09T22:00:03.000Z
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly: - [x] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/google-api-python-client/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [x] Ensure the tests and linter pass - [x] Code coverage does not decrease (if any source code was changed) - [x] Appropriate docs were updated (if necessary) Fixes googleapis#990 🦕
diff --git a/googleapiclient/errors.py b/googleapiclient/errors.py
@@ -48,7 +48,11 @@ def _get_reason(self):
         """Calculate the reason for the error from the response content."""
         reason = self.resp.reason
         try:
-            data = json.loads(self.content.decode("utf-8"))
+            try:
+                data = json.loads(self.content.decode("utf-8"))
+            except json.JSONDecodeError:
+                # In case it is not json
+                data = self.content.decode("utf-8")
             if isinstance(data, dict):
                 reason = data["error"]["message"]
                 error_detail_keyword = next((kw for kw in ["detail", "details", "message"] if kw in data["error"]), "")
@@ -59,6 +63,8 @@ def _get_reason(self):
                 reason = first_error["error"]["message"]
                 if "details" in first_error["error"]:
                     self.error_details = first_error["error"]["details"]
+            else:
+                self.error_details = data
         except (ValueError, KeyError, TypeError):
             pass
         if reason is None:
diff --git a/tests/test_errors.py b/tests/test_errors.py
@@ -94,7 +94,7 @@ def test_bad_json_body(self):
             b"{", {"status": "400", "content-type": "application/json"}, reason="Failed"
         )
         error = HttpError(resp, content)
-        self.assertEqual(str(error), '<HttpError 400 "Failed">')
+        self.assertEqual(str(error), '<HttpError 400 when requesting None returned "Failed". Details: "{">')
 
     def test_with_uri(self):
         """Test handling of passing in the request uri."""
@@ -106,7 +106,7 @@ def test_with_uri(self):
         error = HttpError(resp, content, uri="http://example.org")
         self.assertEqual(
             str(error),
-            '<HttpError 400 when requesting http://example.org returned "Failure">',
+            '<HttpError 400 when requesting http://example.org returned "Failure". Details: "{">',
         )
 
     def test_missing_message_json_body(self):
@@ -121,15 +121,15 @@ def test_missing_message_json_body(self):
 
     def test_non_json(self):
         """Test handling of non-JSON bodies"""
-        resp, content = fake_response(b"}NOT OK", {"status": "400"})
+        resp, content = fake_response(b"Invalid request", {"status": "400"})
         error = HttpError(resp, content)
-        self.assertEqual(str(error), '<HttpError 400 "Ok">')
+        self.assertEqual(str(error), '<HttpError 400 when requesting None returned "Ok". Details: "Invalid request">')
 
     def test_missing_reason(self):
         """Test an empty dict with a missing resp.reason."""
         resp, content = fake_response(b"}NOT OK", {"status": "400"}, reason=None)
         error = HttpError(resp, content)
-        self.assertEqual(str(error), '<HttpError 400 "">')
+        self.assertEqual(str(error), '<HttpError 400 when requesting None returned "". Details: "}NOT OK">')
 
     def test_error_detail_for_missing_message_in_error(self):
         """Test handling of data with missing 'details' or 'detail' element."""
