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.

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

Verify response type and fix exception reporting #1

Merged
merged 3 commits into from Jun 20, 2014

Conversation

Projects
None yet
2 participants
@oychang
Contributor

oychang commented Jun 16, 2014

Ran into this problem when trying to do a bulk request for around 750 names. The Apache server responded with a 414 request below, and the exceptions from the json module confused me so I thought I would help out the next person that runs into this issue. After all, you gets 2500 requests a day, so this quantity isn't that ridiculous if you don't consider how the sausage is made.

<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<html><head>
<title>414 Request-URI Too Large</title>
</head><body>
<h1>Request-URI Too Large</h1>
<p>The requested URL's length exceeds the capacity
limit for this server.<br />
</p>
<hr>
<address>Apache/2.2.15 (CentOS) Server at vps-1154848-20754.manage.myhosting.com Port 80</address>
</body></html>

oychang added some commits Jun 16, 2014

Fix typo in referencing status code.
There is no such attribute as `Response.status`.
Plus the name of the Response object is `response`, not `r`.

http://docs.python-requests.org/en/latest/api/#requests.Response.status_code
Check response headers before parsing as JSON.
Satisfies the case where the server (not the app) does not respond
properly. Otherwise we end up parsing something that's probably not JSON
and json.loads() throws a very unhelpful ValueError.

SteelPangolin added a commit that referenced this pull request Jun 20, 2014

Merge pull request #1 from oychang/bug/response-failure
Verify response type and fix exception reporting

@SteelPangolin SteelPangolin merged commit a7b9a79 into SteelPangolin:master Jun 20, 2014

@SteelPangolin

This comment has been minimized.

Show comment
Hide comment
@SteelPangolin

SteelPangolin Jun 20, 2014

Owner

Thanks for the patch! Looks helpful. I'll merge it.

Owner

SteelPangolin commented Jun 20, 2014

Thanks for the patch! Looks helpful. I'll merge it.

SteelPangolin pushed a commit that referenced this pull request Nov 27, 2014

SteelPangolin pushed a commit that referenced this pull request Nov 27, 2014

SteelPangolin pushed a commit that referenced this pull request Nov 27, 2014

SteelPangolin added a commit that referenced this pull request Jan 16, 2015

Merge pull request #1 from oychang/bug/response-failure
Verify response type and fix exception reporting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment