mirror of
https://github.com/alexta69/metube.git
synced 2026-09-21 13:35:01 +00:00
feat: add retry functionality for failed downloads
This commit is contained in:
+11
@@ -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)
|
||||
|
||||
@@ -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}})
|
||||
|
||||
@@ -302,6 +302,55 @@ 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"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_add_entry_duplicate_while_pending_is_skipped_not_clobbered(dq_env):
|
||||
notifier = AsyncMock()
|
||||
|
||||
@@ -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")
|
||||
|
||||
+46
-8
@@ -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)
|
||||
@@ -1559,6 +1563,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 +1580,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 +1618,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 +1647,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 +1661,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,
|
||||
@@ -1680,6 +1689,35 @@ class DownloadQueue:
|
||||
_add_gen,
|
||||
)
|
||||
|
||||
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.'}
|
||||
|
||||
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,
|
||||
info.ytdl_options_presets,
|
||||
info.ytdl_options_overrides,
|
||||
info.clip_start,
|
||||
info.clip_end,
|
||||
retry_entry=info.entry,
|
||||
)
|
||||
|
||||
async def add_entry(
|
||||
self,
|
||||
entry,
|
||||
|
||||
@@ -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
@@ -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');
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user