Fallback on decoding error responses into string if decoding into JSON fails (#689)

* Fixed a bug in the initializers for the auth managers that produced a spurious warning message if you provide a cache handler and you set a value for the "SPOTIPY_CLIENT_USERNAME" environment variable.

* fixed a bug in _get_auth_response_local_server which occurs if you provide a state parameter and click cancel in the authorization dialog

* Fallback on decoding the error response body into text if decoding into JSON fails.

* Fall back on decoding oauth errors into text if decoding into JSON fails.

* Fixed unused `http_error` name in exeception clause.

* Updated CHANGELOG

Co-authored-by: Stéphane Bruckert <stephane.bruckert@gmail.com>

CHANGELOG.md

@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 
 * Added `MemoryCacheHandler`, a cache handler that simply stores the token info in memory as an instance attribute of this class.
+* If a network request returns an error status code but the response body cannot be decoded into JSON, then fall back on decoding the body into a string.
 * Added `DjangoSessionCacheHandler`, a cache handler that stores the token in the session framework provided by Django. Web apps using spotipy with Django can directly use this for cache handling.
 
 ### Fixed

spotipy/client.py

@@ -247,16 +247,22 @@ class Spotify(object):
         except requests.exceptions.HTTPError as http_error:
             response = http_error.response
             try:
-                msg = response.json()["error"]["message"]
+                json_response = response.json()
-            except (ValueError, KeyError):
+                error = json_response.get("error", {})
-                msg = "error"
+                msg = error.get("message")
-            try:
+                reason = error.get("reason")
-                reason = response.json()["error"]["reason"]
+            except ValueError:
-            except (ValueError, KeyError):
+                # if the response cannnot be decoded into JSON (which raises a ValueError),
+                # then try to decode it into text
+
+                # if we receive an empty string (which is falsy), then replace it with `None`
+                msg = response.text or None
                 reason = None
 
-            logger.error('HTTP Error for %s to %s returned %s due to %s',
+            logger.error(
-                         method, url, response.status_code, msg)
+                'HTTP Error for %s to %s with Params: %s returned %s due to %s',
+                method, url, args.get("params"), response.status_code, msg
+            )
 
             raise SpotifyException(
                 response.status_code,

spotipy/oauth2.py

@@ -129,6 +129,28 @@ class SpotifyAuthBase(object):
         )
         return needle_scope <= haystack_scope
 
+    def _handle_oauth_error(self, http_error):
+        response = http_error.response
+        try:
+            error_payload = response.json()
+            error = error_payload.get('error')
+            error_description = error_payload.get('error_description')
+        except ValueError:
+            # if the response cannnot be decoded into JSON (which raises a ValueError),
+            # then try do decode it into text
+
+            # if we receive an empty string (which is falsy), then replace it with `None`
+            error = response.txt or None
+            error_description = None
+
+        raise SpotifyOauthError(
+            'error: {0}, error_description: {1}'.format(
+                error, error_description
+            ),
+            error=error,
+            error_description=error_description
+        )
+
     def __del__(self):
         """Make sure the connection (pool) gets closed"""
         if isinstance(self._session, requests.Session):
@@ -231,6 +253,7 @@ class SpotifyClientCredentials(SpotifyAuthBase):
             self.OAUTH_TOKEN_URL, headers, payload
         )
 
+        try:
             response = self._session.post(
                 self.OAUTH_TOKEN_URL,
                 data=payload,
@@ -239,15 +262,11 @@ class SpotifyClientCredentials(SpotifyAuthBase):
                 proxies=self.proxies,
                 timeout=self.requests_timeout,
             )
-        if response.status_code != 200:
+            response.raise_for_status()
-            error_payload = response.json()
-            raise SpotifyOauthError(
-                'error: {0}, error_description: {1}'.format(
-                    error_payload['error'], error_payload['error_description']),
-                error=error_payload['error'],
-                error_description=error_payload['error_description'])
             token_info = response.json()
             return token_info
