VR-7747 change client to only show "message" field from http error response if it has one #1650
Conversation
|
cc @conradoverta this is what you want right? |
| @@ -528,7 +528,12 @@ def raise_for_http_error(response): | |||
| curr_time = timestamp_to_str(now(), utc=True) | |||
| time_str = " at {} UTC".format(curr_time) | |||
|
|
|||
| reason = six.ensure_str(response.text.strip()) | |||
| try: | |||
| reason = six.ensure_str(body_to_json(response)['message']) | |||
There was a problem hiding this comment.
Need to call ensure str, otherwise would get 'ascii' codec can't encode character u'\u201c' in position 18: ordinal not in range(128) error.
There was a problem hiding this comment.
Out of curiosity, did you test this in Python 2? (Could you try it out in both 2 and 3, if you can?)
There was a problem hiding this comment.
Just tried for both. LGTM.
| try: | ||
| reason = body_to_json(response)['message'] | ||
| except (ValueError, # not JSON response | ||
| TypeError, # body_to_json return a single string |
There was a problem hiding this comment.
@convoliution apparently body_to_json could return a string 🤷Specifically, response.json() returns LIMIT_ENDPOINT_NUMBER: currently at maximum number of endpoints when we try to create a second endpoint 🤔
There was a problem hiding this comment.
Oh, that complicates things.
In that case, could you do me a favor and split this try-except into multiple checks?
It was already a shaky idea when I tried catching two error types simultaneously—trying to catch 3 is really prone to issues down the line
Something like (pseudocode)
try:
reason = body_to_json()
except ValueError:
raise
if reason is dict and 'message' in reason:
reason['message']
# etc...
|
🚢 |
Error msg is now:
(I removed the url for privacy reasons).
Basically re-introduce the logic changed in #1083, with some slight modifications.