Spotify rate limit re-trigger loop caused by periodic auth probes
This commit is contained in:
parent
0aa8950436
commit
bde2be1cfa
3 changed files with 91 additions and 22 deletions
|
|
@ -27,10 +27,16 @@ _rate_limit_until = 0 # Unix timestamp when the ban expires (0 = not banne
|
||||||
_rate_limit_retry_after = 0 # Original Retry-After value in seconds
|
_rate_limit_retry_after = 0 # Original Retry-After value in seconds
|
||||||
_rate_limit_endpoint = None # Which function triggered the ban
|
_rate_limit_endpoint = None # Which function triggered the ban
|
||||||
_rate_limit_set_at = 0 # When the ban was set
|
_rate_limit_set_at = 0 # When the ban was set
|
||||||
|
_rate_limit_ban_ended_at = 0 # When the last ban expired naturally (for post-ban cooldown)
|
||||||
|
|
||||||
# Threshold: if Retry-After exceeds this, activate global ban instead of sleeping
|
# Threshold: if Retry-After exceeds this, activate global ban instead of sleeping
|
||||||
_LONG_RATE_LIMIT_THRESHOLD = 60 # seconds
|
_LONG_RATE_LIMIT_THRESHOLD = 60 # seconds
|
||||||
|
|
||||||
|
# After a ban expires, wait this long before making any auth probe calls.
|
||||||
|
# This prevents the "immediate re-probe → re-ban" cycle where Spotify's server-side
|
||||||
|
# cooldown outlasts the Retry-After value they sent us.
|
||||||
|
_POST_BAN_COOLDOWN = 300 # 5 minutes
|
||||||
|
|
||||||
|
|
||||||
class SpotifyRateLimitError(Exception):
|
class SpotifyRateLimitError(Exception):
|
||||||
"""Raised when Spotify API calls are blocked due to active global rate limit ban."""
|
"""Raised when Spotify API calls are blocked due to active global rate limit ban."""
|
||||||
|
|
@ -61,15 +67,41 @@ def _set_global_rate_limit(retry_after_seconds, endpoint_name):
|
||||||
|
|
||||||
def _is_globally_rate_limited():
|
def _is_globally_rate_limited():
|
||||||
"""Check if the global rate limit ban is active."""
|
"""Check if the global rate limit ban is active."""
|
||||||
|
global _rate_limit_ban_ended_at
|
||||||
with _rate_limit_lock:
|
with _rate_limit_lock:
|
||||||
if _rate_limit_until <= 0:
|
if _rate_limit_until <= 0:
|
||||||
return False
|
return False
|
||||||
if time.time() >= _rate_limit_until:
|
if time.time() >= _rate_limit_until:
|
||||||
# Ban expired — clear it
|
# Ban expired — record when it ended so post-ban cooldown can apply
|
||||||
|
if _rate_limit_ban_ended_at < _rate_limit_until:
|
||||||
|
_rate_limit_ban_ended_at = time.time()
|
||||||
|
logger.info("Rate limit ban expired, entering post-ban cooldown period")
|
||||||
return False
|
return False
|
||||||
return True
|
return True
|
||||||
|
|
||||||
|
|
||||||
|
def _is_in_post_ban_cooldown():
|
||||||
|
"""Check if we're in the post-ban cooldown period.
|
||||||
|
After a ban expires, we wait _POST_BAN_COOLDOWN seconds before allowing
|
||||||
|
auth probes to prevent the re-probe → re-ban cycle."""
|
||||||
|
with _rate_limit_lock:
|
||||||
|
if _rate_limit_ban_ended_at <= 0:
|
||||||
|
return False
|
||||||
|
elapsed = time.time() - _rate_limit_ban_ended_at
|
||||||
|
if elapsed < _POST_BAN_COOLDOWN:
|
||||||
|
return True
|
||||||
|
return False
|
||||||
|
|
||||||
|
|
||||||
|
def _get_post_ban_cooldown_remaining():
|
||||||
|
"""Get remaining seconds in post-ban cooldown, or 0 if not in cooldown."""
|
||||||
|
with _rate_limit_lock:
|
||||||
|
if _rate_limit_ban_ended_at <= 0:
|
||||||
|
return 0
|
||||||
|
remaining = _POST_BAN_COOLDOWN - (time.time() - _rate_limit_ban_ended_at)
|
||||||
|
return max(0, int(remaining))
|
||||||
|
|
||||||
|
|
||||||
def _get_rate_limit_info():
|
def _get_rate_limit_info():
|
||||||
"""Get current rate limit ban details. Returns None if not rate limited."""
|
"""Get current rate limit ban details. Returns None if not rate limited."""
|
||||||
with _rate_limit_lock:
|
with _rate_limit_lock:
|
||||||
|
|
@ -90,14 +122,16 @@ def _get_rate_limit_info():
|
||||||
|
|
||||||
|
|
||||||
def _clear_rate_limit():
|
def _clear_rate_limit():
|
||||||
"""Manually clear the global rate limit ban (e.g. after disconnect/reconnect)."""
|
"""Manually clear the global rate limit ban AND post-ban cooldown.
|
||||||
global _rate_limit_until, _rate_limit_retry_after, _rate_limit_endpoint, _rate_limit_set_at
|
Used by disconnect/reconnect so the user can immediately retry."""
|
||||||
|
global _rate_limit_until, _rate_limit_retry_after, _rate_limit_endpoint, _rate_limit_set_at, _rate_limit_ban_ended_at
|
||||||
with _rate_limit_lock:
|
with _rate_limit_lock:
|
||||||
_rate_limit_until = 0
|
_rate_limit_until = 0
|
||||||
_rate_limit_retry_after = 0
|
_rate_limit_retry_after = 0
|
||||||
_rate_limit_endpoint = None
|
_rate_limit_endpoint = None
|
||||||
_rate_limit_set_at = 0
|
_rate_limit_set_at = 0
|
||||||
logger.info("Global rate limit ban cleared")
|
_rate_limit_ban_ended_at = 0
|
||||||
|
logger.info("Global rate limit ban cleared (including post-ban cooldown)")
|
||||||
|
|
||||||
|
|
||||||
def rate_limited(func):
|
def rate_limited(func):
|
||||||
|
|
@ -372,7 +406,8 @@ class SpotifyClient:
|
||||||
|
|
||||||
def is_spotify_authenticated(self) -> bool:
|
def is_spotify_authenticated(self) -> bool:
|
||||||
"""Check if Spotify client is specifically authenticated (not just iTunes fallback).
|
"""Check if Spotify client is specifically authenticated (not just iTunes fallback).
|
||||||
Results are cached for 60 seconds to avoid excessive API calls."""
|
Results are cached for 60 seconds to avoid excessive API calls.
|
||||||
|
During rate limit bans and post-ban cooldown, returns False without making API calls."""
|
||||||
if self.sp is None:
|
if self.sp is None:
|
||||||
return False
|
return False
|
||||||
|
|
||||||
|
|
@ -382,6 +417,14 @@ class SpotifyClient:
|
||||||
if _is_globally_rate_limited():
|
if _is_globally_rate_limited():
|
||||||
return False
|
return False
|
||||||
|
|
||||||
|
# Post-ban cooldown: after a ban expires, don't probe Spotify immediately.
|
||||||
|
# Spotify's server-side cooldown can outlast the Retry-After they sent us,
|
||||||
|
# so probing right away would just re-trigger the ban.
|
||||||
|
if _is_in_post_ban_cooldown():
|
||||||
|
remaining = _get_post_ban_cooldown_remaining()
|
||||||
|
logger.debug(f"Post-ban cooldown active ({remaining}s left), skipping auth probe")
|
||||||
|
return False
|
||||||
|
|
||||||
# Check cache first (lock only for brief read)
|
# Check cache first (lock only for brief read)
|
||||||
with self._auth_cache_lock:
|
with self._auth_cache_lock:
|
||||||
if self._auth_cached_result is not None and (time.time() - self._auth_cache_time) < self._AUTH_CACHE_TTL:
|
if self._auth_cached_result is not None and (time.time() - self._auth_cache_time) < self._AUTH_CACHE_TTL:
|
||||||
|
|
@ -453,7 +496,12 @@ class SpotifyClient:
|
||||||
def clear_rate_limit():
|
def clear_rate_limit():
|
||||||
"""Manually clear the rate limit ban."""
|
"""Manually clear the rate limit ban."""
|
||||||
_clear_rate_limit()
|
_clear_rate_limit()
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def get_post_ban_cooldown_remaining():
|
||||||
|
"""Get remaining seconds in post-ban cooldown, or 0 if not in cooldown."""
|
||||||
|
return _get_post_ban_cooldown_remaining()
|
||||||
|
|
||||||
def _ensure_user_id(self) -> bool:
|
def _ensure_user_id(self) -> bool:
|
||||||
"""Ensure user_id is loaded (may make API call)"""
|
"""Ensure user_id is loaded (may make API call)"""
|
||||||
if self.user_id is None and self.sp is not None:
|
if self.user_id is None and self.sp is not None:
|
||||||
|
|
|
||||||
|
|
@ -97,13 +97,19 @@ class SpotifyWorker:
|
||||||
is_idle = is_actually_running and not self.paused and self.stats['pending'] == 0 and self.current_item is None
|
is_idle = is_actually_running and not self.paused and self.stats['pending'] == 0 and self.current_item is None
|
||||||
|
|
||||||
try:
|
try:
|
||||||
# During rate limit, is_spotify_authenticated() returns False to suppress calls,
|
# During rate limit or post-ban cooldown, report as authenticated
|
||||||
# but we're still authenticated — just banned. Report truthfully.
|
# but don't call is_spotify_authenticated() — that would trigger an
|
||||||
|
# auth probe which resets the rate limit timer.
|
||||||
rate_limited = self.client.is_rate_limited()
|
rate_limited = self.client.is_rate_limited()
|
||||||
authenticated = rate_limited or self.client.is_spotify_authenticated()
|
in_cooldown = self.client.get_post_ban_cooldown_remaining() > 0
|
||||||
|
if rate_limited or in_cooldown:
|
||||||
|
authenticated = True # We're authenticated, just banned/cooling down
|
||||||
|
else:
|
||||||
|
authenticated = self.client.is_spotify_authenticated()
|
||||||
except Exception:
|
except Exception:
|
||||||
authenticated = False
|
authenticated = False
|
||||||
rate_limited = False
|
rate_limited = False
|
||||||
|
in_cooldown = False
|
||||||
|
|
||||||
return {
|
return {
|
||||||
'enabled': True,
|
'enabled': True,
|
||||||
|
|
@ -135,6 +141,14 @@ class SpotifyWorker:
|
||||||
time.sleep(min(remaining, 60)) # Check again every 60s max
|
time.sleep(min(remaining, 60)) # Check again every 60s max
|
||||||
continue
|
continue
|
||||||
|
|
||||||
|
# Post-ban cooldown guard — after ban expires, wait before resuming
|
||||||
|
# to avoid immediately re-triggering the rate limit
|
||||||
|
cooldown = self.client.get_post_ban_cooldown_remaining()
|
||||||
|
if cooldown > 0:
|
||||||
|
logger.debug(f"Post-ban cooldown active ({cooldown}s left), sleeping...")
|
||||||
|
time.sleep(min(cooldown, 60))
|
||||||
|
continue
|
||||||
|
|
||||||
# Auth guard — don't process anything without Spotify auth
|
# Auth guard — don't process anything without Spotify auth
|
||||||
if not self.client.is_spotify_authenticated():
|
if not self.client.is_spotify_authenticated():
|
||||||
# Try reloading config in case user re-authenticated via settings
|
# Try reloading config in case user re-authenticated via settings
|
||||||
|
|
|
||||||
|
|
@ -3529,26 +3529,28 @@ def get_status():
|
||||||
|
|
||||||
# Test Spotify - with caching to avoid excessive API calls
|
# Test Spotify - with caching to avoid excessive API calls
|
||||||
if current_time - _status_cache_timestamps['spotify'] > STATUS_CACHE_TTL:
|
if current_time - _status_cache_timestamps['spotify'] > STATUS_CACHE_TTL:
|
||||||
spotify_start = time.time()
|
# Check is_rate_limited() first — this calls _is_globally_rate_limited() which
|
||||||
# Auth check first — may detect and set a new rate limit ban via probe
|
# records ban-end timestamp if the ban just expired, enabling cooldown detection.
|
||||||
spotify_connected = spotify_client.is_spotify_authenticated()
|
is_rate_limited = spotify_client.is_rate_limited()
|
||||||
spotify_response_time = (time.time() - spotify_start) * 1000
|
rate_limit_info = spotify_client.get_rate_limit_info() if is_rate_limited else None
|
||||||
|
cooldown_remaining = spotify_client.get_post_ban_cooldown_remaining()
|
||||||
|
|
||||||
# Check rate limit AFTER auth (auth probe may have just set the ban)
|
if is_rate_limited or cooldown_remaining > 0:
|
||||||
rate_limit_info = spotify_client.get_rate_limit_info()
|
# During rate limit or post-ban cooldown, skip the auth probe entirely.
|
||||||
|
# Probing Spotify here would reset the rate limit timer.
|
||||||
# During rate limit, is_spotify_authenticated() returns False to suppress calls,
|
|
||||||
# but we still report source as 'spotify' so UI doesn't flip to "Apple Music"
|
|
||||||
if rate_limit_info:
|
|
||||||
music_source = 'spotify'
|
music_source = 'spotify'
|
||||||
|
spotify_response_time = 0
|
||||||
else:
|
else:
|
||||||
|
spotify_start = time.time()
|
||||||
|
spotify_connected = spotify_client.is_spotify_authenticated()
|
||||||
|
spotify_response_time = (time.time() - spotify_start) * 1000
|
||||||
music_source = 'spotify' if spotify_connected else 'itunes'
|
music_source = 'spotify' if spotify_connected else 'itunes'
|
||||||
|
|
||||||
_status_cache['spotify'] = {
|
_status_cache['spotify'] = {
|
||||||
'connected': True, # Always true — iTunes fallback is always available
|
'connected': True, # Always true — iTunes fallback is always available
|
||||||
'response_time': round(spotify_response_time, 1),
|
'response_time': round(spotify_response_time, 1),
|
||||||
'source': music_source,
|
'source': music_source,
|
||||||
'rate_limited': rate_limit_info is not None,
|
'rate_limited': is_rate_limited,
|
||||||
'rate_limit': rate_limit_info
|
'rate_limit': rate_limit_info
|
||||||
}
|
}
|
||||||
_status_cache_timestamps['spotify'] = current_time
|
_status_cache_timestamps['spotify'] = current_time
|
||||||
|
|
@ -34849,10 +34851,15 @@ def _build_status_payload():
|
||||||
soulseek_data['source'] = download_mode
|
soulseek_data['source'] = download_mode
|
||||||
|
|
||||||
# Always include fresh rate limit info (it changes over time as ban expires)
|
# Always include fresh rate limit info (it changes over time as ban expires)
|
||||||
|
# Call is_rate_limited() first to ensure ban-end timestamp is recorded for cooldown
|
||||||
spotify_data = dict(_status_cache.get('spotify', {}))
|
spotify_data = dict(_status_cache.get('spotify', {}))
|
||||||
rate_limit_info = spotify_client.get_rate_limit_info() if spotify_client else None
|
is_rl = spotify_client.is_rate_limited() if spotify_client else False
|
||||||
spotify_data['rate_limited'] = rate_limit_info is not None
|
rate_limit_info = spotify_client.get_rate_limit_info() if is_rl else None
|
||||||
|
cooldown_remaining = spotify_client.get_post_ban_cooldown_remaining() if spotify_client else 0
|
||||||
|
spotify_data['rate_limited'] = is_rl
|
||||||
spotify_data['rate_limit'] = rate_limit_info
|
spotify_data['rate_limit'] = rate_limit_info
|
||||||
|
if cooldown_remaining > 0:
|
||||||
|
spotify_data['post_ban_cooldown'] = cooldown_remaining
|
||||||
|
|
||||||
return {
|
return {
|
||||||
'spotify': spotify_data,
|
'spotify': spotify_data,
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue