Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle JSON decode error when parsing error response in client #35

Merged
merged 4 commits into from
Dec 9, 2021
Merged

Handle JSON decode error when parsing error response in client #35

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5a6e0c2
handle JSON decode error when parsing error response
chadawagner Oct 16, 2021
a4d5669
catch only JSONDecodeError
Nov 15, 2021
e3adfc7
raise exception from None
chadawagner Nov 15, 2021
f2f4b1c
handle errors from intermediate proxies in client
chadawagner Nov 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion twirp/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,14 @@ def _make_request(self, *args, url, ctx, request, response_obj, **kwargs):
response = response_obj()
response.ParseFromString(resp.content)
return response
else:
try:
raise exceptions.TwirpServerException.from_json(resp.json())
except:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will always catch the exception. Including exceptions.TwirpServerException

As a result, we will always only get either Unavailable or Unknown

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I was catching the specific error in my local patch, not sure how I missed adding it to the PR. Fixed it, thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks it seems to work now!

if resp.status_code == 503:
code = errors.Errors.Unavailable
else:
code = errors.Errors.Unknown
raise exceptions.TwirpServerException(code=code, message=resp.text)
# Todo: handle error
except requests.exceptions.Timeout as e:
raise exceptions.TwirpServerException(
Expand Down