From fa2122127cd84e0acd25189cb0713dfc1e4f0919 Mon Sep 17 00:00:00 2001 From: cash Date: Sat, 25 Jan 2014 16:56:48 -0500 Subject: [PATCH 1/2] Fixes #302 simplifies json decoding and handling of errors. Adds tests. --- tests/test_core.py | 68 +++++++++++++++++++++++++++++++++++++++++++--- twython/api.py | 52 +++++++++++++++++------------------ 2 files changed, 89 insertions(+), 31 deletions(-) diff --git a/tests/test_core.py b/tests/test_core.py index caf07ae..df20660 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -1,4 +1,4 @@ -from twython import Twython, TwythonError, TwythonAuthError +from twython import Twython, TwythonError, TwythonAuthError, TwythonRateLimitError from .config import ( test_tweet_object, test_tweet_html, unittest @@ -27,10 +27,11 @@ class TwythonAPITestCase(unittest.TestCase): """Convenience function for mapping from endpoint to URL""" return '%s/%s.json' % (self.api.api_url % self.api.api_version, endpoint) - def register_response(self, method, url, body='', match_querystring=False, + def register_response(self, method, url, body='{}', match_querystring=False, status=200, adding_headers=None, stream=False, - content_type='text/plain'): - """Temporary function to work around python 3.3 issue with responses""" + content_type='application/json; charset=utf-8'): + """Wrapper function for responses for simpler unit tests""" + # responses uses BytesIO to hold the body so it needs to be in bytes if not is_py2: body = bytes(body, 'UTF-8') @@ -197,6 +198,65 @@ class TwythonAPITestCase(unittest.TestCase): get_mock.side_effect = requests.RequestException("hostname 'example.com' doesn't match ...") self.assertRaises(TwythonError, self.api.get, 'https://example.com') + @responses.activate + def test_request_should_get_convert_json_to_data(self): + """Test that Twython converts JSON data to a Python object""" + endpoint = 'statuses/show' + url = self.get_url(endpoint) + self.register_response(responses.GET, url, body='{"id": 210462857140252672}') + + data = self.api.request(endpoint, params={'id': 210462857140252672}) + + self.assertEqual({'id': 210462857140252672}, data) + + @responses.activate + def test_request_should_raise_exception_with_invalid_json(self): + """Test that Twython handles invalid JSON (though Twitter should not return it)""" + endpoint = 'statuses/show' + url = self.get_url(endpoint) + self.register_response(responses.GET, url, body='{"id: 210462857140252672}') + + self.assertRaises(TwythonError, self.api.request, endpoint, params={'id': 210462857140252672}) + + @responses.activate + def test_request_should_handle_401(self): + """Test that Twython raises an auth error on 401 error""" + endpoint = 'statuses/home_timeline' + url = self.get_url(endpoint) + self.register_response(responses.GET, url, body='{"errors":[{"message":"Error"}]}', status=401) + + self.assertRaises(TwythonAuthError, self.api.request, endpoint) + + @responses.activate + def test_request_should_handle_400_for_missing_auth_data(self): + """Test that Twython raises an auth error on 400 error when no oauth data sent""" + endpoint = 'statuses/home_timeline' + url = self.get_url(endpoint) + self.register_response(responses.GET, url, + body='{"errors":[{"message":"Bad Authentication data"}]}', status=400) + + self.assertRaises(TwythonAuthError, self.api.request, endpoint) + + @responses.activate + def test_request_should_handle_400_that_is_not_auth_related(self): + """Test that Twython raises a normal error on 400 error when unrelated to authorization""" + endpoint = 'statuses/home_timeline' + url = self.get_url(endpoint) + self.register_response(responses.GET, url, + body='{"errors":[{"message":"Bad request"}]}', status=400) + + self.assertRaises(TwythonError, self.api.request, endpoint) + + @responses.activate + def test_request_should_handle_rate_limit(self): + """Test that Twython raises an rate limit error on 429""" + endpoint = 'statuses/home_timeline' + url = self.get_url(endpoint) + self.register_response(responses.GET, url, + body='{"errors":[{"message":"Rate Limit"}]}', status=429) + + self.assertRaises(TwythonRateLimitError, self.api.request, endpoint) + @responses.activate def test_get_lastfunction_header_should_return_header(self): """Test getting last specific header of the last API call works""" diff --git a/twython/api.py b/twython/api.py index ef1aac6..c457fd7 100644 --- a/twython/api.py +++ b/twython/api.py @@ -143,7 +143,6 @@ class Twython(EndpointsMixin, object): response = func(url, **requests_args) except requests.RequestException as e: raise TwythonError(str(e)) - content = response.content.decode('utf-8') # create stash for last function intel self._last_call = { @@ -153,37 +152,18 @@ class Twython(EndpointsMixin, object): 'headers': response.headers, 'status_code': response.status_code, 'url': response.url, - 'content': content, + 'content': response.text, } - # Wrap the json loads in a try, and defer an error - # Twitter will return invalid json with an error code in the headers - json_error = False - try: - try: - # try to get json - content = content.json() - except AttributeError: - # if unicode detected - content = json.loads(content) - except ValueError: - json_error = True - content = {} - + # greater than 304 (not modified) is an error if response.status_code > 304: - # If there is no error message, use a default. - errors = content.get('errors', - [{'message': 'An error occurred processing your request.'}]) - if errors and isinstance(errors, list): - error_message = errors[0]['message'] - else: - error_message = errors # pragma: no cover + error_message = self._get_error_message(response) self._last_call['api_error'] = error_message ExceptionType = TwythonError if response.status_code == 429: # Twitter API 1.1, always return 429 when rate limit is exceeded - ExceptionType = TwythonRateLimitError # pragma: no cover + ExceptionType = TwythonRateLimitError elif response.status_code == 401 or 'Bad Authentication data' in error_message: # Twitter API 1.1, returns a 401 Unauthorized or # a 400 "Bad Authentication data" for invalid/expired app keys/user tokens @@ -193,12 +173,30 @@ class Twython(EndpointsMixin, object): error_code=response.status_code, retry_after=response.headers.get('retry-after')) - # if we have a json error here, then it's not an official Twitter API error - if json_error and not response.status_code in (200, 201, 202): # pragma: no cover - raise TwythonError('Response was not valid JSON, unable to decode.') + try: + content = response.json() + except json.JSONDecodeError: + raise TwythonError('Response was not valid JSON. Unable to decode.') return content + def _get_error_message(self, response): + """Parse and return the first error message""" + + error_message = 'An error occurred processing your request.' + try: + content = response.json() + # {"errors":[{"code":34,"message":"Sorry, that page does not exist"}]} + error_message = content['errors'][0]['message'] + except json.JSONDecodeError: + # bad json data from Twitter for an error + pass + except (KeyError, IndexError): + # missing data so fallback to default message + pass + + return error_message + def request(self, endpoint, method='GET', params=None, version='1.1'): """Return dict of response received from Twitter's API From 1e627f9fb1cd0bc06704739e4426f6d1218352ac Mon Sep 17 00:00:00 2001 From: cash Date: Sat, 25 Jan 2014 17:15:17 -0500 Subject: [PATCH 2/2] need to use ValueError to account for different json libraries throwing different exceptions --- twython/api.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/twython/api.py b/twython/api.py index c457fd7..db79381 100644 --- a/twython/api.py +++ b/twython/api.py @@ -175,7 +175,7 @@ class Twython(EndpointsMixin, object): try: content = response.json() - except json.JSONDecodeError: + except ValueError: raise TwythonError('Response was not valid JSON. Unable to decode.') return content @@ -188,7 +188,7 @@ class Twython(EndpointsMixin, object): content = response.json() # {"errors":[{"code":34,"message":"Sorry, that page does not exist"}]} error_message = content['errors'][0]['message'] - except json.JSONDecodeError: + except ValueError: # bad json data from Twitter for an error pass except (KeyError, IndexError):