mirror of
https://github.com/alexta69/metube.git
synced 2026-09-21 13:35:01 +00:00
Merge PR #1038: surface add-time failures as failed done entries
Adds __record_add_failure so a URL that fails before a real download starts
(unsupported/unextractable URL, SSRF-rejected, extraction error) appears in the
Completed list as a red-cross entry with retry and error-detail, instead of only
a transient toast and a server log line. Keyed by info.url like any errored
download. Includes _short_title_for_failed_url for a readable hostname title.
The frontend hunk removing the 'Click for details' hint from every error row was
dropped from this merge; that affordance (added in fd3aaea, #143) is kept.
Co-authored-by: streamer1122 <streamer1122@users.noreply.github.com>
This commit is contained in:
@@ -115,6 +115,54 @@ async def test_add_single_video_goes_to_pending_when_auto_start_false(dq_env):
|
|||||||
assert dq.pending.exists("https://example.com/watch?v=1")
|
assert dq.pending.exists("https://example.com/watch?v=1")
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_add_unsupported_url_recorded_as_failed_entry(dq_env):
|
||||||
|
"""An unsupported/unextractable URL must show up as a red-cross entry in the
|
||||||
|
done list, not just a transient toast and a server log line."""
|
||||||
|
import ytdl
|
||||||
|
|
||||||
|
notifier = AsyncMock()
|
||||||
|
url = "https://example.com/not-a-video"
|
||||||
|
|
||||||
|
def boom(self, url, ytdl_options_presets=None, ytdl_options_overrides=None):
|
||||||
|
raise ytdl.yt_dlp.utils.YoutubeDLError(f'Unsupported URL: {url}')
|
||||||
|
|
||||||
|
dq = DownloadQueue(dq_env, notifier)
|
||||||
|
with patch.object(DownloadQueue, "_DownloadQueue__extract_info", boom):
|
||||||
|
result = await dq.add(
|
||||||
|
url, "video", "auto", "any", "best", "", "", 0, auto_start=True,
|
||||||
|
)
|
||||||
|
assert result["status"] == "error"
|
||||||
|
assert dq.done.exists(url)
|
||||||
|
failed = dq.done.get(url)
|
||||||
|
assert failed.info.status == "error"
|
||||||
|
assert failed.info.error == result["msg"]
|
||||||
|
assert failed.info.url == url
|
||||||
|
# The full URL stays in .url/.error for the detail panel; the display
|
||||||
|
# title is shortened to the hostname so the Completed row stays readable.
|
||||||
|
assert failed.info.title == "example.com"
|
||||||
|
notifier.completed.assert_awaited()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_add_ssrf_rejected_url_recorded_as_failed_entry(dq_env):
|
||||||
|
"""A URL rejected by the SSRF guard (before yt-dlp ever runs) must also
|
||||||
|
surface as a failed entry, not just an error status returned to the caller."""
|
||||||
|
notifier = AsyncMock()
|
||||||
|
url = "file:///etc/passwd"
|
||||||
|
|
||||||
|
dq = DownloadQueue(dq_env, notifier)
|
||||||
|
result = await dq.add(
|
||||||
|
url, "video", "auto", "any", "best", "", "", 0, auto_start=True,
|
||||||
|
)
|
||||||
|
assert result["status"] == "error"
|
||||||
|
assert dq.done.exists(url)
|
||||||
|
failed = dq.done.get(url)
|
||||||
|
assert failed.info.status == "error"
|
||||||
|
assert failed.info.error == result["msg"]
|
||||||
|
notifier.completed.assert_awaited()
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_cancel_removes_from_pending(dq_env):
|
async def test_cancel_removes_from_pending(dq_env):
|
||||||
notifier = AsyncMock()
|
notifier = AsyncMock()
|
||||||
|
|||||||
@@ -80,6 +80,7 @@ from ytdl import (
|
|||||||
_resolve_outtmpl_fields,
|
_resolve_outtmpl_fields,
|
||||||
_sanitize_entry_for_pickle,
|
_sanitize_entry_for_pickle,
|
||||||
_sanitize_path_component,
|
_sanitize_path_component,
|
||||||
|
_short_title_for_failed_url,
|
||||||
)
|
)
|
||||||
|
|
||||||
# Detect whether the real yt-dlp is loaded (as opposed to the minimal fake
|
# Detect whether the real yt-dlp is loaded (as opposed to the minimal fake
|
||||||
@@ -808,5 +809,23 @@ class CompactPersistedEntryTests(unittest.TestCase):
|
|||||||
self.assertIsNone(_compact_persisted_entry({"id": "x", "title": "y"}))
|
self.assertIsNone(_compact_persisted_entry({"id": "x", "title": "y"}))
|
||||||
|
|
||||||
|
|
||||||
|
class ShortTitleForFailedUrlTests(unittest.TestCase):
|
||||||
|
def test_uses_hostname_for_a_normal_url(self):
|
||||||
|
self.assertEqual(
|
||||||
|
_short_title_for_failed_url("https://example.com/watch?v=1"),
|
||||||
|
"example.com",
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_falls_back_to_raw_value_when_there_is_no_hostname(self):
|
||||||
|
# file:// URIs and bare search terms/video IDs have no netloc to extract.
|
||||||
|
self.assertEqual(_short_title_for_failed_url("file:///etc/passwd"), "file:///etc/passwd")
|
||||||
|
self.assertEqual(_short_title_for_failed_url("ytsearch:some query"), "ytsearch:some query")
|
||||||
|
|
||||||
|
def test_falls_back_to_raw_value_on_unparseable_input(self):
|
||||||
|
# A malformed IPv6-looking host raises ValueError in urlsplit().hostname.
|
||||||
|
malformed = "https://[::1/watch"
|
||||||
|
self.assertEqual(_short_title_for_failed_url(malformed), malformed)
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
|||||||
+78
-1
@@ -28,6 +28,7 @@ from datetime import datetime
|
|||||||
from state_store import AtomicJsonStore, from_json_compatible, read_legacy_shelf, to_json_compatible
|
from state_store import AtomicJsonStore, from_json_compatible, read_legacy_shelf, to_json_compatible
|
||||||
from subscriptions import _entry_id
|
from subscriptions import _entry_id
|
||||||
from url_guard import validate_url, install_socket_guard
|
from url_guard import validate_url, install_socket_guard
|
||||||
|
from urllib.parse import urlsplit
|
||||||
|
|
||||||
log = logging.getLogger('ytdl')
|
log = logging.getLogger('ytdl')
|
||||||
|
|
||||||
@@ -498,6 +499,17 @@ _PERSISTED_DOWNLOAD_FIELDS = (
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _short_title_for_failed_url(url: str) -> str:
|
||||||
|
"""A concise display title for a URL that failed before yt-dlp could extract a
|
||||||
|
real title (unsupported URL, SSRF-rejected, extraction error). The full URL
|
||||||
|
remains available in DownloadInfo.url and the error-detail panel."""
|
||||||
|
try:
|
||||||
|
hostname = urlsplit(url).hostname
|
||||||
|
except ValueError:
|
||||||
|
hostname = None
|
||||||
|
return hostname or url
|
||||||
|
|
||||||
|
|
||||||
_COMPACT_ENTRY_EXTRA_KEYS = frozenset(("n_entries", "__last_playlist_index"))
|
_COMPACT_ENTRY_EXTRA_KEYS = frozenset(("n_entries", "__last_playlist_index"))
|
||||||
|
|
||||||
|
|
||||||
@@ -1528,6 +1540,58 @@ class DownloadQueue:
|
|||||||
return {'status': 'ok'}
|
return {'status': 'ok'}
|
||||||
return {'status': 'error', 'msg': f'Unsupported resource "{etype}"'}
|
return {'status': 'error', 'msg': f'Unsupported resource "{etype}"'}
|
||||||
|
|
||||||
|
async def __record_add_failure(
|
||||||
|
self,
|
||||||
|
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,
|
||||||
|
):
|
||||||
|
"""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
|
||||||
|
frontend shows it with the same red-cross/retry/error-detail treatment as a
|
||||||
|
download that failed mid-stream, instead of only a toast and a server log line."""
|
||||||
|
info = DownloadInfo(
|
||||||
|
id=url,
|
||||||
|
title=_short_title_for_failed_url(url),
|
||||||
|
url=url,
|
||||||
|
quality=quality,
|
||||||
|
download_type=download_type,
|
||||||
|
codec=codec,
|
||||||
|
format=format,
|
||||||
|
folder=folder,
|
||||||
|
custom_name_prefix=custom_name_prefix,
|
||||||
|
error=msg,
|
||||||
|
entry=None,
|
||||||
|
playlist_item_limit=playlist_item_limit,
|
||||||
|
split_by_chapters=split_by_chapters,
|
||||||
|
chapter_template=chapter_template,
|
||||||
|
subtitle_language=subtitle_language,
|
||||||
|
subtitle_mode=subtitle_mode,
|
||||||
|
ytdl_options_presets=ytdl_options_presets,
|
||||||
|
ytdl_options_overrides=ytdl_options_overrides,
|
||||||
|
clip_start=clip_start,
|
||||||
|
clip_end=clip_end,
|
||||||
|
)
|
||||||
|
info.status = 'error'
|
||||||
|
info.msg = msg
|
||||||
|
download = Download(None, None, None, None, quality, format, {}, info)
|
||||||
|
self.done.put(download)
|
||||||
|
await self.notifier.completed(info)
|
||||||
|
|
||||||
async def add(
|
async def add(
|
||||||
self,
|
self,
|
||||||
url,
|
url,
|
||||||
@@ -1573,6 +1637,12 @@ class DownloadQueue:
|
|||||||
None, partial(validate_url, url, allow_private=self.config.ALLOW_PRIVATE_ADDRESSES))
|
None, partial(validate_url, url, allow_private=self.config.ALLOW_PRIVATE_ADDRESSES))
|
||||||
if url_error is not None:
|
if url_error is not None:
|
||||||
log.warning('Rejected URL "%s": %s', url, url_error)
|
log.warning('Rejected URL "%s": %s', url, url_error)
|
||||||
|
await self.__record_add_failure(
|
||||||
|
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,
|
||||||
|
)
|
||||||
return {'status': 'error', 'msg': url_error}
|
return {'status': 'error', 'msg': url_error}
|
||||||
try:
|
try:
|
||||||
entry = await asyncio.get_running_loop().run_in_executor(
|
entry = await asyncio.get_running_loop().run_in_executor(
|
||||||
@@ -1580,7 +1650,14 @@ class DownloadQueue:
|
|||||||
partial(self.__extract_info, url, ytdl_options_presets, ytdl_options_overrides),
|
partial(self.__extract_info, url, ytdl_options_presets, ytdl_options_overrides),
|
||||||
)
|
)
|
||||||
except yt_dlp.utils.YoutubeDLError as exc:
|
except yt_dlp.utils.YoutubeDLError as exc:
|
||||||
return {'status': 'error', 'msg': str(exc)}
|
msg = str(exc)
|
||||||
|
await self.__record_add_failure(
|
||||||
|
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,
|
||||||
|
)
|
||||||
|
return {'status': 'error', 'msg': msg}
|
||||||
return await self.__add_entry(
|
return await self.__add_entry(
|
||||||
entry,
|
entry,
|
||||||
download_type,
|
download_type,
|
||||||
|
|||||||
Reference in New Issue
Block a user