More robust picking up JSON error responses.
Fixes issue #169.
Another CL towards fixing 169, by making sure we parse the JSON error response
in more cases.
Reviewed in http://codereview.appspot.com/6447045/.
diff --git a/apiclient/errors.py b/apiclient/errors.py
index b58d97f..e744519 100644
--- a/apiclient/errors.py
+++ b/apiclient/errors.py
@@ -41,14 +41,12 @@
def _get_reason(self):
"""Calculate the reason for the error from the response content."""
- if self.resp.get('content-type', '').startswith('application/json'):
- try:
- data = simplejson.loads(self.content)
- reason = data['error']['message']
- except (ValueError, KeyError):
- reason = self.content
- else:
- reason = self.resp.reason
+ reason = self.resp.reason
+ try:
+ data = simplejson.loads(self.content)
+ reason = data['error']['message']
+ except (ValueError, KeyError):
+ pass
return reason
def __repr__(self):
diff --git a/tests/test_errors.py b/tests/test_errors.py
index ab0be28..25e5d2c 100644
--- a/tests/test_errors.py
+++ b/tests/test_errors.py
@@ -45,8 +45,10 @@
}
"""
-def fake_response(data, headers):
- return httplib2.Response(headers), data
+def fake_response(data, headers, reason='Ok'):
+ response = httplib2.Response(headers)
+ response.reason = reason
+ return response, data
class Error(unittest.TestCase):
@@ -55,33 +57,37 @@
def test_json_body(self):
"""Test a nicely formed, expected error response."""
resp, content = fake_response(JSON_ERROR_CONTENT,
- {'status':'400', 'content-type': 'application/json'})
- error = HttpError(resp, content)
- self.assertEqual(str(error), '<HttpError 400 "country is required">')
+ {'status':'400', 'content-type': 'application/json'},
+ reason='Failed')
+ error = HttpError(resp, content, 'http://example.org')
+ self.assertEqual(str(error), '<HttpError 400 when requesting http://example.org returned "country is required">')
def test_bad_json_body(self):
"""Test handling of bodies with invalid json."""
resp, content = fake_response('{',
- {'status':'400', 'content-type': 'application/json'})
+ { 'status':'400', 'content-type': 'application/json'},
+ reason='Failed')
error = HttpError(resp, content)
- self.assertEqual(str(error), '<HttpError 400 "{">')
+ self.assertEqual(str(error), '<HttpError 400 "Failed">')
def test_with_uri(self):
"""Test handling of passing in the request uri."""
resp, content = fake_response('{',
- {'status':'400', 'content-type': 'application/json'})
+ {'status':'400', 'content-type': 'application/json'},
+ reason='Failure')
error = HttpError(resp, content, 'http://example.org')
- self.assertEqual(str(error), '<HttpError 400 when requesting http://example.org returned "{">')
+ self.assertEqual(str(error), '<HttpError 400 when requesting http://example.org returned "Failure">')
def test_missing_message_json_body(self):
"""Test handling of bodies with missing expected 'message' element."""
resp, content = fake_response('{}',
- {'status':'400', 'content-type': 'application/json'})
+ {'status':'400', 'content-type': 'application/json'},
+ reason='Failed')
error = HttpError(resp, content)
- self.assertEqual(str(error), '<HttpError 400 "{}">')
+ self.assertEqual(str(error), '<HttpError 400 "Failed">')
def test_non_json(self):
"""Test handling of non-JSON bodies"""
- resp, content = fake_response('NOT OK', {'status':'400'})
+ resp, content = fake_response('}NOT OK', {'status':'400'})
error = HttpError(resp, content)
self.assertEqual(str(error), '<HttpError 400 "Ok">')
diff --git a/tests/test_http.py b/tests/test_http.py
index 95de47a..d067afe 100644
--- a/tests/test_http.py
+++ b/tests/test_http.py
@@ -354,7 +354,7 @@
RESPONSE = """HTTP/1.1 200 OK
-Content-Type application/json
+Content-Type: application/json
Content-Length: 14
ETag: "etag/pony"\r\n\r\n{"answer": 42}"""
@@ -365,7 +365,7 @@
Content-ID: <randomness+1>
HTTP/1.1 200 OK
-Content-Type application/json
+Content-Type: application/json
Content-Length: 14
ETag: "etag/pony"\r\n\r\n{"foo": 42}
@@ -375,7 +375,7 @@
Content-ID: <randomness+2>
HTTP/1.1 200 OK
-Content-Type application/json
+Content-Type: application/json
Content-Length: 14
ETag: "etag/sheep"\r\n\r\n{"baz": "qux"}
--batch_foobarbaz--"""
@@ -387,7 +387,7 @@
Content-ID: <randomness+1>
HTTP/1.1 200 OK
-Content-Type application/json
+Content-Type: application/json
Content-Length: 14
ETag: "etag/pony"\r\n\r\n{"foo": 42}
@@ -397,11 +397,9 @@
Content-ID: <randomness+2>
HTTP/1.1 403 Access Not Configured
-Content-Type application/json
-Content-Length: 14
-ETag: "etag/sheep"\r\n\r\nContent-Length: 245
-
-{
+Content-Type: application/json
+Content-Length: 245
+ETag: "etag/sheep"\r\n\r\n{
"error": {
"errors": [
{
@@ -425,7 +423,7 @@
Content-ID: <randomness+1>
HTTP/1.1 401 Authorization Required
-Content-Type application/json
+Content-Type: application/json
Content-Length: 14
ETag: "etag/pony"\r\n\r\n{"error": {"message":
"Authorizaton failed."}}
@@ -436,7 +434,7 @@
Content-ID: <randomness+2>
HTTP/1.1 200 OK
-Content-Type application/json
+Content-Type: application/json
Content-Length: 14
ETag: "etag/sheep"\r\n\r\n{"baz": "qux"}
--batch_foobarbaz--"""
@@ -448,7 +446,7 @@
Content-ID: <randomness+1>
HTTP/1.1 200 OK
-Content-Type application/json
+Content-Type: application/json
Content-Length: 14
ETag: "etag/pony"\r\n\r\n{"foo": 42}
--batch_foobarbaz--"""
@@ -708,7 +706,7 @@
self.assertEqual({'foo': 42}, callbacks.responses['1'])
self.assertEqual({'baz': 'qux'}, callbacks.responses['2'])
- def test_execute_http_error(self):
+ def test_execute_batch_http_error(self):
callbacks = Callbacks()
batch = BatchHttpRequest(callback=callbacks.f)
diff --git a/tests/test_json_model.py b/tests/test_json_model.py
index 43b0421..ececdab 100644
--- a/tests/test_json_model.py
+++ b/tests/test_json_model.py
@@ -147,7 +147,7 @@
content = model.response(resp, content)
self.fail('Should have thrown an exception')
except HttpError, e:
- self.assertTrue('Unauthorized' in str(e))
+ self.assertTrue('not authorized' in str(e))
resp['content-type'] = 'application/json'