diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c8ac5b..afe7865 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/spotipy/client.py b/spotipy/client.py index 8c1fc8a..9704b9c 100644 --- a/spotipy/client.py +++ b/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"] - except (ValueError, KeyError): - msg = "error" - try: - reason = response.json()["error"]["reason"] - except (ValueError, KeyError): + json_response = response.json() + error = json_response.get("error", {}) + msg = error.get("message") + reason = error.get("reason") + except ValueError: + # 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', - method, url, response.status_code, msg) + logger.error( + '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, diff --git a/spotipy/oauth2.py b/spotipy/oauth2.py index aba2eb2..a7ce334 100644 --- a/spotipy/oauth2.py +++ b/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,23 +253,20 @@ class SpotifyClientCredentials(SpotifyAuthBase): self.OAUTH_TOKEN_URL, headers, payload ) - response = self._session.post( - self.OAUTH_TOKEN_URL, - data=payload, - headers=headers, - verify=True, - proxies=self.proxies, - timeout=self.requests_timeout, - ) - 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() - return token_info + try: + response = self._session.post( + self.OAUTH_TOKEN_URL, + data=payload, + headers=headers, + verify=True, + proxies=self.proxies, + timeout=self.requests_timeout, + ) + response.raise_for_status() + 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: - raise SpotifyStateError(self.state, server.state) - - if server.auth_code is not None: - return server.auth_code - elif server.error is not None: + if 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,25 +547,22 @@ class SpotifyOAuth(SpotifyAuthBase): self.OAUTH_TOKEN_URL, headers, payload ) - response = self._session.post( - self.OAUTH_TOKEN_URL, - data=payload, - headers=headers, - verify=True, - proxies=self.proxies, - timeout=self.requests_timeout, - ) - 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) - self.cache_handler.save_token_to_cache(token_info) - return token_info if as_dict else token_info["access_token"] + try: + response = self._session.post( + self.OAUTH_TOKEN_URL, + data=payload, + headers=headers, + verify=True, + proxies=self.proxies, + timeout=self.requests_timeout, + ) + response.raise_for_status() + 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,28 +577,23 @@ class SpotifyOAuth(SpotifyAuthBase): self.OAUTH_TOKEN_URL, headers, payload ) - response = self._session.post( - self.OAUTH_TOKEN_URL, - data=payload, - headers=headers, - proxies=self.proxies, - timeout=self.requests_timeout, - ) - - 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 + try: + response = self._session.post( + self.OAUTH_TOKEN_URL, + data=payload, + headers=headers, + proxies=self.proxies, + timeout=self.requests_timeout, + ) + response.raise_for_status() + 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,26 +911,22 @@ class SpotifyPKCE(SpotifyAuthBase): self.OAUTH_TOKEN_URL, headers, payload ) - response = self._session.post( - self.OAUTH_TOKEN_URL, - data=payload, - headers=headers, - verify=True, - proxies=self.proxies, - timeout=self.requests_timeout, - ) - if response.status_code != 200: - 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"] + try: + response = self._session.post( + self.OAUTH_TOKEN_URL, + data=payload, + headers=headers, + verify=True, + proxies=self.proxies, + timeout=self.requests_timeout, + ) + response.raise_for_status() + 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,28 +942,23 @@ class SpotifyPKCE(SpotifyAuthBase): self.OAUTH_TOKEN_URL, headers, payload ) - response = self._session.post( - self.OAUTH_TOKEN_URL, - data=payload, - headers=headers, - proxies=self.proxies, - timeout=self.requests_timeout, - ) - - 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 + try: + response = self._session.post( + self.OAUTH_TOKEN_URL, + data=payload, + headers=headers, + proxies=self.proxies, + timeout=self.requests_timeout, + ) + response.raise_for_status() + 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