From 33294930d6487056cca7b00ec3db7eac850f414f Mon Sep 17 00:00:00 2001 From: unsync <1211591+unsync@users.noreply.github.com> Date: Sat, 21 Dec 2024 17:01:29 +0100 Subject: [PATCH 1/2] feature: add rate limiter handler to spotify client --- spotdl/utils/spotify.py | 119 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 116 insertions(+), 3 deletions(-) diff --git a/spotdl/utils/spotify.py b/spotdl/utils/spotify.py index d7669a213..5626db20c 100644 --- a/spotdl/utils/spotify.py +++ b/spotdl/utils/spotify.py @@ -10,7 +10,9 @@ import json import logging -from typing import Dict, Optional +import time +from typing import Dict, Optional, Tuple, Union +from datetime import datetime import requests from spotipy import Spotify @@ -34,6 +36,75 @@ class SpotifyError(Exception): """ +class RateLimitHandler: + """ + Handles rate limiting for Spotify API requests + """ + def __init__(self): + self.rate_limits = {} + self.requests_timestamp = [] + # Spotify's default rate limits + self.max_requests_per_window = 100 + self.window_size = 30 # seconds + + def update_limits(self, response: Union[requests.Response, Dict]) -> None: + """Update rate limit information from response headers or error dict""" + if isinstance(response, requests.Response): + headers = response.headers + self.rate_limits = { + 'remaining': int(headers.get('Retry-After', 0)), + 'reset': int(headers.get('X-RateLimit-Reset', 0)), + 'limit': int(headers.get('X-RateLimit-Limit', self.max_requests_per_window)) + } + elif isinstance(response, dict) and 'error' in response: + error = response['error'] + if error.get('status') == 429: + self.rate_limits = { + 'remaining': int(error.get('Retry-After', 0)), + 'reset': int(time.time() + error.get('Retry-After', 0)), + 'limit': self.max_requests_per_window + } + + def should_retry(self, response: Union[requests.Response, Dict]) -> Tuple[bool, float]: + """Determine if request should be retried and how long to wait""" + # Check if it's a rate limit response + is_rate_limited = False + if isinstance(response, requests.Response): + is_rate_limited = response.status_code == 429 + elif isinstance(response, dict) and 'error' in response: + is_rate_limited = response['error'].get('status') == 429 + + if not is_rate_limited: + return False, 0 + + self.update_limits(response) + wait_time = self.rate_limits.get('remaining', 1) + return True, wait_time + + def track_request(self) -> None: + """Track new request and clean old ones""" + current_time = time.time() + self.requests_timestamp = [ + ts for ts in self.requests_timestamp + if current_time - ts < self.window_size + ] + self.requests_timestamp.append(current_time) + + def should_wait(self) -> Tuple[bool, float]: + """Check if we should wait before making a new request""" + if len(self.requests_timestamp) < self.max_requests_per_window: + return False, 0 + + oldest_timestamp = self.requests_timestamp[0] + current_time = time.time() + time_passed = current_time - oldest_timestamp + + if time_passed < self.window_size: + wait_time = self.window_size - time_passed + return True, wait_time + return False, 0 + + class Singleton(type): """ Singleton metaclass for SpotifyClient. Ensures that SpotifyClient is not @@ -106,6 +177,7 @@ def init( # pylint: disable=bad-mcs-method-argument scope="user-library-read,user-follow-read,playlist-read-private", cache_handler=cache_handler, open_browser=not headless, + requests_timeout=10, ) # Use SpotifyClientCredentials as auth manager else: @@ -113,6 +185,7 @@ def init( # pylint: disable=bad-mcs-method-argument client_id=client_id, client_secret=client_secret, cache_handler=cache_handler, + requests_timeout=10, ) if auth_token is not None: credential_manager = None @@ -127,6 +200,10 @@ def init( # pylint: disable=bad-mcs-method-argument auth=auth_token, auth_manager=credential_manager, status_forcelist=(429, 500, 502, 503, 504, 404), + retries=0, + status_retries=0, + requests_timeout=5, + backoff_factor=1, ) # Return instance @@ -154,6 +231,7 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self._initialized = True + self.rate_limit_handler = RateLimitHandler() use_cache_file: bool = self.use_cache_file # type: ignore # pylint: disable=E1101 cache_file_loc = get_spotify_cache_path() @@ -168,7 +246,7 @@ def __init__(self, *args, **kwargs): def _get(self, url, args=None, payload=None, **kwargs): """ Overrides the get method of the SpotifyClient. - Allows us to cache requests + Allows us to cache requests and handle rate limiting """ use_cache = not self.no_cache # type: ignore # pylint: disable=E1101 @@ -176,6 +254,7 @@ def _get(self, url, args=None, payload=None, **kwargs): if args: kwargs.update(args) + # Check cache first cache_key = None if use_cache: key_obj = dict(kwargs) @@ -187,22 +266,56 @@ def _get(self, url, args=None, payload=None, **kwargs): if self.cache.get(cache_key) is not None: return self.cache[cache_key] + # Rate limit checking + should_wait, wait_time = self.rate_limit_handler.should_wait() + if should_wait: + logger.debug(f"Rate limit approaching, waiting {wait_time:.2f} seconds") + time.sleep(wait_time) + # Wrap in a try-except and retry up to `retries` times. response = None retries = self.max_retries # type: ignore # pylint: disable=E1101 - while response is None: + while response is None and retries > 0: try: + self.rate_limit_handler.track_request() response = self._internal_call("GET", url, payload, kwargs) + + # Handle rate limit response + should_retry, retry_after = self.rate_limit_handler.should_retry(response) + if should_retry: + logger.warning(f"Rate limited, waiting {retry_after} seconds") + time.sleep(retry_after) + retries -= 1 + response = None + continue + except (requests.exceptions.Timeout, requests.ConnectionError) as exc: retries -= 1 if retries <= 0: raise exc + time.sleep(1) # Basic backoff + + if response is None: + raise SpotifyError("Max retries exceeded") if use_cache and cache_key is not None: self.cache[cache_key] = response return response + def get_rate_limit_status(self) -> Dict: + """ + Get current rate limit status + + ### Returns + - Dict containing current rate limits and request count + """ + return { + 'limits': self.rate_limit_handler.rate_limits, + 'requests_in_window': len(self.rate_limit_handler.requests_timestamp), + 'window_size': self.rate_limit_handler.window_size + } + def save_spotify_cache(cache: Dict[str, Optional[Dict]]): """ From 062c588a3c3dc2458d3d1ed9df261f25c9cddccf Mon Sep 17 00:00:00 2001 From: unsync <1211591+unsync@users.noreply.github.com> Date: Thu, 26 Dec 2024 19:10:21 +0100 Subject: [PATCH 2/2] feat: use wider limits --- spotdl/utils/spotify.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/spotdl/utils/spotify.py b/spotdl/utils/spotify.py index 5626db20c..c2e0483d6 100644 --- a/spotdl/utils/spotify.py +++ b/spotdl/utils/spotify.py @@ -43,9 +43,8 @@ class RateLimitHandler: def __init__(self): self.rate_limits = {} self.requests_timestamp = [] - # Spotify's default rate limits - self.max_requests_per_window = 100 - self.window_size = 30 # seconds + self.max_requests_per_window = 50 # Reduced from 100 + self.window_size = 35 # Increased from 30 seconds def update_limits(self, response: Union[requests.Response, Dict]) -> None: """Update rate limit information from response headers or error dict""" @@ -100,7 +99,7 @@ def should_wait(self) -> Tuple[bool, float]: time_passed = current_time - oldest_timestamp if time_passed < self.window_size: - wait_time = self.window_size - time_passed + wait_time = self.window_size - time_passed + 1 # Added 1 second buffer return True, wait_time return False, 0 @@ -202,8 +201,8 @@ def init( # pylint: disable=bad-mcs-method-argument status_forcelist=(429, 500, 502, 503, 504, 404), retries=0, status_retries=0, - requests_timeout=5, - backoff_factor=1, + requests_timeout=10, + backoff_factor=2 ) # Return instance