Parsing a 504 error

[FabioFleitas]

Hey guys,
Sometimes we receive a HTTP 504 response error that is not appropriately being caught. In parse_response when you call payload = resp.json(), it will throw a ValueError("No JSON object could be decoded") error.

What I'm thinking of adding before payload = resp.json():

if resp.status_code == 504:
    raise error.APIConnectionError(resp.content,
        resp.content, resp.status_code, resp)

Thoughts? Happy to submit a PR if you think this is good.

[robinjoseph08]

Hey @FabioFleitas thanks for looking into this. I think this is solid way to handle this case. It's a unique situation because when a 504 occurs, the response returned is empty, so I'm curious to see what value resp.content has. Is it empty as well or is it the status message of Gateway Timeout.

If you submit a PR for it, we'll gladly merge it in!

[FabioFleitas]

You're right, it seems like the response returned is an empty string. Perhaps we could modify it to be:

if resp.status_code == 504:
    raise error.APIConnectionError(resp.content or 'Gateway Timeout',
        resp.content, resp.status_code, resp)

In the chance that resp.content is an empty string?

[robinjoseph08]

Hmm... Try seeing if there's any content in resp.reason. That might have the more generic message that is mapped to the status code.

[FabioFleitas]

Unfortunately our logs didn't save the resp.reason so not sure how I could go about testing to see what it says.

[FabioFleitas]

Perhaps we go with error_msg = resp.content or resp.reason or 'Gateway Timeout' if we're unsure?

[robinjoseph08]

I just quickly tested it using http://httpstat.us/504 and this:

print resp.status_code
if resp.status_code == 504:
    print resp.reason

printed this:

504
Gateway Timeout

So I think relying on resp.reason is a safe bet 👍

[FabioFleitas]

Alright, let's do resp.content or resp.reason in case later down the road there is something in content? I noticed for example that http://httpstat.us/504 had "504 Gateway Timeout" as the content.