#880: retry 429 mid-walk when paginating Tidal Favorite Tracks (don't truncate)
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.
This commit is contained in:
parent
afa07690f5
commit
2905fe0853
2 changed files with 75 additions and 0 deletions
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in a new issue