Fixes #302 simplifies json decoding and handling of errors. Adds tests.

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"""

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