+        except requests.exceptions.HTTPError as http_error:
+            self._handle_oauth_error(http_error)
 
     def _add_custom_values_to_token_info(self, token_info):
         """
@@ -439,13 +458,12 @@ class SpotifyOAuth(SpotifyAuthBase):
         self._open_auth_url()
         server.handle_request()
 
-        if self.state is not None and server.state != self.state:
+        if server.error is not None:
-            raise SpotifyStateError(self.state, server.state)
-
-        if server.auth_code is not None:
-            return server.auth_code
-        elif server.error is not None:
             raise server.error
+        elif self.state is not None and server.state != self.state:
+            raise SpotifyStateError(self.state, server.state)
+        elif server.auth_code is not None:
+            return server.auth_code
         else:
             raise SpotifyOauthError("Server listening on localhost has not been accessed")
 
@@ -529,6 +547,7 @@ class SpotifyOAuth(SpotifyAuthBase):
             self.OAUTH_TOKEN_URL, headers, payload
         )
 
+        try:
             response = self._session.post(
                 self.OAUTH_TOKEN_URL,
                 data=payload,
@@ -537,17 +556,13 @@ class SpotifyOAuth(SpotifyAuthBase):
                 proxies=self.proxies,
                 timeout=self.requests_timeout,
             )
-        if response.status_code != 200:
+            response.raise_for_status()
-            error_payload = response.json()
-            raise SpotifyOauthError(
-                'error: {0}, error_description: {1}'.format(
-                    error_payload['error'], error_payload['error_description']),
-                error=error_payload['error'],
-                error_description=error_payload['error_description'])
             token_info = response.json()
             token_info = self._add_custom_values_to_token_info(token_info)
             self.cache_handler.save_token_to_cache(token_info)
             return token_info if as_dict else token_info["access_token"]
+        except requests.exceptions.HTTPError as http_error:
+            self._handle_oauth_error(http_error)
 
     def refresh_access_token(self, refresh_token):
         payload = {
@@ -562,6 +577,7 @@ class SpotifyOAuth(SpotifyAuthBase):
             self.OAUTH_TOKEN_URL, headers, payload
         )
 
+        try:
             response = self._session.post(
                 self.OAUTH_TOKEN_URL,
                 data=payload,
@@ -569,21 +585,15 @@ class SpotifyOAuth(SpotifyAuthBase):
                 proxies=self.proxies,
                 timeout=self.requests_timeout,
             )
-
+            response.raise_for_status()
-        if response.status_code != 200:
-            error_payload = response.json()
-            raise SpotifyOauthError(
-                'error: {0}, error_description: {1}'.format(
-                    error_payload['error'], error_payload['error_description']),
-                error=error_payload['error'],
-                error_description=error_payload['error_description'])
-
             token_info = response.json()
             token_info = self._add_custom_values_to_token_info(token_info)
             if "refresh_token" not in token_info:
                 token_info["refresh_token"] = refresh_token
             self.cache_handler.save_token_to_cache(token_info)
             return token_info
+        except requests.exceptions.HTTPError as http_error:
+            self._handle_oauth_error(http_error)
 
     def _add_custom_values_to_token_info(self, token_info):
         """
@@ -901,6 +911,7 @@ class SpotifyPKCE(SpotifyAuthBase):
             self.OAUTH_TOKEN_URL, headers, payload
         )
 
+        try:
             response = self._session.post(
                 self.OAUTH_TOKEN_URL,
                 data=payload,
@@ -909,18 +920,13 @@ class SpotifyPKCE(SpotifyAuthBase):
                 proxies=self.proxies,
                 timeout=self.requests_timeout,
             )
-        if response.status_code != 200:
+            response.raise_for_status()
-            error_payload = response.json()
-            raise SpotifyOauthError('error: {0}, error_descr: {1}'.format(error_payload['error'],
-                                                                          error_payload[
-                                                                              'error_description'
-            ]),
-                error=error_payload['error'],
-                error_description=error_payload['error_description'])
             token_info = response.json()
             token_info = self._add_custom_values_to_token_info(token_info)
             self.cache_handler.save_token_to_cache(token_info)
             return token_info["access_token"]
+        except requests.exceptions.HTTPError as http_error:
+            self._handle_oauth_error(http_error)
 
     def refresh_access_token(self, refresh_token):
         payload = {
@@ -936,6 +942,7 @@ class SpotifyPKCE(SpotifyAuthBase):
             self.OAUTH_TOKEN_URL, headers, payload
         )
 
+        try:
             response = self._session.post(
                 self.OAUTH_TOKEN_URL,
                 data=payload,
@@ -943,21 +950,15 @@ class SpotifyPKCE(SpotifyAuthBase):
                 proxies=self.proxies,
                 timeout=self.requests_timeout,
             )
-
+            response.raise_for_status()
-        if response.status_code != 200:
-            error_payload = response.json()
-            raise SpotifyOauthError(
-                'error: {0}, error_description: {1}'.format(
-                    error_payload['error'], error_payload['error_description']),
-                error=error_payload['error'],
-                error_description=error_payload['error_description'])
-
             token_info = response.json()
             token_info = self._add_custom_values_to_token_info(token_info)
             if "refresh_token" not in token_info:
                 token_info["refresh_token"] = refresh_token
             self.cache_handler.save_token_to_cache(token_info)
             return token_info
+        except requests.exceptions.HTTPError as http_error:
+            self._handle_oauth_error(http_error)
 
     def parse_response_code(self, url):
         """ Parse the response code in the given response url