Merge PR #1041: preserve playlist folder when retrying failed downloads

Replaces the client-side retry (which rebuilt an /add payload from the public
download dict) with a server-side POST /retry that re-adds from the stored
DownloadInfo. The public dict deliberately excludes `entry`, so the UI could
never carry playlist context across a retry: retried playlist items lost
playlist_index and landed in the root directory instead of their playlist
folder. The completed queue now persists the compacted entry for status=error
records only, so the context also survives a restart; successful records still
drop it. Also adds track_number to _COMPACT_ENTRY_EXTRA_KEYS so the #1031 music
metadata survives a failure/retry cycle.

Merged with two review fixes (08dccd9): retry_entry is now carried through the
url/url_transparent recursion in __add_entry, and retry() re-applies the
ALLOW_YTDL_OPTIONS_OVERRIDES and configured-preset gates that /add enforces via
parse_download_options.

Co-authored-by: jahruz67 <jahruz67@users.noreply.github.com>
This commit is contained in:
Alex Shnitman
2026-07-27 21:14:21 +03:00
9 changed files with 317 additions and 34 deletions
+11
View File
@@ -893,6 +893,16 @@ async def cancel_add(request):
return web.Response(text=serializer.encode({'status': 'ok'}), content_type='application/json')
@routes.post(config.URL_PREFIX + 'retry')
async def retry(request):
post = await _read_json_request(request)
ids = _require_id_list(post)
if len(ids) != 1:
raise web.HTTPBadRequest(reason="'ids' must contain exactly one download id")
status = await dqueue.retry(ids[0])
return web.Response(text=serializer.encode(status), content_type='application/json')
@routes.post(config.URL_PREFIX + 'subscribe')
async def subscribe(request):
post = await _read_json_request(request)
@@ -1227,6 +1237,7 @@ async def add_cors(request):
app.router.add_route('OPTIONS', config.URL_PREFIX + 'add', add_cors)
app.router.add_route('OPTIONS', config.URL_PREFIX + 'cancel-add', add_cors)
app.router.add_route('OPTIONS', config.URL_PREFIX + 'retry', add_cors)
app.router.add_route('OPTIONS', config.URL_PREFIX + 'subscribe', add_cors)
app.router.add_route('OPTIONS', config.URL_PREFIX + 'subscriptions', add_cors)
app.router.add_route('OPTIONS', config.URL_PREFIX + 'subscriptions/update', add_cors)
+9
View File
@@ -20,6 +20,7 @@ def mock_dqueue(monkeypatch):
d = MagicMock()
d.initialize = AsyncMock(return_value=None)
d.add = AsyncMock(return_value={"status": "ok"})
d.retry = AsyncMock(return_value={"status": "ok"})
d.cancel = AsyncMock(return_value={"status": "ok"})
d.clear = AsyncMock(return_value={"status": "ok"})
d.start_pending = AsyncMock(return_value={"status": "ok"})
@@ -69,6 +70,14 @@ async def test_add_ok(mock_dqueue):
mock_dqueue.add.assert_awaited_once()
@pytest.mark.asyncio
async def test_retry_passes_failed_download_id(mock_dqueue):
req = _json_request({"ids": ["https://example.com/watch?v=1"]})
resp = await main.retry(req)
assert resp.status == 200
mock_dqueue.retry.assert_awaited_once_with("https://example.com/watch?v=1")
@pytest.mark.asyncio
async def test_add_passes_preset_and_overrides(mock_dqueue, monkeypatch):
monkeypatch.setattr(main.config, "YTDL_OPTIONS_PRESETS", {"Preset A": {"writesubtitles": True}})
+173
View File
@@ -302,6 +302,179 @@ async def test_add_entry_queues_single_video_without_reextracting(dq_env):
assert dq.pending.exists("https://example.com/watch?v=1")
@pytest.mark.asyncio
async def test_retry_restores_playlist_output_context(dq_env):
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"
failed_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="",
)
failed_info.status = "error"
dq.done.put(Download(None, None, None, None, "best", "any", {}, failed_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.output_template == "My Playlist/%(title)s.%(ext)s"
assert queued.info.entry["playlist_index"] == "01"
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()
+15 -3
View File
@@ -146,12 +146,12 @@ class PersistentQueueTests(unittest.TestCase):
self.assertNotIn("formats", record["entry"])
self.assertNotIn("description", record["entry"])
def test_completed_queue_does_not_persist_entry_or_transient_progress(self):
def test_completed_queue_persists_only_failed_retry_context(self):
with tempfile.TemporaryDirectory() as tmp:
path = os.path.join(tmp, "completed")
pq = PersistentQueue("completed", path)
info = _make_info("http://done.example")
info.status = "finished"
info.status = "error"
info.percent = 88
info.speed = 123
info.eta = 9
@@ -167,12 +167,24 @@ class PersistentQueueTests(unittest.TestCase):
payload = json.load(f)
record = payload["items"][0]["info"]
self.assertNotIn("entry", record)
self.assertEqual(
record["entry"],
{
"playlist_index": "01",
"playlist_title": "Playlist",
},
)
self.assertNotIn("percent", record)
self.assertNotIn("speed", record)
self.assertNotIn("eta", record)
self.assertEqual(record["filename"], "done.mp4")
info.status = "finished"
pq.put(_FakeDownload(info))
with open(path + ".json", encoding="utf-8") as f:
payload = json.load(f)
self.assertNotIn("entry", payload["items"][0]["info"])
def test_invalid_json_is_quarantined_and_legacy_is_imported(self):
with tempfile.TemporaryDirectory() as tmp:
path = os.path.join(tmp, "queue")
+61 -9
View File
@@ -510,7 +510,7 @@ def _short_title_for_failed_url(url: str) -> str:
return hostname or url
_COMPACT_ENTRY_EXTRA_KEYS = frozenset(("n_entries", "__last_playlist_index"))
_COMPACT_ENTRY_EXTRA_KEYS = frozenset(("n_entries", "__last_playlist_index", "track_number"))
def _compact_persisted_entry(entry: Any) -> Optional[dict[str, Any]]:
@@ -944,8 +944,12 @@ class PersistentQueue:
]
return sorted(items, key=lambda item: item[1].timestamp)
def _should_persist_entry(self) -> bool:
return self.identifier != "completed"
def _should_persist_entry(self, info: DownloadInfo | dict[str, Any]) -> bool:
# Failed downloads need their compact playlist/channel context so a
# retry after a server restart still resolves the original outtmpl.
# Successful completed entries continue to omit extractor metadata.
status = info.get("status") if isinstance(info, dict) else info.status
return self.identifier != "completed" or status == "error"
def _serialize_items(self):
return [
@@ -953,7 +957,7 @@ class PersistentQueue:
"key": key,
"info": _download_info_to_record(
download.info,
include_entry=self._should_persist_entry(),
include_entry=self._should_persist_entry(download.info),
),
}
for key, download in self.dict.items()
@@ -972,7 +976,7 @@ class PersistentQueue:
"key": item["key"],
"info": _download_info_to_record(
_download_info_from_record(item["info"]),
include_entry=self._should_persist_entry(),
include_entry=self._should_persist_entry(item["info"]),
),
}
for item in items
@@ -993,7 +997,7 @@ class PersistentQueue:
"key": key,
"info": _download_info_to_record(
value,
include_entry=self._should_persist_entry(),
include_entry=self._should_persist_entry(value),
),
}
for key, value in sorted(legacy_items, key=lambda item: item[1].timestamp)
@@ -1394,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."}
@@ -1412,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,
@@ -1432,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):
@@ -1559,6 +1569,7 @@ class DownloadQueue:
ytdl_options_overrides,
clip_start,
clip_end,
entry=None,
):
"""Surface a URL that failed before a DownloadInfo could be created (unsupported
URL, SSRF-rejected, extraction error) as a failed entry in the done list, so the
@@ -1575,7 +1586,7 @@ class DownloadQueue:
folder=folder,
custom_name_prefix=custom_name_prefix,
error=msg,
entry=None,
entry=entry,
playlist_item_limit=playlist_item_limit,
split_by_chapters=split_by_chapters,
chapter_template=chapter_template,
@@ -1613,6 +1624,7 @@ class DownloadQueue:
clip_end=None,
already=None,
_add_gen=None,
retry_entry=None,
):
if ytdl_options_presets is None:
ytdl_options_presets = []
@@ -1641,7 +1653,7 @@ class DownloadQueue:
url, url_error, download_type, codec, format, quality, folder,
custom_name_prefix, playlist_item_limit, split_by_chapters, chapter_template,
subtitle_language, subtitle_mode, ytdl_options_presets, ytdl_options_overrides,
clip_start, clip_end,
clip_start, clip_end, retry_entry,
)
return {'status': 'error', 'msg': url_error}
try:
@@ -1655,9 +1667,12 @@ class DownloadQueue:
url, msg, download_type, codec, format, quality, folder,
custom_name_prefix, playlist_item_limit, split_by_chapters, chapter_template,
subtitle_language, subtitle_mode, ytdl_options_presets, ytdl_options_overrides,
clip_start, clip_end,
clip_start, clip_end, retry_entry,
)
return {'status': 'error', 'msg': msg}
retry_context = _compact_persisted_entry(retry_entry)
if isinstance(entry, dict) and retry_context is not None:
entry = {**entry, **copy.deepcopy(retry_context)}
return await self.__add_entry(
entry,
download_type,
@@ -1678,6 +1693,43 @@ class DownloadQueue:
clip_end,
already,
_add_gen,
retry_entry,
)
async def retry(self, id):
if not self.done.exists(id):
return {'status': 'error', 'msg': 'Failed download no longer exists.'}
info = self.done.get(id).info
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,
info.codec,
info.format,
info.quality,
info.folder,
info.custom_name_prefix,
info.playlist_item_limit,
True,
info.split_by_chapters,
info.chapter_template,
info.subtitle_language,
info.subtitle_mode,
presets,
overrides,
info.clip_start,
info.clip_end,
retry_entry=info.entry,
)
async def add_entry(
+33
View File
@@ -19,6 +19,7 @@ class DownloadsServiceStub {
customDirsChanged = new Subject<Record<string, string[]>>();
ytdlOptionsChanged = new Subject<Record<string, unknown>>();
updated = new Subject<void>();
retryCalls: string[] = [];
getCookieStatus() {
return of({ status: 'ok', has_cookies: false });
@@ -32,6 +33,11 @@ class DownloadsServiceStub {
return of({ status: 'ok' as const });
}
retry(id: string) {
this.retryCalls.push(id);
return of({ status: 'ok' as const });
}
cancelAdd() {
return of({ status: 'ok' as const });
}
@@ -269,6 +275,33 @@ describe('App', () => {
expect(payload.clipEnd).toBe('1:20');
});
it('retries a failed download by its server-side queue id', () => {
const fixture = TestBed.createComponent(App);
const app = fixture.componentInstance;
const download = {
id: 'vid1',
title: 'Test Video',
url: 'https://example.com/v',
download_type: 'video',
quality: 'best',
format: 'any',
folder: '',
custom_name_prefix: '',
playlist_item_limit: 0,
status: 'error',
msg: 'temporary failure',
percent: 0,
speed: 0,
eta: 0,
filename: '',
checked: false,
};
app.retryDownload(download.url, download);
expect(downloads.retryCalls).toEqual([download.url]);
});
it('blocks subscribe with invalid title regex', () => {
const toasts = TestBed.inject(ToastService);
const errorSpy = vi.spyOn(toasts, 'error').mockImplementation(() => undefined);
+1 -22
View File
@@ -1146,30 +1146,9 @@ export class App implements AfterViewInit, OnInit, OnDestroy {
}
retryDownload(key: string, download: Download) {
const payload = this.buildAddPayload({
url: download.url,
downloadType: download.download_type,
codec: download.codec,
quality: download.quality,
format: download.format,
folder: download.folder,
customNamePrefix: download.custom_name_prefix,
playlistItemLimit: download.playlist_item_limit,
autoStart: true,
splitByChapters: download.split_by_chapters,
chapterTemplate: download.chapter_template,
subtitleLanguage: download.subtitle_language,
subtitleMode: download.subtitle_mode,
ytdlOptionsPresets: download.ytdl_options_presets?.length
? [...download.ytdl_options_presets]
: [],
ytdlOptionsOverrides: download.ytdl_options_overrides ? JSON.stringify(download.ytdl_options_overrides) : '',
clipStart: download.clip_start != null ? String(download.clip_start) : '',
clipEnd: download.clip_end != null ? String(download.clip_end) : '',
});
// Only remove the done-list record once the retry is confirmed queued —
// deleting it eagerly would silently lose history if the re-add fails.
this.downloads.add(payload)
this.downloads.retry(key)
.pipe(takeUntilDestroyed(this.destroyRef))
.subscribe((status: Status) => {
if (status.status === 'error') {
@@ -117,6 +117,14 @@ describe('DownloadsService', () => {
req.flush({ presets: ['Preset A'] });
});
it('retry() posts the failed download id', () => {
service.retry('https://example.com/v').subscribe();
const req = httpMock.expectOne('retry');
expect(req.request.method).toBe('POST');
expect(req.request.body).toEqual({ ids: ['https://example.com/v'] });
req.flush({ status: 'ok' });
});
it('cancelAdd posts to cancel-add', () => {
service.cancelAdd().subscribe();
const req = httpMock.expectOne('cancel-add');
+6
View File
@@ -169,6 +169,12 @@ export class DownloadsService {
);
}
public retry(id: string) {
return this.http.post<Status>('retry', { ids: [id] }).pipe(
catchError(this.handleHTTPError)
);
}
public startById(ids: string[]) {
return this.http.post<Status>('start', {ids: ids}).pipe(
catchError(this.handleHTTPError)