From 2905fe0853601534d0da84abd949e96c46f87f5f Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 15 Jun 2026 19:56:53 -0700 Subject: [PATCH] #880: retry 429 mid-walk when paginating Tidal Favorite Tracks (don't truncate) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Favorites collection walker (_iter_collection_resource_ids) broke on ANY non-200 — including a transient 429. So a rate-limit mid-pagination silently truncated the collection: the log shows `status=429` then `Retrieved 98/100`, and the mirror saved 98 of a 524-track favorites list. The auto-sync cycle only "worked" because it dodged the 429. The regular-playlist paginator already retries 429; the collection walker didn't. Fix: retry the same cursor page with backoff (5/10/15/20s, 4 attempts) on 429, mirroring the playlist paginator; 401/403 still bail (+ reconnect flag), other non-200 still break. Regression tests: 429 mid-walk completes the full chain; exhausted retries return partial without hanging; 429 doesn't set reconnect. --- core/tidal_client.py | 26 ++++++++++++++ tests/test_tidal_collection_tracks.py | 49 +++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/core/tidal_client.py b/core/tidal_client.py index a18ab04f..4b542d3d 100644 --- a/core/tidal_client.py +++ b/core/tidal_client.py @@ -1655,6 +1655,8 @@ class TidalClient: ids: List[str] = [] next_path: Optional[str] = None + consecutive_429 = 0 + MAX_PAGE_RETRIES = 4 while True: if next_path: @@ -1687,6 +1689,28 @@ class TidalClient: logger.warning(f"Tidal collection page request failed: {e}") break + # Rate limited mid-walk → retry the SAME cursor page with backoff + # rather than silently truncating the collection. Without this a 429 + # on page ~5 capped a 513-track favorites list at ~98 (issue #880); + # the regular-playlist paginator already retries 429 the same way. + if resp.status_code == 429: + consecutive_429 += 1 + if consecutive_429 <= MAX_PAGE_RETRIES: + backoff = 5.0 * consecutive_429 # 5s, 10s, 15s, 20s + logger.warning( + f"Tidal collection {expected_type} rate limited (429) — " + f"retry {consecutive_429}/{MAX_PAGE_RETRIES} in {backoff}s " + f"({len(ids)} fetched so far)" + ) + time.sleep(backoff) + continue # next_path/cursor unchanged → re-request same page + logger.error( + f"Tidal collection {expected_type} still rate limited after " + f"{MAX_PAGE_RETRIES} retries — returning {len(ids)} IDs " + f"(PARTIAL: the collection may be larger)" + ) + break + if resp.status_code != 200: # 401/403 = scope/permission issue. Token predates the # `collection.read` scope expansion or the user revoked @@ -1704,6 +1728,8 @@ class TidalClient: ) break + consecutive_429 = 0 # a good page → reset the retry budget + try: data = resp.json() except ValueError as e: diff --git a/tests/test_tidal_collection_tracks.py b/tests/test_tidal_collection_tracks.py index b821a93c..88695e99 100644 --- a/tests/test_tidal_collection_tracks.py +++ b/tests/test_tidal_collection_tracks.py @@ -252,6 +252,55 @@ class TestIterCollectionTrackIds: assert ids == [] + def test_429_mid_walk_retries_and_completes(self): + """Regression for #880: a 429 mid-pagination must RETRY the same + cursor page (with backoff), not truncate the collection. Before the + fix a transient 429 on page ~5 capped a 513-track favorites list at + ~98 (the log showed `status=429` then `Retrieved 98/100`).""" + client = _make_authed_client() + # page1 (200) → 429 (transient) → page2 (200, end of chain) + responses = iter([ + _FakeResp(200, _PAGE_ONE), + _FakeResp(429, text=""), # rate limited — retry, don't truncate + _FakeResp(200, _PAGE_TWO), + ]) + with patch.object(client, '_ensure_valid_token', return_value=True), \ + patch('core.tidal_client.requests.get', side_effect=lambda *a, **kw: next(responses)), \ + patch('core.tidal_client.time.sleep'): + ids = client._iter_collection_track_ids() + + assert ids == ['1001', '1002', '1003', '1004', '1005'] # full chain, nothing dropped + + def test_429_does_not_set_reconnect_flag(self): + """A rate-limit is transient, NOT a scope problem — must not tell the + user to reconnect.""" + client = _make_authed_client() + responses = iter([_FakeResp(200, _PAGE_ONE), _FakeResp(429, text=""), _FakeResp(200, _PAGE_TWO)]) + with patch.object(client, '_ensure_valid_token', return_value=True), \ + patch('core.tidal_client.requests.get', side_effect=lambda *a, **kw: next(responses)), \ + patch('core.tidal_client.time.sleep'): + client._iter_collection_track_ids() + + assert client.collection_needs_reconnect() is False + + def test_429_exhausts_retries_returns_partial(self): + """If the 429s never clear, give up after the retry budget and return + what we have (PARTIAL) rather than looping forever.""" + client = _make_authed_client() + + def gen(): + yield _FakeResp(200, _PAGE_ONE) + while True: + yield _FakeResp(429, text="") + + responses = gen() + with patch.object(client, '_ensure_valid_token', return_value=True), \ + patch('core.tidal_client.requests.get', side_effect=lambda *a, **kw: next(responses)), \ + patch('core.tidal_client.time.sleep'): + ids = client._iter_collection_track_ids() + + assert ids == ['1001', '1002', '1003'] # page 1 survives; no hang + # --------------------------------------------------------------------------- # get_collection_tracks_count