diff --git a/config/settings.py b/config/settings.py index 18c42124..2cfc3eeb 100644 --- a/config/settings.py +++ b/config/settings.py @@ -466,7 +466,14 @@ class ConfigManager: "download_path": "./downloads", "transfer_path": "./Transfer", "max_peer_queue": 0, - "download_timeout": 600 + "download_timeout": 600, + # Reddit report (YeloMelo95, Bell Canada): the existing + # 35-per-220s sliding-window cap allows all 35 searches in + # rapid succession before throttling — that burst trips ISP + # anti-abuse. This knob forces a min gap between consecutive + # searches even when the window cap isn't hit. 0 = disabled + # (preserves prior behavior). + "search_min_delay_seconds": 0, }, "download_source": { "mode": "soulseek", # Options: "soulseek", "youtube", "tidal", "qobuz", "hifi", "hybrid" diff --git a/core/soulseek_client.py b/core/soulseek_client.py index 1f7cb9c7..6b928688 100644 --- a/core/soulseek_client.py +++ b/core/soulseek_client.py @@ -48,18 +48,87 @@ _SLSKD_DEFAULT_TIMEOUT = aiohttp.ClientTimeout( ) +# Search-rate-limit defaults. Pre-fix these were hardcoded magic numbers +# inside `SoulseekClient.__init__`. Lifted to module level so they're +# greppable + bumpable in one place, and so the reddit-reported case +# (Bell Canada anti-abuse trips on slskd peer-connection bursts) can +# tune them via `soulseek.search_*` config without touching code. +_DEFAULT_MAX_PER_WINDOW = 35 +_DEFAULT_WINDOW_SECONDS = 220 +_DEFAULT_MIN_DELAY_SECONDS = 0 # 0 = disabled (preserves prior behavior) + + +def compute_search_wait_seconds( + timestamps: List[float], + last_search_at: float, + now: float, + *, + max_per_window: int, + window_seconds: float, + min_delay_seconds: float, +) -> float: + """Pure scheduler for the slskd search throttle. + + Returns how many seconds the caller should sleep before issuing + the next search. ``timestamps`` is the list of recent search + timestamps already pruned to the current window (caller's job). + ``last_search_at`` is the timestamp of the most recent search + (0.0 if there hasn't been one). ``now`` is the current monotonic / + wall-clock time (caller chooses — pure function only does math). + + Two independent gates, return the larger: + + 1. **Sliding-window cap** — when ``len(timestamps) >= max_per_window``, + sleep until the oldest timestamp ages out of the window. Same + semantics as the pre-fix hardcoded behavior. + + 2. **Min-delay between searches** — when ``min_delay_seconds > 0``, + sleep until at least that many seconds have passed since + ``last_search_at``. Smooths bursts even when the window isn't + full — this is the actual fix for the Reddit-reported case where + Bell Canada's anti-abuse trips on the rapid peer-connection + bursts that 35 back-to-back searches generate. + + Returns 0.0 (no wait) when ``min_delay_seconds`` is 0 / negative + AND the window isn't full. Pure: no I/O, no side effects, no + mutation of the inputs. + """ + window_wait = 0.0 + if max_per_window > 0 and len(timestamps) >= max_per_window: + oldest = timestamps[0] + window_wait = max(0.0, oldest + window_seconds - now) + + delay_wait = 0.0 + if min_delay_seconds > 0 and last_search_at > 0: + elapsed = now - last_search_at + delay_wait = max(0.0, min_delay_seconds - elapsed) + + return max(window_wait, delay_wait) + + class SoulseekClient(DownloadSourcePlugin): def __init__(self): self.base_url: Optional[str] = None self.api_key: Optional[str] = None self.download_path: Path = Path("./downloads") self.active_searches: Dict[str, bool] = {} # search_id -> still_active - - # Rate limiting for searches - self.search_timestamps: List[float] = [] # Track search timestamps - self.max_searches_per_window = 35 # Conservative limit to prevent Soulseek bans - self.rate_limit_window = 220 # seconds (3 minutes 40 seconds) - + + # Rate limiting for searches. Cap + window stay hardcoded — + # nobody has reported issues with the 35/220 defaults. The + # min-delay knob is the actual fix for the Reddit-reported + # case (Bell Canada anti-abuse cuts the WAN after rapid + # peer-connection bursts) — smooths bursts even when the + # sliding-window cap isn't hit. 0 = disabled (preserves prior + # behavior). + self.search_timestamps: List[float] = [] + self._last_search_at: float = 0.0 + self.max_searches_per_window = _DEFAULT_MAX_PER_WINDOW + self.rate_limit_window = _DEFAULT_WINDOW_SECONDS + self.search_min_delay_seconds = float( + config_manager.get('soulseek.search_min_delay_seconds', _DEFAULT_MIN_DELAY_SECONDS) + or _DEFAULT_MIN_DELAY_SECONDS + ) + self._setup_client() def _setup_client(self): @@ -103,22 +172,36 @@ class SoulseekClient(DownloadSourcePlugin): self.search_timestamps = [ts for ts in self.search_timestamps if ts > cutoff_time] async def _wait_for_rate_limit(self): - """Wait if necessary to respect rate limiting""" + """Wait if necessary to respect search rate limits. + + Delegates the wait math to ``compute_search_wait_seconds`` so + the throttle logic is testable independently of asyncio.sleep + and the singleton client. Two gates apply (max wins): sliding- + window cap on searches per N seconds, plus optional min-delay + between consecutive searches (the burst-smoother). + """ self._clean_old_timestamps() - - if len(self.search_timestamps) >= self.max_searches_per_window: - # Calculate how long to wait - oldest_timestamp = self.search_timestamps[0] - wait_time = oldest_timestamp + self.rate_limit_window - time.time() - - if wait_time > 0: - logger.info(f"Rate limit reached ({len(self.search_timestamps)}/{self.max_searches_per_window} searches). Waiting {wait_time:.1f} seconds...") - await asyncio.sleep(wait_time) - # Clean up again after waiting - self._clean_old_timestamps() - + wait_time = compute_search_wait_seconds( + self.search_timestamps, + self._last_search_at, + time.time(), + max_per_window=self.max_searches_per_window, + window_seconds=self.rate_limit_window, + min_delay_seconds=self.search_min_delay_seconds, + ) + if wait_time > 0: + logger.info( + f"Search rate limit: waiting {wait_time:.1f}s " + f"({len(self.search_timestamps)}/{self.max_searches_per_window} in window, " + f"min_delay={self.search_min_delay_seconds:.1f}s)" + ) + await asyncio.sleep(wait_time) + self._clean_old_timestamps() + # Record this search attempt - self.search_timestamps.append(time.time()) + now = time.time() + self.search_timestamps.append(now) + self._last_search_at = now def get_rate_limit_status(self) -> Dict[str, Any]: """Get current rate limiting status""" diff --git a/tests/test_soulseek_search_throttle.py b/tests/test_soulseek_search_throttle.py new file mode 100644 index 00000000..e476656e --- /dev/null +++ b/tests/test_soulseek_search_throttle.py @@ -0,0 +1,254 @@ +"""Pin `compute_search_wait_seconds` — the pure scheduler behind the +slskd search throttle. + +Reddit report (YeloMelo95, Bell Canada): ISP anti-abuse cuts the user's +WAN connection after a burst of slskd searches. The pre-fix throttle +was hardcoded to 35 searches per 220s sliding window, which allowed all +35 in rapid succession and only blocked once the cap was hit. That's +fine for soulseek-side bans but doesn't smooth bursts at the ISP layer. + +Fix lifts the cap + window to config and adds a new `min_delay_seconds` +knob. The pure helper takes the throttle inputs and returns how long to +sleep — easy to test independently of asyncio.sleep / the singleton +client / wall-clock time. +""" + +from __future__ import annotations + +import pytest + +from core.soulseek_client import compute_search_wait_seconds + + +# --------------------------------------------------------------------------- +# Defaults / no-throttle path +# --------------------------------------------------------------------------- + + +class TestNoThrottleNeeded: + def test_empty_state_returns_zero(self): + """First search ever → no timestamps, no last-search → no wait.""" + assert compute_search_wait_seconds( + timestamps=[], + last_search_at=0.0, + now=100.0, + max_per_window=35, + window_seconds=220, + min_delay_seconds=0, + ) == 0.0 + + def test_below_window_cap_returns_zero(self): + """When timestamps haven't filled the window cap and min-delay + is disabled, no wait. Preserves prior behavior for existing + users who don't tune the new knob.""" + assert compute_search_wait_seconds( + timestamps=[10.0, 20.0, 30.0], + last_search_at=30.0, + now=100.0, + max_per_window=35, + window_seconds=220, + min_delay_seconds=0, + ) == 0.0 + + def test_min_delay_zero_is_disabled(self): + """Explicit zero (the default) means no min-delay enforcement + even when the last search was a millisecond ago. Confirms + backwards compat — existing users see no new wait.""" + assert compute_search_wait_seconds( + timestamps=[], + last_search_at=99.99, + now=100.0, + max_per_window=35, + window_seconds=220, + min_delay_seconds=0, + ) == 0.0 + + +# --------------------------------------------------------------------------- +# Sliding-window cap (legacy behavior preserved) +# --------------------------------------------------------------------------- + + +class TestSlidingWindowCap: + def test_window_full_waits_for_oldest_to_age_out(self): + """35 timestamps in window → wait until oldest ages out. + Same semantics as the pre-fix hardcoded behavior.""" + timestamps = [10.0 + i for i in range(35)] # 10..44 + # now = 50, window = 220, oldest = 10 → ages out at 230 → wait 180 + wait = compute_search_wait_seconds( + timestamps=timestamps, + last_search_at=44.0, + now=50.0, + max_per_window=35, + window_seconds=220, + min_delay_seconds=0, + ) + assert wait == pytest.approx(180.0, abs=1e-9) + + def test_window_full_but_oldest_already_aged_out_returns_zero(self): + """If now is past oldest+window, the negative is clamped to 0 + (the caller is expected to prune timestamps before passing — + this is just defense-in-depth).""" + wait = compute_search_wait_seconds( + timestamps=[10.0] * 35, + last_search_at=10.0, + now=400.0, + max_per_window=35, + window_seconds=220, + min_delay_seconds=0, + ) + assert wait == 0.0 + + def test_custom_max_per_window_honored(self): + """User dials max down to 10 (paranoia mode for ISP anti-abuse). + Cap kicks in at 10, not 35.""" + timestamps = [10.0 + i for i in range(10)] + wait = compute_search_wait_seconds( + timestamps=timestamps, + last_search_at=19.0, + now=20.0, + max_per_window=10, + window_seconds=60, + min_delay_seconds=0, + ) + # oldest = 10, ages out at 70, now = 20 → wait 50 + assert wait == pytest.approx(50.0, abs=1e-9) + + def test_max_per_window_zero_disables_window_cap(self): + """Defensive: max=0 means no cap (don't divide by zero, don't + block forever). Min-delay still applies if set.""" + wait = compute_search_wait_seconds( + timestamps=[10.0] * 100, + last_search_at=50.0, + now=51.0, + max_per_window=0, + window_seconds=220, + min_delay_seconds=0, + ) + assert wait == 0.0 + + +# --------------------------------------------------------------------------- +# Min-delay between searches (the new knob — Bell Canada fix) +# --------------------------------------------------------------------------- + + +class TestMinDelayBetweenSearches: + def test_recent_last_search_blocks_for_remaining_delay(self): + """User sets min_delay=5s. Last search 2s ago → wait 3s. + Smooths the burst pattern that trips Bell's anti-abuse even + when the sliding window isn't full.""" + wait = compute_search_wait_seconds( + timestamps=[100.0, 102.0], + last_search_at=102.0, + now=104.0, + max_per_window=35, + window_seconds=220, + min_delay_seconds=5, + ) + assert wait == pytest.approx(3.0, abs=1e-9) + + def test_min_delay_already_elapsed_returns_zero(self): + """Last search 10s ago, min-delay 5s → already cleared, no wait.""" + wait = compute_search_wait_seconds( + timestamps=[100.0], + last_search_at=100.0, + now=110.0, + max_per_window=35, + window_seconds=220, + min_delay_seconds=5, + ) + assert wait == 0.0 + + def test_min_delay_skipped_on_very_first_search(self): + """`last_search_at == 0` means there's never been a search. + Don't gate the very first one — that would force an arbitrary + startup delay for no reason.""" + wait = compute_search_wait_seconds( + timestamps=[], + last_search_at=0.0, + now=100.0, + max_per_window=35, + window_seconds=220, + min_delay_seconds=10, + ) + assert wait == 0.0 + + +# --------------------------------------------------------------------------- +# Both gates active — max wins +# --------------------------------------------------------------------------- + + +class TestMaxOfBothGates: + def test_returns_window_wait_when_window_wait_is_larger(self): + """Window says wait 100s, min-delay says wait 5s → return 100s.""" + timestamps = [0.0 + i for i in range(35)] # 0..34 + # now = 5, window = 220, oldest = 0 → ages out at 220 → wait 215 + # min_delay = 5, last = 4, now = 5 → wait 4 + wait = compute_search_wait_seconds( + timestamps=timestamps, + last_search_at=4.0, + now=5.0, + max_per_window=35, + window_seconds=220, + min_delay_seconds=5, + ) + assert wait == pytest.approx(215.0, abs=1e-9) + + def test_returns_min_delay_wait_when_min_delay_is_larger(self): + """Window not full → window wait = 0. Min-delay 30s, last 5s + ago → wait 25s. Min-delay drives it.""" + wait = compute_search_wait_seconds( + timestamps=[100.0, 105.0], + last_search_at=105.0, + now=110.0, + max_per_window=35, + window_seconds=220, + min_delay_seconds=30, + ) + assert wait == pytest.approx(25.0, abs=1e-9) + + def test_both_zero_returns_zero(self): + """Window not full + min-delay clear → zero. Sanity.""" + wait = compute_search_wait_seconds( + timestamps=[100.0], + last_search_at=50.0, + now=200.0, + max_per_window=35, + window_seconds=220, + min_delay_seconds=10, + ) + assert wait == 0.0 + + +# --------------------------------------------------------------------------- +# Defensive — input shape variations +# --------------------------------------------------------------------------- + + +class TestDefensive: + def test_negative_min_delay_treated_as_disabled(self): + """Defensive: a negative min-delay (somehow) shouldn't return + a negative wait or trigger weird behavior. Treat as disabled.""" + wait = compute_search_wait_seconds( + timestamps=[], + last_search_at=99.0, + now=100.0, + max_per_window=35, + window_seconds=220, + min_delay_seconds=-5, + ) + assert wait == 0.0 + + def test_returns_float(self): + """Caller passes to asyncio.sleep which wants a float. Pin shape.""" + wait = compute_search_wait_seconds( + timestamps=[], + last_search_at=0.0, + now=100.0, + max_per_window=35, + window_seconds=220, + min_delay_seconds=0, + ) + assert isinstance(wait, float) diff --git a/webui/index.html b/webui/index.html index 77fb01f4..64f3aa50 100644 --- a/webui/index.html +++ b/webui/index.html @@ -4329,6 +4329,16 @@ Extra time to wait for late results (5-60 seconds) +