mirror of
https://github.com/JefferyHcool/BiliNote.git
synced 2026-08-30 12:40:07 +08:00
fix(downloader): yt-dlp 未设 retries 时零重试，网络抖动一次任务即失败
## 问题
任意一次瞬时网络故障都会让整个笔记任务失败，即便立刻重试同一个链接就能成功。
实际遇到的报错：
ERROR: [download] Got error: HTTPSConnectionPool(
host='upos-sz-mirrorcosov.bilivideo.com', port=443): Read timed out.
同一个视频一分钟后重新提交，10.18 MiB 四秒下完。
## 原因
yt-dlp 文档里 `retries` 默认 10，但那个默认值是**命令行参数解析器**给的，
Python API 不套用。项目所有 `ydl_opts` 都没设 `retries`，于是：
# 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
也就是**每次下载只尝试一次，零重试**。这不是 B 站独有的问题，
YouTube 以及所有走 yt-dlp 的路径都一样。
## 改动
- `base.py`：新增 `YDL_RETRY_OPTS`（`retries` / `fragment_retries` /
`socket_timeout`），两个 downloader 本来就从 base 导入，不额外引入模块。
- `youtube_downloader.py`（2 处）、`bilibili_downloader.py`（3 处）：
所有 `ydl_opts` 都展开该常量。只修报错的那一处会把其余四处继续留在零重试。
- 用的是 yt-dlp 自带的重试机制，没有自写重试循环。
取值偏保守（3 次而非 CLI 的 10）：笔记任务是用户在前台等的，
重试太久不如早点失败让用户重来。
## 测试
新增 `tests/test_ydl_retry_opts.py`：
- 行为：常量给出的 RetryManager 预算 > 0；并显式钉住
`RetryManager(None).retries == 0` 这个被规避的坑。
- 结构：用 AST 断言两个 downloader 里**每一个** `ydl_opts` 字面量都展开了
`YDL_RETRY_OPTS`，防止以后新增下载路径时又悄悄回到零重试。
结构用例确认过 red-green：去掉任一处展开即失败，并指出具体文件行号。
无 yt-dlp 的环境下两个行为用例自动 skip，与仓库既有测试风格一致。
This commit is contained in:
pumpkinperson996
@@ -13,6 +13,19 @@ QUALITY_MAP = {
|
||||
"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):
|
||||
|
||||
@@ -7,7 +7,7 @@ from typing import Union, Optional, List
|
||||
|
||||
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
|
||||
@@ -63,6 +63,7 @@ class BilibiliDownloader(Downloader, ABC):
|
||||
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'},
|
||||
@@ -122,6 +123,7 @@ class BilibiliDownloader(Downloader, ABC):
|
||||
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'},
|
||||
@@ -183,6 +185,7 @@ class BilibiliDownloader(Downloader, ABC):
|
||||
video_id = extract_video_id(video_url, "bilibili")
|
||||
|
||||
ydl_opts = {
|
||||
**YDL_RETRY_OPTS,
|
||||
'writesubtitles': True,
|
||||
'writeautomaticsub': True,
|
||||
'subtitleslangs': langs,
|
||||
|
||||
@@ -5,7 +5,7 @@ from typing import Union, Optional, List
|
||||
|
||||
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
|
||||
@@ -47,6 +47,7 @@ class YoutubeDownloader(Downloader, ABC):
|
||||
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,
|
||||
@@ -95,6 +96,7 @@ class YoutubeDownloader(Downloader, ABC):
|
||||
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,
|
||||
|
||||
@@ -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()
|
||||
Reference in New Issue
Block a user