Skip to content

Commit

Permalink
fix: Improve support for error_details (#1126)
Browse files Browse the repository at this point in the history
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 #990 馃
  • Loading branch information
parthea committed Dec 9, 2020
1 parent 8325d24 commit e6a1da3
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 6 deletions.
8 changes: 7 additions & 1 deletion googleapiclient/errors.py
Expand Up @@ -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"]), "")
Expand All @@ -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:
Expand Down
10 changes: 5 additions & 5 deletions tests/test_errors.py
Expand Up @@ -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."""
Expand All @@ -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):
Expand All @@ -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."""
Expand Down

0 comments on commit e6a1da3

Please sign in to comment.