Compare commits

...

12 Commits

Author SHA1 Message Date
Alex Shnitman ac46fff6d9 Merge PR #1058: DEFAULT_FOLDER pre-selects a download folder 2026-08-17 09:36:30 +02:00
Alex Shnitman 05c21326b3 Merge PR #1059: show the queued format in the Downloading table 2026-08-17 09:36:24 +02:00
Alex Shnitman 99da62dcbb Merge PR #1057: shift-click to select a range of rows 2026-08-17 09:36:19 +02:00
Alex Shnitman d0ad36baad Merge PR #1060: keep generated filenames within the filesystem limit 2026-08-17 09:36:01 +02:00
Alex Shnitman d2095caea2 Merge PR #1056: surface yt-dlp warning context on failed downloads 2026-08-17 09:35:56 +02:00
Alex Shnitman e15aff3339 fix: detect channels addressed without a tab (closes #1024)
__is_channel_extraction keyed on id == channel_id. That holds for a channel
tab - /channel/UC..., and the videos, streams, shorts and playlists tabs of a
/@handle URL - but not for a channel addressed on its own: yt-dlp reports the
id in the form the channel was asked for, so a bare handle URL yields '@handle'
and a legacy /c/ URL yields the vanity name.

Neither matched, so both fell through to OUTPUT_TEMPLATE_PLAYLIST and the
folder came out as the feed's title. That is why e2c7778 fixed the reporter's
tab URLs while a bare channel URL - what you get copying the address bar - went
on ignoring OUTPUT_TEMPLATE.

Both forms match uploader_id, which is the handle either way, so compare
against that as well, without case: a legacy vanity name and the handle it
became need not agree on it. A real playlist carries its owner's channel_id and
uploader_id but keeps an id of its own, so it still reads as a playlist; no
playlist id can collide with a handle, since those are 'PL...', 'OLAK...' and
the like.

Verified against the live extractor for all six channel URL forms and a real
playlist.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-17 09:31:56 +02:00
Cursor Agent fccd207799 fix: carry yt-dlp warning context on the DownloadError path (closes #1047)
MeTube never sets ignoreerrors, so yt-dlp's trouble() raises DownloadError
instead of returning nonzero for failures like "No video formats found!".
That is the path issue #1047 reports, and it dropped the warnings that
explain the failure because only the nonzero-return branch attached them.

The exception branch now reports the retained warnings with the exception
text as the final line, so the actual error stays prominent under the
context. yt-dlp labels errors "ERROR:" but hands warnings to the logger
unlabelled, so a last warning that repeats the error text is skipped.

Retention is bounded to the last five distinct warnings: fragmented and
live downloads emit a warning per fragment, and the joined message is
persisted with the completed queue and broadcast to every client.

Co-authored-by: Matt Van Horn <mvanhorn@users.noreply.github.com>
2026-08-16 22:13:02 +00:00
tjelite1986 6461924bf8 fix: keep generated filenames within the filesystem limit (closes #1034)
Sites that put a description in the title produce output names past what
the filesystem accepts, and the download fails outright with
"[Errno 36] File name too long". The name is now shortened to fit inside
prepare_filename, which every output path already passes through, so the
main file, its chapter files, thumbnails and subtitles stay consistent.

The limit is read from the filesystem (PC_NAME_MAX, falling back to 255)
and counted in bytes, not characters: a title of accented or CJK
characters reaches it in a third of the characters. The extension is
kept, a cut landing inside a multi-byte character does not leave a broken
sequence, and a reserve is held back for the suffixes yt-dlp appends
afterwards -- '.part', '.ytdl', '.f<format_id>', '-Frag<n>' -- since it
is those that pushed the reported name over the limit.

Names that already fit are untouched, so nothing that downloads today
changes name.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-16 14:19:36 +02:00
tjelite1986 c68fcaddd1 feat: show the queued format in the Downloading table (closes #551)
With the format picked per download rather than left on one setting, the
Downloading list gave no way to tell which item was queued as what until
it landed in Completed. The new column labels the format exactly as the
form does, reusing the same option lists, and falls back to the raw value
uppercased for a record queued before an option existed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-16 12:56:29 +02:00
tjelite1986 a4454ac460 feat: DEFAULT_FOLDER pre-selects a download folder (closes #875)
Most downloads from a given install land in the same custom directory,
which today means picking it by hand every time. DEFAULT_FOLDER seeds the
folder field once the configuration arrives; the field stays editable, so
per-download folders still work, and a folder already typed this session
is not overwritten.

The value is trimmed of surrounding slashes, and dropped with a warning
when CUSTOM_DIRS is off, since the UI hides the field in that mode and
the download path check rejects a folder anyway.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-16 12:53:07 +02:00
tjelite1986 75fe1f0c11 feat: shift-click to select a range of rows (closes #525)
Clicking a row's checkbox with Shift held takes every row between it and
the last one toggled to the state the click produced, so a stretch of a
long queue can be cancelled without ticking each entry.

The range follows the order the rows are rendered in, which for the
Completed list is the sort the user picked rather than the order the map
holds; the master checkbox takes that order as an input. An anchor whose
row has since left the list (a download finishing moves it to Completed)
falls back to a plain toggle.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-16 12:49:39 +02:00
Matt Van Horn 5826d0dc2b fix: surface yt-dlp warning context on failed downloads
Fixes #1047
2026-08-15 18:31:45 -07:00
13 changed files with 790 additions and 10 deletions
+3
View File
@@ -62,6 +62,7 @@ Certain values can be set via environment variables, using the `-e` parameter on
* __CUSTOM_DIRS__: Whether to enable downloading videos into custom directories within the __DOWNLOAD_DIR__ (or __AUDIO_DOWNLOAD_DIR__). When enabled, a dropdown appears next to the Add button to specify the download directory. Defaults to `true`.
* __CREATE_CUSTOM_DIRS__: Whether to support automatically creating directories within the __DOWNLOAD_DIR__ (or __AUDIO_DOWNLOAD_DIR__) if they do not exist. When enabled, the download directory selector supports free-text input, and the specified directory will be created recursively. Defaults to `true`.
* __CUSTOM_DIRS_EXCLUDE_REGEX__: Regular expression to exclude some custom directories from the dropdown. Empty regex disables exclusion. Defaults to `(^|/)[.@].*$`, which means directories starting with `.` or `@`.
* __DEFAULT_FOLDER__: Custom directory to pre-select in the download folder field, relative to __DOWNLOAD_DIR__ (or __AUDIO_DOWNLOAD_DIR__), for when most downloads go to the same place. It is only a starting value — the field stays editable, so any other folder can still be picked per download. Requires __CUSTOM_DIRS__; ignored with a warning otherwise. Defaults to empty, i.e. the base download directory.
* __DOWNLOAD_DIRS_INDEXABLE__: If `true`, the download directories (__DOWNLOAD_DIR__ and __AUDIO_DOWNLOAD_DIR__) are indexable on the web server. Defaults to `false`.
* __STATE_DIR__: Path to where MeTube will store its persistent state files (`queue.json`, `pending.json`, `completed.json`, `subscriptions.json`). Defaults to `/downloads/.metube` in the Docker image, and `.` otherwise.
* __TEMP_DIR__: Path where intermediary download files will be saved. Defaults to `/downloads` in the Docker image, and `.` otherwise.
@@ -83,6 +84,8 @@ Certain values can be set via environment variables, using the `-e` parameter on
* __ALLOW_PRIVATE_ADDRESSES__: Whether to allow downloads from private, loopback, link-local and other non-global addresses. Defaults to `false`, which protects against SSRF by refusing URLs that resolve to internal hosts. Set to `true` only in trusted environments — for example when routing traffic through a proxy/VPN client in Fake-IP mode (sing-box, Clash, Mihomo), which resolves hosts to the `198.18.0.0/15` range. Enabling this disables the SSRF protection entirely, so only use it when you control the network. You do **not** need this to use a proxy on an internal address: a proxy configured through the `proxy` option in `YTDL_OPTIONS` (or the `*_proxy` environment variables) is always reachable at its own host and port, wherever it lives.
* __YTDL_NIGHTLY_UPDATE_TIME__: If set, will cause MeTube to use [nightly yt-dlp builds](https://github.com/yt-dlp/yt-dlp-nightly-builds) instead of the stable releases. Set to the time (`HH:MM`, 24-hour) when you want the daily upgrades and MeTube restart to happen. Defaults to empty (disabled).
A filename that would exceed the limit the filesystem accepts is shortened to fit, keeping its extension, with room left for the suffixes yt-dlp adds while downloading. Sites that put a long description in the title would otherwise fail the download outright with `File name too long`. Use `trim_file_name` in `YTDL_OPTIONS` if you want names shorter than the filesystem's own limit, or `restrictfilenames` to strip non-ASCII characters.
Enabling `writeinfojson` or `writethumbnail` in `YTDL_OPTIONS` also writes a feed-level `.info.json` and thumbnail when you add a playlist or channel. These reuse the template of the items they belong to — `OUTPUT_TEMPLATE_CHANNEL` or `OUTPUT_TEMPLATE_PLAYLIST` — evaluated against the feed itself, so with the defaults they land in the same folder as the videos, named after the feed. Set `allow_playlist_files` to `false` in `YTDL_OPTIONS` to skip them.
### 🌐 Web Server & URLs
+14
View File
@@ -62,6 +62,7 @@ class Config:
'CUSTOM_DIRS': 'true',
'CREATE_CUSTOM_DIRS': 'true',
'CUSTOM_DIRS_EXCLUDE_REGEX': r'(^|/)[.@].*$',
'DEFAULT_FOLDER': '',
'DELETE_FILE_ON_TRASHCAN': 'false',
'STATE_DIR': '.',
'URL_PREFIX': '',
@@ -127,6 +128,18 @@ class Config:
if val and not val.endswith('/'):
setattr(self, attr, val + '/')
# DEFAULT_FOLDER only pre-fills the form's folder field, which the UI
# does not even show without CUSTOM_DIRS. Sending one anyway would fail
# every download on the server's own folder check, so drop it and say so
# rather than leaving the user with a form that cannot submit.
self.DEFAULT_FOLDER = self.DEFAULT_FOLDER.strip().strip('/')
if self.DEFAULT_FOLDER and not self.CUSTOM_DIRS:
log.warning(
'Ignoring DEFAULT_FOLDER "%s" because CUSTOM_DIRS is not enabled',
self.DEFAULT_FOLDER,
)
self.DEFAULT_FOLDER = ''
# Convert relative addresses to absolute addresses to prevent the failure of file address comparison
if self.YTDL_OPTIONS_FILE and self.YTDL_OPTIONS_FILE.startswith('.'):
self.YTDL_OPTIONS_FILE = str(Path(self.YTDL_OPTIONS_FILE).resolve())
@@ -187,6 +200,7 @@ class Config:
_FRONTEND_KEYS = (
'CUSTOM_DIRS',
'CREATE_CUSTOM_DIRS',
'DEFAULT_FOLDER',
'OUTPUT_TEMPLATE_CHAPTER',
'PUBLIC_HOST_URL',
'PUBLIC_HOST_AUDIO_URL',
+22
View File
@@ -115,6 +115,28 @@ class ConfigTests(unittest.TestCase):
self.assertNotIn("HOST", safe)
self.assertEqual(safe["ALLOW_YTDL_OPTIONS_OVERRIDES"], False)
def test_default_folder_empty_by_default(self):
with patch.dict(os.environ, _base_env(), clear=False):
c = Config()
self.assertEqual(c.DEFAULT_FOLDER, "")
def test_default_folder_is_trimmed_and_reaches_the_frontend(self):
with patch.dict(os.environ, _base_env(DEFAULT_FOLDER=" /youtube/ "), clear=False):
c = Config()
self.assertEqual(c.DEFAULT_FOLDER, "youtube")
self.assertEqual(c.frontend_safe()["DEFAULT_FOLDER"], "youtube")
def test_default_folder_ignored_without_custom_dirs(self):
# The folder field is not shown at all without CUSTOM_DIRS, and sending
# a folder anyway is rejected by the download path check.
with patch.dict(
os.environ,
_base_env(DEFAULT_FOLDER="youtube", CUSTOM_DIRS="false"),
clear=False,
):
c = Config()
self.assertEqual(c.DEFAULT_FOLDER, "")
def test_allow_ytdl_options_overrides_boolean_loaded(self):
with patch.dict(os.environ, _base_env(ALLOW_YTDL_OPTIONS_OVERRIDES="true"), clear=False):
c = Config()
+76
View File
@@ -625,6 +625,82 @@ async def test_playlist_download_not_treated_as_channel(dq_env):
assert download.output_template.startswith("My Playlist/")
def _channel_extraction(entry_id, **extra):
"""A channel yt-dlp reported as a playlist, addressed by *entry_id*."""
return {
"_type": "playlist",
"id": entry_id,
"channel_id": "UCabcd123",
"channel": "Odin",
"title": "Odin",
**extra,
"entries": [
{
"id": "vid1",
"title": "Salvia Plath - Pondering",
"url": "https://example.com/watch?v=1",
"webpage_url": "https://example.com/watch?v=1",
"channel": "Odin",
"upload_date": "20130804",
},
],
}
async def _add_and_get_template(dq_env, extraction, url):
dq_env.OUTPUT_TEMPLATE = "%(channel)s [YT]/%(title)s.%(ext)s"
dq_env.OUTPUT_TEMPLATE_CHANNEL = ""
dq_env.OUTPUT_TEMPLATE_PLAYLIST = "%(playlist_title)s/%(title)s.%(ext)s"
def fake_extract(self, _url, *_args, **_kwargs):
return extraction
dq = DownloadQueue(dq_env, AsyncMock())
with patch.object(DownloadQueue, "_DownloadQueue__extract_info", fake_extract):
result = await dq.add(url, "video", "auto", "any", "best", "", "", 0, auto_start=False)
assert result["status"] == "ok"
return dq.pending.get("https://example.com/watch?v=1").output_template
@pytest.mark.asyncio
async def test_bare_handle_channel_url_is_treated_as_a_channel(dq_env):
"""A channel addressed as /@handle reports its id as the handle, not the
channel id, and was falling through to OUTPUT_TEMPLATE_PLAYLIST."""
template = await _add_and_get_template(
dq_env,
_channel_extraction("@odin", uploader_id="@odin"),
"https://www.youtube.com/@odin",
)
assert template.startswith("Odin [YT]/")
@pytest.mark.asyncio
async def test_legacy_vanity_channel_url_is_treated_as_a_channel(dq_env):
"""A legacy /c/Name URL reports the vanity name as its id, while
uploader_id is still the handle."""
template = await _add_and_get_template(
dq_env,
_channel_extraction("Odin", uploader_id="@odin"),
"https://www.youtube.com/c/Odin",
)
assert template.startswith("Odin [YT]/")
@pytest.mark.asyncio
async def test_playlist_with_owner_uploader_id_is_still_a_playlist(dq_env):
"""A real playlist carries its owner's channel_id and uploader_id, but its
own id matches neither, so it must keep the playlist template."""
template = await _add_and_get_template(
dq_env,
_channel_extraction("PLxyz789", uploader_id="@odin", title="My Playlist"),
"https://www.youtube.com/playlist?list=PLxyz789",
)
assert template.startswith("My Playlist/")
@pytest.mark.asyncio
async def test_add_merges_global_preset_and_override_options(dq_env):
notifier = AsyncMock()
+244
View File
@@ -50,6 +50,7 @@ class _YoutubeDL:
fake_utils.DownloadError = type("DownloadError", (Exception,), {})
fake_utils.YoutubeDLError = fake_utils.DownloadError
fake_yt_dlp.YoutubeDL = _YoutubeDL
fake_impersonate.ImpersonateTarget = _ImpersonateTarget
fake_networking.impersonate = fake_impersonate
@@ -376,6 +377,49 @@ class ConfinedYoutubeDLTests(unittest.TestCase):
self.assertEqual(self._prepared_path(""), "")
self.assertEqual(self._prepared_path("-"), "-")
def test_overlong_name_is_trimmed_to_fit_the_filesystem(self):
# A title long enough to blow the filename limit is what made these
# downloads fail outright with [Errno 36] File name too long.
long_path = os.path.join(self.base, "a" * 400 + ".mp4")
result = self._prepared_path(long_path)
name = os.path.basename(result)
self.assertTrue(name.endswith(".mp4"))
self.assertLessEqual(len(name.encode("utf-8")), 255 - 32)
self.assertEqual(os.path.dirname(result), self.base)
# The file must still be writable once yt-dlp adds its own suffixes.
self.assertLessEqual(len(f"{name}.f1229065279304024v.part".encode("utf-8")), 255)
def test_name_within_the_limit_is_left_alone(self):
ok = os.path.join(self.base, "Ordinary Title.mp4")
self.assertEqual(self._prepared_path(ok), ok)
def test_limit_counts_bytes_not_characters(self):
# 200 CJK characters are 600 bytes: a character count would pass this.
long_path = os.path.join(self.base, "" * 200 + ".mp4")
name = os.path.basename(self._prepared_path(long_path))
self.assertLessEqual(len(name.encode("utf-8")), 255 - 32)
# A trim landing mid-character must not leave a broken byte sequence.
self.assertEqual(name, name.encode("utf-8").decode("utf-8"))
self.assertTrue(name.endswith(".mp4"))
def test_a_long_tail_is_not_mistaken_for_an_extension(self):
# os.path.splitext on a title containing a dot late in the string would
# otherwise "preserve" a 100-character extension and trim nothing.
long_path = os.path.join(self.base, "b" * 300 + "." + "c" * 100)
name = os.path.basename(self._prepared_path(long_path))
self.assertLessEqual(len(name.encode("utf-8")), 255 - 32)
def test_trimming_still_cannot_escape_the_download_directory(self):
escaping = os.path.join(self.base, "..", "..", "d" * 400 + ".mp4")
with self.assertRaises(ytdl.yt_dlp.utils.DownloadError):
self._prepared_path(escaping)
class SanitizeEntryForPickleTests(unittest.TestCase):
def test_nested(self):
@@ -434,6 +478,206 @@ def _make_test_download() -> Download:
return Download("/tmp", "/tmp", "%(title)s.%(ext)s", "%(title)s.%(ext)s", "best", "any", {}, info)
class DownloadLoggerTests(unittest.TestCase):
def test_routes_messages_and_retains_only_non_empty_warnings(self):
logger = ytdl._DownloadYtdlLogger()
with self.assertLogs('ytdl', level='DEBUG') as logs:
logger.debug('debug detail')
logger.warning(' useful warning ')
logger.warning(' ')
logger.error('error detail')
self.assertEqual(logger.warnings, ['useful warning'])
self.assertIn('DEBUG:ytdl:debug detail', logs.output)
self.assertIn('WARNING:ytdl: useful warning ', logs.output)
self.assertIn('ERROR:ytdl:error detail', logs.output)
def test_retains_only_the_last_distinct_warnings(self):
logger = ytdl._DownloadYtdlLogger()
cap = ytdl._MAX_RETAINED_WARNINGS
with self.assertLogs('ytdl', level='WARNING') as logs:
for index in range(cap + 3):
logger.warning(f'fragment {index} not found')
self.assertEqual(
logger.warnings,
[f'fragment {index} not found' for index in range(3, cap + 3)],
)
# Every warning still reaches the log; only the retained list is bounded.
self.assertEqual(len(logs.output), cap + 3)
def test_repeated_warning_is_retained_once(self):
logger = ytdl._DownloadYtdlLogger()
with self.assertLogs('ytdl', level='WARNING'):
logger.warning('Requested format is not available')
logger.warning('Only images are available for download')
logger.warning('Requested format is not available')
self.assertEqual(
logger.warnings,
['Requested format is not available', 'Only images are available for download'],
)
def test_failure_message_puts_the_error_last(self):
logger = ytdl._DownloadYtdlLogger()
with self.assertLogs('ytdl', level='WARNING'):
logger.warning('Only images are available for download')
self.assertEqual(
logger.failure_message('ERROR: [youtube] u2HSc2Ym1Vk: No video formats found!'),
'Only images are available for download\n'
'ERROR: [youtube] u2HSc2Ym1Vk: No video formats found!',
)
def test_failure_message_skips_a_last_warning_that_repeats_the_error(self):
logger = ytdl._DownloadYtdlLogger()
with self.assertLogs('ytdl', level='WARNING'):
logger.warning('Video unavailable')
# yt-dlp labels errors but hands warnings to the logger unlabelled,
# so the same text can arrive through both routes.
logger.warning('Requested format is not available')
self.assertEqual(
logger.failure_message('ERROR: Requested format is not available'),
'Video unavailable\nERROR: Requested format is not available',
)
def test_failure_message_without_warnings_is_the_error_alone(self):
logger = ytdl._DownloadYtdlLogger()
self.assertEqual(logger.failure_message('ERROR: boom'), 'ERROR: boom')
class DownloadResultTests(unittest.TestCase):
def _run_download(self, result=0, warnings=(), error=None):
download = _make_test_download()
statuses = []
download.status_queue = types.SimpleNamespace(put=statuses.append)
captured_params = {}
class FakeYoutubeDL:
def download(self, urls):
self.urls = urls
for warning in warnings:
captured_params['logger'].warning(warning)
if error is not None:
raise error
return result
def make_youtube_dl(params):
captured_params.update(params)
return FakeYoutubeDL()
with patch.object(download, '_make_youtube_dl', side_effect=make_youtube_dl), \
patch('ytdl.install_socket_guard'), \
patch('ytdl.os.setpgrp'):
download._download()
return statuses, captured_params
def test_nonzero_result_includes_warning_context_and_forwards_logs(self):
warnings = [
'The uploader has blocked this video in your country',
'No video formats found',
]
with self.assertLogs('ytdl', level='WARNING') as logs:
statuses, params = self._run_download(result=1, warnings=warnings)
self.assertEqual(
statuses[-1],
{'status': 'error', 'msg': '\n'.join(warnings)},
)
self.assertIs(params['logger'].__class__, ytdl._DownloadYtdlLogger)
for warning in warnings:
self.assertTrue(any(warning in entry for entry in logs.output))
def test_nonzero_result_without_warning_uses_fallback_message(self):
statuses, _ = self._run_download(result=2)
self.assertEqual(
statuses[-1],
{'status': 'error', 'msg': 'yt-dlp failed with exit code 2'},
)
def test_warning_does_not_change_success_status(self):
statuses, _ = self._run_download(result=0, warnings=['A recoverable warning'])
self.assertEqual(statuses[-1], {'status': 'finished'})
def test_youtube_dl_error_carries_the_warnings_that_explain_it(self):
# The sequence from issue #1047: yt-dlp raises DownloadError, so the
# warnings naming the real cause only reach the user if the exception
# branch carries them too.
statuses, _ = self._run_download(
warnings=[
'[youtube] Video unavailable. This video contains content from bryhuangpub,'
' who has blocked it from display on this website or application',
'Only images are available for download. use --list-formats to see them',
'Requested format is not available',
],
error=ytdl.yt_dlp.utils.YoutubeDLError(
'ERROR: [youtube] u2HSc2Ym1Vk: No video formats found!'
),
)
self.assertEqual(
statuses[-1],
{
'status': 'error',
'msg': '[youtube] Video unavailable. This video contains content from bryhuangpub,'
' who has blocked it from display on this website or application\n'
'Only images are available for download. use --list-formats to see them\n'
'Requested format is not available\n'
'ERROR: [youtube] u2HSc2Ym1Vk: No video formats found!',
},
)
def test_youtube_dl_error_drops_a_last_warning_that_repeats_it(self):
statuses, _ = self._run_download(
warnings=['Earlier warning', 'Requested format is not available'],
error=ytdl.yt_dlp.utils.YoutubeDLError('ERROR: Requested format is not available'),
)
self.assertEqual(
statuses[-1],
{
'status': 'error',
'msg': 'Earlier warning\nERROR: Requested format is not available',
},
)
def test_youtube_dl_error_message_is_bounded(self):
cap = ytdl._MAX_RETAINED_WARNINGS
statuses, _ = self._run_download(
warnings=[f'fragment {index} not found' for index in range(cap + 4)],
error=ytdl.yt_dlp.utils.YoutubeDLError('ERROR: giving up'),
)
msg = statuses[-1]['msg']
self.assertEqual(
msg.split('\n'),
[f'fragment {index} not found' for index in range(4, cap + 4)] + ['ERROR: giving up'],
)
def test_nonzero_result_message_is_bounded(self):
cap = ytdl._MAX_RETAINED_WARNINGS
statuses, _ = self._run_download(
result=1,
warnings=[f'fragment {index} not found' for index in range(cap + 4)],
)
self.assertEqual(
statuses[-1]['msg'].split('\n'),
[f'fragment {index} not found' for index in range(4, cap + 4)],
)
class ProgressThrottleTests(unittest.TestCase):
def test_downloading_ticks_are_throttled(self):
dl = _make_test_download()
+141 -6
View File
@@ -32,6 +32,55 @@ from urllib.parse import urlsplit
log = logging.getLogger('ytdl')
# Fragmented and live downloads can emit a warning per fragment, and the joined
# text is persisted with the completed queue and broadcast to every client, so
# only the last few distinct warnings are kept.
_MAX_RETAINED_WARNINGS = 5
_REPORT_LABEL_RE = re.compile(r'^(?:ERROR|WARNING):\s*')
def _report_body(message):
"""yt-dlp labels errors with an ``ERROR:`` prefix but hands warnings to the
logger unlabelled, so compare the two with any such label removed."""
return _REPORT_LABEL_RE.sub('', message).strip()
class _DownloadYtdlLogger:
"""Forward yt-dlp output while retaining warnings for failed downloads."""
def __init__(self):
self._warnings = collections.deque(maxlen=_MAX_RETAINED_WARNINGS)
@property
def warnings(self):
return list(self._warnings)
def debug(self, msg):
log.debug('%s', msg)
def warning(self, msg):
log.warning('%s', msg)
if msg is not None and (warning := str(msg).strip()) and warning not in self._warnings:
self._warnings.append(warning)
def error(self, msg):
log.error('%s', msg)
def failure_message(self, error_text):
"""Retained warnings followed by *error_text*, kept last so the actual
error stays prominent under the context that explains it."""
lines = self.warnings
error_text = (error_text or '').strip()
if not error_text:
return '\n'.join(lines)
if lines and _report_body(lines[-1]) == _report_body(error_text):
lines.pop()
lines.append(error_text)
return '\n'.join(lines)
# Python 3.14 switches the default multiprocessing start method on Linux
# (this app's only supported deployment target, per the Dockerfile) from fork
# to forkserver. Download._download relies on inheriting process state the
@@ -147,6 +196,56 @@ def _sanitize_path_component(value: Any) -> Any:
return value.lstrip('.').strip() or '_'
# Room left for the suffixes yt-dlp appends after prepare_filename has run:
# '.part' and '.ytdl' while the download is in flight, '.f<format_id>' for a
# stream fetched on its own before merging, '-Frag<n>' for fragmented
# downloads. A name trimmed to exactly the limit would still fail the moment
# one of those is added, which is what the '.part' in the reported errors is.
_NAME_SUFFIX_RESERVE_BYTES = 32
# POSIX guarantees at least this much, and it is what ext4/xfs/btrfs allow.
_FALLBACK_NAME_MAX_BYTES = 255
# Keep a recognisable stem even on a filesystem with a very short limit.
_MIN_STEM_BYTES = 16
# Longer than this is not really an extension (a title ending in '.something'),
# so the whole name is treated as the stem rather than preserving it.
_MAX_EXT_BYTES = 16
def _name_max_bytes(directory: str) -> int:
"""The filesystem's filename limit, in bytes, for *directory*."""
try:
return int(os.pathconf(directory or '.', 'PC_NAME_MAX'))
except (OSError, ValueError, AttributeError):
# The directory may not exist yet (CREATE_CUSTOM_DIRS makes it during
# the download), and pathconf is not available on every platform.
return _FALLBACK_NAME_MAX_BYTES
def _trim_to_name_max(path: str) -> str:
"""Shorten the final component of *path* to what the filesystem accepts.
The limit is a byte count, not a character count: a title of accented or
CJK characters hits it in half as many characters, or fewer. The extension
is preserved, since it is what decides how the file is handled afterwards.
"""
directory, name = os.path.split(path)
if not name:
return path
encoded = name.encode('utf-8', 'surrogatepass')
limit = _name_max_bytes(directory) - _NAME_SUFFIX_RESERVE_BYTES
if len(encoded) <= limit:
return path
stem, ext = os.path.splitext(name)
ext_bytes = ext.encode('utf-8', 'surrogatepass')
if len(ext_bytes) > _MAX_EXT_BYTES:
stem, ext, ext_bytes = name, '', b''
stem_limit = max(limit - len(ext_bytes), _MIN_STEM_BYTES)
# 'ignore' drops a multi-byte character the cut landed inside of.
trimmed = stem.encode('utf-8', 'surrogatepass')[:stem_limit].decode('utf-8', 'ignore').rstrip()
return os.path.join(directory, (trimmed or '_') + ext)
class _ConfinedYoutubeDL(yt_dlp.YoutubeDL):
"""A ``YoutubeDL`` that refuses to emit any output path outside the allowed roots.
@@ -170,6 +269,12 @@ class _ConfinedYoutubeDL(yt_dlp.YoutubeDL):
def prepare_filename(self, *args, **kwargs):
filename = super().prepare_filename(*args, **kwargs)
# Titles long enough to exceed the filesystem's filename limit are
# common on some sites, and the download fails outright when they do.
# Every output path comes through here, so trimming once keeps the
# main file, its chapter files, thumbnails and subtitles consistent.
if filename and filename != '-':
filename = _trim_to_name_max(filename)
if filename and filename != '-' and self._allowed_roots:
resolved = os.path.realpath(filename)
if not any(_is_within_directory(root, resolved) for root in self._allowed_roots):
@@ -664,6 +769,9 @@ class Download:
# anything else. Skipped when ALLOW_PRIVATE_ADDRESSES trusts the environment.
install_socket_guard(self.allow_private, proxy_urls=(self.ytdl_opts.get('proxy'),))
log.info(f"Starting download for: {self.info.title} ({self.info.url})")
# Bound outside the try so the except branch can read what was captured
# before the error was raised.
ytdl_logger = _DownloadYtdlLogger()
try:
debug_logging = logging.getLogger().isEnabledFor(logging.DEBUG)
put_status = self._make_progress_hook()
@@ -710,6 +818,9 @@ class Download:
'postprocessor_hooks': [put_status_postprocessor],
**self.ytdl_opts,
}
# Set after the ytdl_opts merge: the failure messages below depend on
# this logger, so a user-supplied one must not replace it.
ytdl_params['logger'] = ytdl_logger
# Add chapter splitting options if enabled
if self.info.split_by_chapters:
@@ -732,11 +843,15 @@ class Download:
)
ret = self._make_youtube_dl(ytdl_params).download([self.info.url])
self.status_queue.put({'status': 'finished' if ret == 0 else 'error'})
if ret == 0:
self.status_queue.put({'status': 'finished'})
else:
msg = '\n'.join(ytdl_logger.warnings) or f'yt-dlp failed with exit code {ret}'
self.status_queue.put({'status': 'error', 'msg': msg})
log.info(f"Finished download for: {self.info.title}")
except yt_dlp.utils.YoutubeDLError as exc:
log.error(f"Download error for {self.info.title}: {str(exc)}")
self.status_queue.put({'status': 'error', 'msg': str(exc)})
self.status_queue.put({'status': 'error', 'msg': ytdl_logger.failure_message(str(exc))})
async def start(self, notifier, executor=None):
log.info(f"Preparing download for: {self.info.title}")
@@ -1063,13 +1178,33 @@ class DownloadQueue:
@staticmethod
def __is_channel_extraction(entry):
"""Return True when yt-dlp reported a channel tab as a playlist.
"""Return True when yt-dlp reported a channel as a playlist.
YouTube channel tabs are extracted with ``_type: 'playlist'`` but set
``id`` equal to ``channel_id``; real playlists keep a distinct id.
A channel *tab* -- ``/channel/UC...``, ``/@handle/videos``, and the
streams, shorts and playlists tabs -- is extracted with ``id`` equal to
``channel_id``. A channel addressed without a tab keeps the form it was
asked for instead: ``@handle`` for a handle URL and the vanity name for
a legacy ``/c/`` URL. Both of those match ``uploader_id``, which is the
handle either way, so compare against it as well.
A real playlist has an id of its own and matches neither, even though
it also carries its owner's ``channel_id``.
"""
channel_id = entry.get('channel_id')
return bool(channel_id) and entry.get('id') == channel_id
entry_id = entry.get('id')
if not channel_id or not entry_id:
return False
if entry_id == channel_id:
return True
uploader_id = entry.get('uploader_id')
if not uploader_id:
return False
# Compared without case because a legacy vanity name and the handle it
# became need not agree on it. No playlist id can collide here: those
# are 'PL...', 'OLAK...' and the like, never a handle.
handle = uploader_id.casefold()
entry_id = entry_id.casefold()
return handle in (entry_id, f'@{entry_id}')
async def __import_queue(self):
for k, v in self.queue.saved_items():
+3 -1
View File
@@ -693,6 +693,7 @@
<app-select-all-checkbox #queueMasterCheckboxRef [id]="'queue'" [list]="downloads.queue" (changed)="queueSelectionChanged($event)" />
</th>
<th scope="col">Video</th>
<th scope="col" style="width: 7rem;">Format</th>
<th scope="col" style="width: 8rem;">Speed</th>
<th scope="col" style="width: 7rem;">ETA</th>
<th scope="col" style="width: 6rem;"></th>
@@ -726,6 +727,7 @@
}
</div>
</td>
<td class="text-nowrap">{{ formatLabel(download.value) }}</td>
<td>{{ download.value.speed | speed }}</td>
<td>{{ download.value.eta | eta }}</td>
<td>
@@ -757,7 +759,7 @@
<thead>
<tr>
<th scope="col" style="width: 1rem;">
<app-select-all-checkbox #doneMasterCheckboxRef [id]="'done'" [list]="downloads.done" (changed)="doneSelectionChanged($event)" />
<app-select-all-checkbox #doneMasterCheckboxRef [id]="'done'" [list]="downloads.done" [orderedIds]="cachedSortedDoneIds" (changed)="doneSelectionChanged($event)" />
</th>
<th scope="col">Video</th>
<th scope="col">Type</th>
+60
View File
@@ -6,6 +6,7 @@ import { DownloadsService } from './services/downloads.service';
import { SubscriptionsService } from './services/subscriptions.service';
import { ToastService } from './services/toast.service';
import { CookieService } from 'ngx-cookie-service';
import { Download } from './interfaces';
class DownloadsServiceStub {
loading = false;
@@ -148,6 +149,25 @@ describe('App', () => {
expect(app).toBeTruthy();
});
it('pre-fills the download folder from DEFAULT_FOLDER', () => {
const fixture = TestBed.createComponent(App);
fixture.detectChanges();
downloads.configurationChanged.next({ DEFAULT_FOLDER: 'youtube' });
expect(fixture.componentInstance.folder).toBe('youtube');
});
it('does not overwrite a folder the user already typed', () => {
const fixture = TestBed.createComponent(App);
fixture.detectChanges();
fixture.componentInstance.folder = 'music';
downloads.configurationChanged.next({ DEFAULT_FOLDER: 'youtube' });
expect(fixture.componentInstance.folder).toBe('music');
});
it('asIsOrder returns a stable comparator value (insertion order preserved)', () => {
const fixture = TestBed.createComponent(App);
const app = fixture.componentInstance;
@@ -229,6 +249,46 @@ describe('App', () => {
expect(root.textContent).toContain('starts in');
});
it('shows the queued format in the Downloading table', () => {
downloads.queue.set('https://example.com/v', {
id: 'v1',
title: 'Some Video',
url: 'https://example.com/v',
download_type: 'audio',
quality: 'best',
format: 'flac',
folder: '',
custom_name_prefix: '',
playlist_item_limit: 0,
status: 'downloading',
msg: '',
percent: 10,
speed: 0,
eta: 0,
filename: '',
checked: false,
});
downloads.queueChanged.next();
const fixture = TestBed.createComponent(App);
fixture.detectChanges();
const row = (fixture.nativeElement as HTMLElement).querySelector('tbody tr');
expect(row?.textContent).toContain('FLAC');
});
it('labels formats the way the form does, and copes with an unknown one', () => {
const app = TestBed.createComponent(App).componentInstance;
const base = { format: '' } as Download;
expect(app.formatLabel({ ...base, format: 'any' })).toBe('Auto');
expect(app.formatLabel({ ...base, format: 'mp4' })).toBe('MP4');
expect(app.formatLabel({ ...base, format: 'srt' })).toBe('SRT');
// A format from a record older than the option list still reads sensibly.
expect(app.formatLabel({ ...base, format: 'mkv' })).toBe('MKV');
expect(app.formatLabel(base)).toBe('-');
});
it('includes titleRegex in subscribe payload', () => {
const fixture = TestBed.createComponent(App);
const app = fixture.componentInstance;
+26
View File
@@ -137,6 +137,9 @@ export class App implements AfterViewInit, OnInit, OnDestroy {
sortAscending = false;
expandedErrors: Set<string> = new Set<string>();
cachedSortedDone: [string, Download][] = [];
// The done ids in rendered order, so a shift-click range follows the sort
// the user is looking at rather than the map's insertion order.
cachedSortedDoneIds: string[] = [];
lastCopiedErrorId: string | null = null;
private previousDownloadType = 'video';
private addRequestSub?: Subscription;
@@ -434,6 +437,12 @@ export class App implements AfterViewInit, OnInit, OnDestroy {
if (!Number.isNaN(playlistItemLimit) && playlistItemLimit > 0) {
this.playlistItemLimit = playlistItemLimit;
}
// Pre-fill the download folder, unless the user has already typed one
// this session. The server drops DEFAULT_FOLDER when CUSTOM_DIRS is
// off, so there is nothing to guard against here.
if (!this.folder) {
this.folder = String(config['DEFAULT_FOLDER'] ?? '');
}
// Set chapter template from backend config if not already set by cookie
if (!this.chapterTemplate) {
this.chapterTemplate = config['OUTPUT_TEMPLATE_CHAPTER'];
@@ -909,6 +918,22 @@ export class App implements AfterViewInit, OnInit, OnDestroy {
return type.charAt(0).toUpperCase() + type.slice(1);
}
// The format the download was queued with, labelled the way the form labels
// it, so a queued item can be told apart while it is still downloading.
formatLabel(download: Download): string {
const format = (download.format || '').trim();
if (!format) {
return '-';
}
const options: Option[] = [
...this.videoFormats,
...this.audioFormats,
...this.captionFormats,
...this.thumbnailFormats,
];
return options.find(o => o.id === format)?.text ?? format.toUpperCase();
}
formatCodecLabel(download: Download): string {
if (download.download_type !== 'video') {
const format = (download.format || '').toUpperCase();
@@ -1532,6 +1557,7 @@ export class App implements AfterViewInit, OnInit, OnDestroy {
result.reverse();
}
this.cachedSortedDone = result;
this.cachedSortedDoneIds = result.map(([key]) => key);
}
toggleErrorDetail(id: string) {
@@ -2,6 +2,38 @@ import { TestBed } from '@angular/core/testing';
import { SelectAllCheckboxComponent } from './master-checkbox.component';
import { Checkable } from '../interfaces';
function makeList(ids: string[]): Map<string, Checkable> {
const list = new Map<string, Checkable>();
for (const id of ids) {
list.set(id, { checked: false });
}
return list;
}
function makeMaster(list: Map<string, Checkable>, orderedIds: string[] | null = null) {
const fixture = TestBed.createComponent(SelectAllCheckboxComponent);
fixture.componentRef.setInput('id', 'queue');
fixture.componentRef.setInput('list', list);
if (orderedIds) {
fixture.componentRef.setInput('orderedIds', orderedIds);
}
fixture.detectChanges();
return fixture;
}
// Simulates what the item checkbox does: ngModel writes the new state, then
// the change handler reports the click to the master.
function clickItem(
master: SelectAllCheckboxComponent,
list: Map<string, Checkable>,
id: string,
shift = false,
) {
const item = list.get(id)!;
item.checked = !item.checked;
master.selectionChanged(id, shift);
}
describe('SelectAllCheckboxComponent', () => {
beforeEach(async () => {
await TestBed.configureTestingModule({
@@ -20,4 +52,87 @@ describe('SelectAllCheckboxComponent', () => {
fixture.componentInstance.clicked();
expect(list.get('u1')?.checked).toBe(true);
});
it('shift-click checks every item between the two clicks', () => {
const list = makeList(['u1', 'u2', 'u3', 'u4', 'u5']);
const master = makeMaster(list).componentInstance;
clickItem(master, list, 'u2');
clickItem(master, list, 'u4', true);
expect([...list.values()].map((i) => i.checked)).toEqual([false, true, true, true, false]);
});
it('extends upwards as well as downwards', () => {
const list = makeList(['u1', 'u2', 'u3', 'u4']);
const master = makeMaster(list).componentInstance;
clickItem(master, list, 'u4');
clickItem(master, list, 'u2', true);
expect([...list.values()].map((i) => i.checked)).toEqual([false, true, true, true]);
});
it('shift-clicking a checked box clears the range', () => {
const list = makeList(['u1', 'u2', 'u3']);
list.forEach((item) => (item.checked = true));
const master = makeMaster(list).componentInstance;
clickItem(master, list, 'u1');
clickItem(master, list, 'u3', true);
expect([...list.values()].map((i) => i.checked)).toEqual([false, false, false]);
});
it('follows the rendered order, not the map order', () => {
// The done list renders newest-first, so its rendered order is not the
// order the entries sit in the map. u2 lies inside the range on screen
// and outside it in the map, which is what separates the two.
const list = makeList(['u1', 'u2', 'u3', 'u4']);
const master = makeMaster(list, ['u4', 'u2', 'u3', 'u1']).componentInstance;
clickItem(master, list, 'u4');
clickItem(master, list, 'u3', true);
// u1 (rendered last) stays clear; u2 is swept up with the range.
expect([...list.values()].map((i) => i.checked)).toEqual([false, true, true, true]);
});
it('a plain click after a range starts a new anchor', () => {
const list = makeList(['u1', 'u2', 'u3', 'u4']);
const master = makeMaster(list).componentInstance;
clickItem(master, list, 'u1');
clickItem(master, list, 'u2', true);
clickItem(master, list, 'u4');
expect([...list.values()].map((i) => i.checked)).toEqual([true, true, false, true]);
});
it('select-all clears the anchor so the next shift-click is a plain toggle', () => {
const list = makeList(['u1', 'u2', 'u3']);
const fixture = makeMaster(list);
const master = fixture.componentInstance;
clickItem(master, list, 'u1');
master.selected = true;
master.clicked();
master.selected = false;
master.clicked();
clickItem(master, list, 'u3', true);
expect([...list.values()].map((i) => i.checked)).toEqual([false, false, true]);
});
it('ignores a range whose anchor row is gone', () => {
const list = makeList(['u1', 'u2', 'u3']);
const master = makeMaster(list).componentInstance;
clickItem(master, list, 'u1');
// The anchor finishes downloading and leaves the queue.
list.delete('u1');
clickItem(master, list, 'u3', true);
expect([...list.values()].map((i) => i.checked)).toEqual([false, true]);
});
});
@@ -20,17 +20,33 @@ import { FormsModule } from "@angular/forms";
export class SelectAllCheckboxComponent {
readonly id = input.required<string>();
readonly list = input.required<Map<string, Checkable>>();
// The ids in the order the rows are rendered. The done list is sorted for
// display, so its order is not the map's insertion order, and a range
// selection has to follow what the user sees. Left unset, the map order is
// the rendered order.
readonly orderedIds = input<string[] | null>(null);
readonly changed = output<number>();
readonly masterCheckbox = viewChild.required<ElementRef>('masterCheckbox');
selected!: boolean;
// The item a range extends from: the last one toggled on its own.
private anchorId: string | null = null;
clicked() {
this.list().forEach(item => item.checked = this.selected);
// Select-all is not a position, so there is nothing to extend from next.
this.anchorId = null;
this.selectionChanged();
}
selectionChanged() {
selectionChanged(id?: string, extend = false) {
if (id !== undefined) {
if (extend && this.anchorId !== null && this.anchorId !== id) {
this.applyRange(this.anchorId, id);
}
this.anchorId = id;
}
const masterCheckbox = this.masterCheckbox();
if (!masterCheckbox)
return;
@@ -40,4 +56,27 @@ export class SelectAllCheckboxComponent {
masterCheckbox.nativeElement.indeterminate = checked > 0 && checked < this.list().size;
this.changed.emit(checked);
}
// Everything between the anchor and the just-clicked row takes the state the
// click produced, so shift-clicking a checked box clears the range and
// shift-clicking an unchecked one fills it.
private applyRange(fromId: string, toId: string) {
const ids = this.orderedIds() ?? Array.from(this.list().keys());
const from = ids.indexOf(fromId);
const to = ids.indexOf(toId);
// A row can disappear between two clicks (a download finishing moves it
// from the queue to the done list); without both ends there is no range.
if (from < 0 || to < 0) {
return;
}
const target = this.list().get(toId)?.checked ?? false;
const start = Math.min(from, to);
const end = Math.max(from, to);
for (let i = start; i <= end; i++) {
const item = this.list().get(ids[i]);
if (item) {
item.checked = target;
}
}
}
}
@@ -22,4 +22,33 @@ describe('ItemCheckboxComponent', () => {
itemFixture.detectChanges();
expect(itemFixture.componentInstance).toBeTruthy();
});
it('reports the shift modifier from the click to the master', () => {
const masterFixture = TestBed.createComponent(SelectAllCheckboxComponent);
masterFixture.componentRef.setInput('id', 'q');
masterFixture.componentRef.setInput('list', new Map());
masterFixture.detectChanges();
const master = masterFixture.componentInstance;
const reported: [string | undefined, boolean | undefined][] = [];
master.selectionChanged = (id?: string, extend?: boolean) => {
reported.push([id, extend]);
};
const itemFixture = TestBed.createComponent(ItemCheckboxComponent);
itemFixture.componentRef.setInput('id', 'row1');
itemFixture.componentRef.setInput('master', master);
itemFixture.componentRef.setInput('checkable', { checked: false });
itemFixture.detectChanges();
const item = itemFixture.componentInstance;
item.clicked(new MouseEvent('click', { shiftKey: true }));
item.changed();
// The modifier must not stick to the next toggle.
item.changed();
expect(reported).toEqual([
['row1', true],
['row1', false],
]);
});
});
@@ -7,7 +7,7 @@ import { FormsModule } from '@angular/forms';
selector: 'app-item-checkbox',
template: `
<div class="form-check">
<input type="checkbox" class="form-check-input" id="{{master().id()}}-{{id()}}-select" [(ngModel)]="checkable().checked" (change)="master().selectionChanged()" [attr.aria-label]="'Select item ' + id()">
<input type="checkbox" class="form-check-input" id="{{master().id()}}-{{id()}}-select" [(ngModel)]="checkable().checked" (click)="clicked($event)" (change)="changed()" [attr.aria-label]="'Select item ' + id()">
<label class="form-check-label visually-hidden" for="{{master().id()}}-{{id()}}-select">Select item</label>
</div>
`,
@@ -22,4 +22,19 @@ export class ItemCheckboxComponent {
readonly id = input.required<string>();
readonly master = input.required<SelectAllCheckboxComponent>();
readonly checkable = input.required<Checkable>();
// click fires before change, so the modifier is recorded here and read once
// ngModel has written the new state into the checkable. Keyboard activation
// fires change without a click, which is a plain toggle.
private extend = false;
clicked(event: MouseEvent) {
this.extend = event.shiftKey;
}
changed() {
const extend = this.extend;
this.extend = false;
this.master().selectionChanged(this.id(), extend);
}
}