Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions backend/app/downloaders/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,19 @@
"slow": "128"
}

# yt-dlp 的 `retries` 默认值(10)是命令行参数解析器给的,Python API 不套用它:
# 不显式设置时 HttpFD 拿到的是 `self.params.get('retries')` == None,而
# `RetryManager.__init__` 做的是 `self.retries = _retries or 0`——也就是
# 一次都不重试。任何一次网络抖动(例如 B 站 CDN
# upos-sz-mirror*.bilivideo.com 读超时)都会让整个笔记任务直接失败。
#
# 这里的值偏保守:笔记任务是用户在前台等的,重试太多不如早点失败让用户重来。
YDL_RETRY_OPTS = {
"retries": 3,
"fragment_retries": 3,
"socket_timeout": 30,
}


class Downloader(ABC):
def __init__(self):
Expand Down
5 changes: 4 additions & 1 deletion backend/app/downloaders/bilibili_downloader.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import yt_dlp

from app.downloaders.base import Downloader, DownloadQuality, QUALITY_MAP
from app.downloaders.base import Downloader, DownloadQuality, QUALITY_MAP, YDL_RETRY_OPTS
from app.downloaders.bilibili_dm_patch import apply_bilibili_dm_img_patch
from app.downloaders.bilibili_subtitle import BilibiliSubtitleFetcher
from app.models.notes_model import AudioDownloadResult
Expand Down Expand Up @@ -63,6 +63,7 @@ def download(
output_path = os.path.join(output_dir, "%(id)s.%(ext)s")

ydl_opts = {
**YDL_RETRY_OPTS,
'format': 'bestaudio[ext=m4a]/bestaudio/best',
'outtmpl': output_path,
'http_headers': {'Referer': 'https://www.bilibili.com'},
Expand Down Expand Up @@ -122,6 +123,7 @@ def download_video(
output_path = os.path.join(output_dir, "%(id)s.%(ext)s")

ydl_opts = {
**YDL_RETRY_OPTS,
'format': 'bv*[ext=mp4]/bestvideo+bestaudio/best',
'outtmpl': output_path,
'http_headers': {'Referer': 'https://www.bilibili.com'},
Expand Down Expand Up @@ -183,6 +185,7 @@ def download_subtitles(self, video_url: str, output_dir: str = None,
video_id = extract_video_id(video_url, "bilibili")

ydl_opts = {
**YDL_RETRY_OPTS,
'writesubtitles': True,
'writeautomaticsub': True,
'subtitleslangs': langs,
Expand Down
4 changes: 3 additions & 1 deletion backend/app/downloaders/youtube_downloader.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import yt_dlp

from app.downloaders.base import Downloader, DownloadQuality
from app.downloaders.base import Downloader, DownloadQuality, YDL_RETRY_OPTS
from app.downloaders.youtube_subtitle import YouTubeSubtitleFetcher
from app.models.notes_model import AudioDownloadResult
from app.models.transcriber_model import TranscriptResult
Expand Down Expand Up @@ -47,6 +47,7 @@ def download(
output_path = os.path.join(output_dir, "%(id)s.%(ext)s")

ydl_opts = {
**YDL_RETRY_OPTS,
'format': 'bestaudio[ext=m4a]/bestaudio/best',
'outtmpl': output_path,
'noplaylist': True,
Expand Down Expand Up @@ -95,6 +96,7 @@ def download_video(
output_path = os.path.join(output_dir, "%(id)s.%(ext)s")

ydl_opts = {
**YDL_RETRY_OPTS,
'format': 'bestvideo[ext=mp4]+bestaudio[ext=m4a]/best[ext=mp4]',
'outtmpl': output_path,
'noplaylist': True,
Expand Down
125 changes: 125 additions & 0 deletions backend/tests/test_ydl_retry_opts.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
"""
Coverage for the yt-dlp retry settings shared by the downloaders.

Background: yt-dlp's documented `retries` default of 10 comes from its *command
line* option parser. Nothing applies that default to the Python API, so a
`YoutubeDL({...})` built without `retries` ends up in:

# yt_dlp/downloader/http.py
for retry in RetryManager(self.params.get('retries'), ...) # -> None
# yt_dlp/utils/_utils.py
self.retries = _retries or 0 # -> 0

i.e. exactly one attempt and no retries. A single transient network hiccup
(observed: read timeout from upos-sz-mirrorcosov.bilivideo.com) then fails the
whole note task, even though an immediate re-run succeeds.

These tests pin both halves of the fix: the constant produces a real retry
budget, and every yt-dlp options dict in the downloaders actually carries it.
"""
import ast
import importlib.util
import pathlib
import sys
import types
import unittest

ROOT = pathlib.Path(__file__).resolve().parents[1]
DOWNLOADERS = ROOT / "app" / "downloaders"
DOWNLOADER_SOURCES = ["youtube_downloader.py", "bilibili_downloader.py"]


def _load_base():
"""Load app/downloaders/base.py with its app-level imports stubbed out."""
for name, attrs in {
"app": {},
"app.enmus": {},
"app.models": {},
"app.enmus.note_enums": {"DownloadQuality": str},
"app.models.notes_model": {"AudioDownloadResult": object},
"app.models.transcriber_model": {"TranscriptResult": object},
}.items():
module = types.ModuleType(name)
for key, value in attrs.items():
setattr(module, key, value)
sys.modules.setdefault(name, module)

spec = importlib.util.spec_from_file_location("dl_base", DOWNLOADERS / "base.py")
if spec is None or spec.loader is None:
raise ImportError("base module spec not found")
module = importlib.util.module_from_spec(spec)
spec.loader.exec_module(module)
return module


class RetryOptsValueTest(unittest.TestCase):
def setUp(self):
self.base = _load_base()

def test_retry_budget_is_not_zero(self):
try:
from yt_dlp.utils import RetryManager
except Exception as exc: # pragma: no cover - env without yt-dlp
self.skipTest(f"yt-dlp not importable: {exc}")

retries = self.base.YDL_RETRY_OPTS["retries"]
budget = RetryManager(retries, lambda *a, **k: None).retries
self.assertGreater(budget, 0)

def test_documents_the_zero_default_being_guarded_against(self):
"""The bug this guards: an unset `retries` collapses to a 0 budget."""
try:
from yt_dlp.utils import RetryManager
except Exception as exc: # pragma: no cover - env without yt-dlp
self.skipTest(f"yt-dlp not importable: {exc}")

self.assertEqual(RetryManager(None, lambda *a, **k: None).retries, 0)

def test_socket_timeout_is_bounded(self):
# Without a bound, a stalled read can hang a task instead of failing
# fast enough for the retries above to be useful.
timeout = self.base.YDL_RETRY_OPTS["socket_timeout"]
self.assertGreater(timeout, 0)


class RetryOptsAreAppliedTest(unittest.TestCase):
"""
Structural check: every `ydl_opts = {...}` literal in the downloaders must
unpack YDL_RETRY_OPTS. Catches a newly added download path that silently
goes back to the zero-retry default.
"""

def _ydl_opts_dicts(self, path):
tree = ast.parse(path.read_text(encoding="utf-8"))
found = []
for node in ast.walk(tree):
if not isinstance(node, ast.Assign) or not isinstance(node.value, ast.Dict):
continue
names = [t.id for t in node.targets if isinstance(t, ast.Name)]
if "ydl_opts" in names:
found.append(node.value)
return found

def test_every_ydl_opts_dict_unpacks_retry_opts(self):
for filename in DOWNLOADER_SOURCES:
path = DOWNLOADERS / filename
dicts = self._ydl_opts_dicts(path)
self.assertTrue(dicts, f"no ydl_opts dict found in {filename}")

for index, node in enumerate(dicts):
with self.subTest(file=filename, dict_index=index, line=node.lineno):
unpacked = {
value.id
for key, value in zip(node.keys, node.values)
if key is None and isinstance(value, ast.Name)
}
self.assertIn(
"YDL_RETRY_OPTS",
unpacked,
f"{filename}:{node.lineno} builds yt-dlp options without "
f"YDL_RETRY_OPTS, so it gets zero retries",
)


if __name__ == "__main__":
unittest.main()