From 5826d0dc2b72637766ccecb13772215d767f4c06 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Sat, 15 Aug 2026 18:31:45 -0700 Subject: [PATCH] fix: surface yt-dlp warning context on failed downloads Fixes #1047 --- app/tests/test_ytdl_utils.py | 83 ++++++++++++++++++++++++++++++++++++ app/ytdl.py | 27 +++++++++++- 2 files changed, 109 insertions(+), 1 deletion(-) diff --git a/app/tests/test_ytdl_utils.py b/app/tests/test_ytdl_utils.py index 858a2aa..b671733 100644 --- a/app/tests/test_ytdl_utils.py +++ b/app/tests/test_ytdl_utils.py @@ -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 @@ -434,6 +435,88 @@ 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) + + +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_message_takes_precedence_over_warnings(self): + statuses, _ = self._run_download( + warnings=['Earlier warning'], + error=ytdl.yt_dlp.utils.YoutubeDLError('extractor failed'), + ) + + self.assertEqual(statuses[-1], {'status': 'error', 'msg': 'extractor failed'}) + + class ProgressThrottleTests(unittest.TestCase): def test_downloading_ticks_are_throttled(self): dl = _make_test_download() diff --git a/app/ytdl.py b/app/ytdl.py index df26599..e086617 100644 --- a/app/ytdl.py +++ b/app/ytdl.py @@ -32,6 +32,25 @@ from urllib.parse import urlsplit log = logging.getLogger('ytdl') + +class _DownloadYtdlLogger: + """Forward yt-dlp output while retaining warnings for failed downloads.""" + + def __init__(self): + 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()): + self.warnings.append(warning) + + def error(self, msg): + log.error('%s', msg) + + # 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 @@ -667,6 +686,7 @@ class Download: try: debug_logging = logging.getLogger().isEnabledFor(logging.DEBUG) put_status = self._make_progress_hook() + ytdl_logger = _DownloadYtdlLogger() def put_status_postprocessor(d): if d['postprocessor'] == 'MoveFiles' and d['status'] == 'finished': @@ -710,6 +730,7 @@ class Download: 'postprocessor_hooks': [put_status_postprocessor], **self.ytdl_opts, } + ytdl_params['logger'] = ytdl_logger # Add chapter splitting options if enabled if self.info.split_by_chapters: @@ -732,7 +753,11 @@ 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)}")