fix: carry retry context through url indirection and re-gate retry options

Two review fixes on top of the retry endpoint:

- __add_entry dropped retry_entry when extraction returned an unprocessed
  url/url_transparent result and it recursed back into add(). Since
  __extract_info runs with extract_flat=True, that path is live, and a retried
  playlist item taking it fell back to OUTPUT_TEMPLATE and landed in the root
  directory instead of its playlist folder. The playlist child loop keeps
  passing retry_entry=None on purpose: those entries get fresh playlist context
  stamped on them from the current extraction.

- retry() called dqueue.add() directly, so it bypassed the
  parse_download_options gates that /add applies. Stored ytdl_options_overrides
  were re-applied even after ALLOW_YTDL_OPTIONS_OVERRIDES was turned off, and
  preset names removed from the configuration were still passed through. Both
  are re-checked against the current configuration at retry time.
This commit is contained in:
Alex Shnitman
2026-07-27 21:13:59 +03:00
parent 8a29f3a084
commit 08dccd98fb
2 changed files with 140 additions and 2 deletions
+124
View File
@@ -351,6 +351,130 @@ async def test_retry_restores_playlist_output_context(dq_env):
assert queued.info.entry["playlist_title"] == "My Playlist"
def _failed_playlist_item(url, **overrides):
"""A done-list entry for a playlist item that failed mid-download."""
info = DownloadInfo(
id="vid1",
title="Test Video",
url=url,
quality="best",
download_type="video",
codec="auto",
format="any",
folder="",
custom_name_prefix="",
error="temporary failure",
entry={
"playlist_index": "01",
"playlist_title": "My Playlist",
"playlist_count": 10,
},
playlist_item_limit=0,
split_by_chapters=False,
chapter_template="",
**overrides,
)
info.status = "error"
return info
@pytest.mark.asyncio
async def test_retry_keeps_playlist_context_through_url_indirection(dq_env):
# extract_flat=True makes yt-dlp hand back url/url_transparent results
# unprocessed, so __add_entry recurses into add() a second time. The retry
# context has to survive that hop or the item lands in the root directory.
notifier = AsyncMock()
dq_env.OUTPUT_TEMPLATE_PLAYLIST = "%(playlist_title)s/%(title)s.%(ext)s"
dq = DownloadQueue(dq_env, notifier)
url = "https://example.com/watch?v=1"
resolved = "https://example.com/resolved?v=1"
dq.done.put(Download(None, None, None, None, "best", "any", {}, _failed_playlist_item(url)))
def fake_extract(self, extracted_url, ytdl_options_presets=None, ytdl_options_overrides=None):
if extracted_url == url:
return {"_type": "url", "url": resolved, "id": "vid1"}
return {
"_type": "video",
"id": "vid1",
"title": "Test Video",
"url": extracted_url,
"webpage_url": extracted_url,
}
with patch.object(DownloadQueue, "_DownloadQueue__extract_info", fake_extract), \
patch.object(DownloadQueue, "_DownloadQueue__start_download", new=AsyncMock()):
result = await dq.retry(url)
assert result["status"] == "ok"
queued = dq.queue.get(resolved)
assert queued.output_template == "My Playlist/%(title)s.%(ext)s"
assert queued.info.entry["playlist_title"] == "My Playlist"
@pytest.mark.asyncio
async def test_retry_reapplies_current_options_gates(dq_env):
# The stored options passed parse_download_options when first submitted, but
# the configuration can have changed since; retry must not resurrect
# overrides or presets the current configuration no longer allows.
notifier = AsyncMock()
dq_env.ALLOW_YTDL_OPTIONS_OVERRIDES = False
dq_env.YTDL_OPTIONS_PRESETS = {"Still There": {"writesubtitles": True}}
dq = DownloadQueue(dq_env, notifier)
url = "https://example.com/watch?v=1"
info = _failed_playlist_item(
url,
ytdl_options_presets=["Still There", "Removed Preset"],
ytdl_options_overrides={"paths": {"home": "/etc"}},
)
dq.done.put(Download(None, None, None, None, "best", "any", {}, info))
def fake_extract(self, extracted_url, ytdl_options_presets=None, ytdl_options_overrides=None):
return {
"_type": "video",
"id": "vid1",
"title": "Test Video",
"url": extracted_url,
"webpage_url": extracted_url,
}
with patch.object(DownloadQueue, "_DownloadQueue__extract_info", fake_extract), \
patch.object(DownloadQueue, "_DownloadQueue__start_download", new=AsyncMock()):
result = await dq.retry(url)
assert result["status"] == "ok"
queued = dq.queue.get(url)
assert queued.info.ytdl_options_overrides == {}
assert queued.info.ytdl_options_presets == ["Still There"]
assert queued.ytdl_opts.get("paths", {}).get("home") != "/etc"
@pytest.mark.asyncio
async def test_retry_keeps_overrides_while_still_allowed(dq_env):
notifier = AsyncMock()
dq_env.ALLOW_YTDL_OPTIONS_OVERRIDES = True
dq_env.YTDL_OPTIONS_PRESETS = {}
dq = DownloadQueue(dq_env, notifier)
url = "https://example.com/watch?v=1"
info = _failed_playlist_item(url, ytdl_options_overrides={"writesubtitles": True})
dq.done.put(Download(None, None, None, None, "best", "any", {}, info))
def fake_extract(self, extracted_url, ytdl_options_presets=None, ytdl_options_overrides=None):
return {
"_type": "video",
"id": "vid1",
"title": "Test Video",
"url": extracted_url,
"webpage_url": extracted_url,
}
with patch.object(DownloadQueue, "_DownloadQueue__extract_info", fake_extract), \
patch.object(DownloadQueue, "_DownloadQueue__start_download", new=AsyncMock()):
result = await dq.retry(url)
assert result["status"] == "ok"
assert dq.queue.get(url).info.ytdl_options_overrides == {"writesubtitles": True}
@pytest.mark.asyncio
async def test_add_entry_duplicate_while_pending_is_skipped_not_clobbered(dq_env):
notifier = AsyncMock()
+16 -2
View File
@@ -1398,6 +1398,7 @@ class DownloadQueue:
clip_end,
already,
_add_gen=None,
retry_entry=None,
):
if not entry:
return {'status': 'error', 'msg': "Invalid/empty data was given."}
@@ -1416,6 +1417,10 @@ class DownloadQueue:
if etype.startswith('url'):
log.debug('Processing as a url')
# retry_entry must ride along: extraction can hand back an
# unprocessed url/url_transparent result, and dropping the retry
# context here would send the retried item back to the root
# directory instead of its original playlist folder.
return await self.add(
entry['url'],
download_type,
@@ -1436,6 +1441,7 @@ class DownloadQueue:
clip_end,
already,
_add_gen,
retry_entry,
)
elif etype == 'playlist' or etype == 'channel':
if etype == 'playlist' and self.__is_channel_extraction(entry):
@@ -1687,6 +1693,7 @@ class DownloadQueue:
clip_end,
already,
_add_gen,
retry_entry,
)
async def retry(self, id):
@@ -1697,6 +1704,13 @@ class DownloadQueue:
if info.status != 'error':
return {'status': 'error', 'msg': 'Only failed downloads can be retried.'}
# The stored options were validated by parse_download_options when the
# download was first submitted, but the configuration can have changed
# since. Re-apply the same gates here so a retry can't resurrect
# overrides or presets the current configuration no longer allows.
overrides = info.ytdl_options_overrides if self.config.ALLOW_YTDL_OPTIONS_OVERRIDES else {}
presets = [p for p in info.ytdl_options_presets if p in self.config.YTDL_OPTIONS_PRESETS]
return await self.add(
info.url,
info.download_type,
@@ -1711,8 +1725,8 @@ class DownloadQueue:
info.chapter_template,
info.subtitle_language,
info.subtitle_mode,
info.ytdl_options_presets,
info.ytdl_options_overrides,
presets,
overrides,
info.clip_start,
info.clip_end,
retry_entry=info.entry,