From e1d325b942dbcfbc12974eea6009ce9c876a33a6 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sat, 10 Oct 2026 21:56:47 +0200 Subject: [PATCH] fix(downloads): a download larger than the space left says so, not "processing failed" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit yt-dlp refuses a file announced over `max_filesize` by logging one line and returning with no file (measured, and pinned in test_formats against a local server): the worker found no output and failed the job as `postprocess`. `formats.watching_size_cap` now sees that line through our yt-dlp logger, and the download (and, should it ever happen, a YouTube clip's range fetch) fails as the owner's refusal with DOWNLOAD_TOO_BIG_MESSAGE, classified `quota_exceeded`. The download's quota copy says it does not fit in what is left — true at a full cap too — instead of "Storage full". A pre-existing type error the change touched is fixed (baseline down by one). --- backend/.basedpyright/baseline.json | 8 ---- backend/app/downloads/errors.py | 13 +++--- backend/app/downloads/formats.py | 24 ++++++++++ backend/app/downloads/pipeline.py | 36 +++++++++------ backend/tests/test_clip.py | 26 ++++++++++- backend/tests/test_download_errors.py | 1 + backend/tests/test_download_limits.py | 49 ++++++++++++++++++++- backend/tests/test_formats.py | 44 ++++++++++++++++++ frontend/src/i18n/locales/en/downloads.json | 4 +- frontend/src/i18n/locales/hu/downloads.json | 4 +- frontend/src/lib/downloadFailure.test.ts | 20 ++++++++- 11 files changed, 195 insertions(+), 34 deletions(-) diff --git a/backend/.basedpyright/baseline.json b/backend/.basedpyright/baseline.json index b8bd307c..cc5f74f1 100644 --- a/backend/.basedpyright/baseline.json +++ b/backend/.basedpyright/baseline.json @@ -437,14 +437,6 @@ "lineCount": 1 } }, - { - "code": "reportArgumentType", - "range": { - "startColumn": 12, - "endColumn": 13, - "lineCount": 10 - } - }, { "code": "reportAttributeAccessIssue", "range": { diff --git a/backend/app/downloads/errors.py b/backend/app/downloads/errors.py index 2289e647..0f50f62d 100644 --- a/backend/app/downloads/errors.py +++ b/backend/app/downloads/errors.py @@ -37,9 +37,9 @@ from app.downloads.credentials import host_key_for_url # Worker-authored strings that this module owns because each is coupled to `_RULES`. # -# QUOTA_FULL_MESSAGE, CLIP_TOO_BIG_MESSAGE and CONVERTED_TOO_BIG_MESSAGE are raised by the worker and -# must classify back to `quota_exceeded`; their rules below match a fragment of each, and -# test_download_errors pins each one, because a message and its rule drifting apart is exactly how a +# QUOTA_FULL_MESSAGE and the three *_TOO_BIG_MESSAGEs are raised by the worker and must classify +# back to `quota_exceeded`; their rules below match a fragment of each, and test_download_errors pins +# each one, because a message and its rule drifting apart is exactly how a # category silently becomes unreachable. # # NETWORK_REDACTED_MESSAGE REPLACES a message after classification (`user_error_text`). It is written @@ -54,8 +54,11 @@ from app.downloads.credentials import host_key_for_url # constants, imported above; each has a rule below that gives its category back, and a catalog entry # that gives the site's sentence. QUOTA_FULL_MESSAGE = "Your download storage is full — free up space, then try again." -# A cut clip, or an upload's converted file, held to the space left, which is not zero: "full" -# would be untrue. Both classify by the one phrase they share. +# A download, a cut clip, or an upload's converted file, held to the space left, which is not zero: +# "full" would be untrue. All three classify by the one phrase they share. +DOWNLOAD_TOO_BIG_MESSAGE = ( + "The video is larger than the download storage you have left — free up space, then try again." +) CLIP_TOO_BIG_MESSAGE = ( "The clip is larger than the download storage you have left — " "free up space or cut a shorter clip, then try again." diff --git a/backend/app/downloads/formats.py b/backend/app/downloads/formats.py index 1f56d1c7..0d957185 100644 --- a/backend/app/downloads/formats.py +++ b/backend/app/downloads/formats.py @@ -105,6 +105,28 @@ def log_safe(text: object) -> str: return _PATH_RE.sub("…", text) +# yt-dlp's HTTP downloader, refusing a file announced larger than `max_filesize`, logs this line and +# RETURNS — no exception and no file (measured; test_formats holds it against the pinned yt-dlp). +_OVER_CAP_LINE = "File is larger than max-filesize" +_OVER_CAP: contextvars.ContextVar[list[bool] | None] = contextvars.ContextVar( + "formats_over_cap", default=None +) + + +@contextlib.contextmanager +def watching_size_cap() -> Iterator[list[bool]]: + """While yt-dlp works inside it: a one-slot flag, set when it refused a file over the size cap + (`build_ydl_opts(max_bytes=…)`). It then returns as if it had downloaded, and all the caller + would find is no file — a processing fault, to anyone reading it, for what is the owner's + storage. A context variable, like `hiding`: yt-dlp's logger calls run on the extraction's thread.""" + flag = [False] + token = _OVER_CAP.set(flag) + try: + yield flag + finally: + _OVER_CAP.reset(token) + + class _YdlLog: """yt-dlp's logger protocol, pointed at ours. @@ -124,6 +146,8 @@ class _YdlLog: is the one line whose level should say so.""" def _emit(self, level: int, msg: str) -> None: + if _OVER_CAP_LINE in msg and (over := _OVER_CAP.get()) is not None: + over[0] = True # The line goes as an ARGUMENT, not as the format string. yt-dlp writes progress as # `[download] 53.2% of 12.00MiB`, and while stdlib logging only applies `%` formatting to # a record that has args, this way the question never arises — and it is the shape ruff's diff --git a/backend/app/downloads/pipeline.py b/backend/app/downloads/pipeline.py index d995e394..dee150a3 100644 --- a/backend/app/downloads/pipeline.py +++ b/backend/app/downloads/pipeline.py @@ -535,8 +535,8 @@ def _download( # free space, which is what the `quota_exceeded` category tells them. if remaining_bytes == 0: raise _OwnerRefused(dlerrors.QUOTA_FULL_MESSAGE) - info = _media_info( - _extract_with_fallback( + with formats.watching_size_cap() as over_cap: + fetched = _extract_with_fallback( job_id, asset_id, url, @@ -546,7 +546,11 @@ def _download( cookiefile=str(cookiefile) if cookiefile else None, max_bytes=remaining_bytes, ) - ) + info = _media_info(dict(fetched)) + # yt-dlp refused a file larger than what is left: it returns with no file, which would + # otherwise read as a processing fault. The owner's storage, so the owner's refusal. + if over_cap[0]: + raise _OwnerRefused(dlerrors.DOWNLOAD_TOO_BIG_MESSAGE) # Re-resolve now that yt-dlp's info reveals the true source (a bare/redirected url only # identifies post-extraction) so the .nfo uniqueid type + uploader_url agree on one source. adapter = adapters.resolve_adapter(source_kind, source_ref, info) @@ -959,17 +963,21 @@ def _clip_source( ) _set_job(job_id, phase="downloading", progress=0) try: - info = _extract_with_fallback( - job_id, - asset_id, - url, - formats.normalize(clipmod.YOUTUBE_FETCH_SPEC), - staging, - adapters.resolve_adapter("youtube", spec["ref"]), - cookiefile=str(cookiefile) if cookiefile else None, - max_bytes=remaining, - section=(spec["start_s"], spec["end_s"]), - ) + with formats.watching_size_cap() as over_cap: + info = _extract_with_fallback( + job_id, + asset_id, + url, + formats.normalize(clipmod.YOUTUBE_FETCH_SPEC), + staging, + adapters.resolve_adapter("youtube", spec["ref"]), + cookiefile=str(cookiefile) if cookiefile else None, + max_bytes=remaining, + section=(spec["start_s"], spec["end_s"]), + ) + # Should yt-dlp's own cap ever refuse the range (see `_download`): the owner's storage. + if over_cap[0]: + raise _OwnerRefused(dlerrors.CLIP_TOO_BIG_MESSAGE) except (_Aborted, _OwnerRefused): raise except Exception as exc: # re-raised below with the download catalog's diagnosis diff --git a/backend/tests/test_clip.py b/backend/tests/test_clip.py index 8e06c5c0..8566ca0d 100644 --- a/backend/tests/test_clip.py +++ b/backend/tests/test_clip.py @@ -15,7 +15,7 @@ from app import worker as workermod from app.config import settings from app.downloads import clip as clipmod from app.downloads import errors as dlerrors -from app.downloads import pipeline, quota, runner, service +from app.downloads import formats, pipeline, quota, runner, service from app.downloads import probe as probemod from app.downloads.edit import INPUT_GUARD from app.models import Channel, DownloadJob, MediaAsset, PlexItem, PlexLibrary, PlexShow, Video @@ -705,6 +705,30 @@ def test_a_clip_larger_than_what_its_owner_has_left_is_refused_and_not_placed( ) +def test_a_youtube_clip_range_yt_dlp_refuses_for_its_cap_says_it_is_too_big( + db, make_user, as_user, yt_video, worker, monkeypatch +): + # Should yt-dlp's own `max_filesize` refuse the range, it returns with no file (test_formats): + # the owner's storage, not a processing fault, and nothing is cut. + def refused(*_a, max_bytes=None, **_k): + formats._YDL_LOG.debug( + f"[download] File is larger than max-filesize (9000 bytes > {max_bytes} bytes). Aborting." + ) + return {"requested_downloads": [{}]} + + monkeypatch.setattr(pipeline, "_extract_with_fallback", refused) + job = _clip(as_user(make_user()), source="youtube", ref=yt_video).json() + monkeypatch.setattr(quota, "remaining_bytes", lambda _db, _uid: 500) + job, asset = worker["run"](job) + assert (job.status, job.error_code, job.error, asset.status) == ( + "error", + "quota_exceeded", + dlerrors.CLIP_TOO_BIG_MESSAGE, + "pending", + ) + assert "cmd" not in worker + + def test_a_plex_clip_of_a_silent_file_is_cut_silent(db, make_user, as_user, plex_episode, worker): # The player names the track it plays 0 even when the file has none. worker["probe"] = {"streams": [{"codec_type": "video"}]} diff --git a/backend/tests/test_download_errors.py b/backend/tests/test_download_errors.py index 9451d3df..92738261 100644 --- a/backend/tests/test_download_errors.py +++ b/backend/tests/test_download_errors.py @@ -65,6 +65,7 @@ def test_our_own_worker_refusals_classify_as_themselves(): assert classify(dlerrors.QUOTA_FULL_MESSAGE) == "quota_exceeded" assert classify(dlerrors.CLIP_TOO_BIG_MESSAGE) == "quota_exceeded" assert classify(dlerrors.CONVERTED_TOO_BIG_MESSAGE) == "quota_exceeded" + assert classify(dlerrors.DOWNLOAD_TOO_BIG_MESSAGE) == "quota_exceeded" def test_the_redacted_network_text_classifies_back_to_network(): diff --git a/backend/tests/test_download_limits.py b/backend/tests/test_download_limits.py index b1958c8f..19545904 100644 --- a/backend/tests/test_download_limits.py +++ b/backend/tests/test_download_limits.py @@ -11,7 +11,8 @@ its OWN session, so every test COMMITS its setup before calling it. """ from app.config import settings -from app.downloads import pipeline, quota +from app.downloads import errors as dlerrors +from app.downloads import formats, pipeline, quota from app.models import DownloadJob, DownloadQuota, MediaAsset, User from app.worker import _claim_job @@ -297,3 +298,49 @@ def test_a_refusal_for_one_user_does_not_poison_the_shared_asset(db, monkeypatch assert db.get(MediaAsset, shared.id).status == "pending" assert db.get(MediaAsset, shared.id).error is None assert db.get(DownloadJob, theirs.id).status == "queued" + + +def test_a_download_over_the_space_left_says_so_and_releases_the_shared_asset( + db, monkeypatch, tmp_path +): + # yt-dlp refuses a file over `max_filesize` by saying so and returning with no file (pinned in + # test_formats). That is the owner's storage, not a processing fault: their refusal, in words + # true of a cap with space still left, and the shared asset goes back for the next job. + monkeypatch.setattr(settings, "download_root", str(tmp_path)) + u = User(email="nearcap@example.com") + db.add(u) + db.flush() + db.add( + DownloadQuota(user_id=u.id, max_bytes=1000, max_concurrent=2, max_jobs=100, unlimited=False) + ) + held = _sized_asset(db, 400, "near-held") + _job(db, u.id, queue_pos=0, status="done", asset_id=held.id) + shared = MediaAsset( + source_kind="youtube", + source_ref="y" * 11, + format_sig="s", + status="downloading", + ref_count=1, + ) + db.add(shared) + db.flush() + mine = _job(db, u.id, queue_pos=1, status="running", asset_id=shared.id) + db.commit() + caps = [] + + def refuses_over_the_cap(*args, max_bytes=None, **kwargs): + caps.append(max_bytes) + formats._YDL_LOG.debug( + f"[download] File is larger than max-filesize (2000 bytes > {max_bytes} bytes). Aborting." + ) + return {"id": "y" * 11, "requested_downloads": [{}]} + + monkeypatch.setattr(pipeline, "_extract_with_fallback", refuses_over_the_cap) + pipeline._download(mine.id, shared.id, "youtube", "y" * 11, {}, u.id) + + db.expire_all() + job = db.get(DownloadJob, mine.id) + assert caps == [600] + assert (job.status, job.error_code) == ("error", "quota_exceeded") + assert job.error == dlerrors.DOWNLOAD_TOO_BIG_MESSAGE + assert db.get(MediaAsset, shared.id).status == "pending" diff --git a/backend/tests/test_formats.py b/backend/tests/test_formats.py index ab461d42..a0369c5d 100644 --- a/backend/tests/test_formats.py +++ b/backend/tests/test_formats.py @@ -3,6 +3,13 @@ playability guarantee) and must SELECT H.264 (not merely prefer it), so shared d every browser incl. iOS/Safari. Non-mp4 containers and non-H.264 vcodecs are coerced, not honoured. See the iPad AV1 broken-player fix + the format narrowing.""" +import contextlib +import functools +import http.server +import threading + +import yt_dlp + from app.downloads import edit as editmod from app.downloads import formats @@ -162,3 +169,40 @@ def test_the_mp3_fix_moved_the_audio_cache_identity(monkeypatch): current = formats.format_sig(audio) monkeypatch.setattr(formats, "_AUDIO_SIG_VERSION", 4) assert formats.format_sig(audio) != current + + +class _QuietFiles(http.server.SimpleHTTPRequestHandler): + def log_message(self, *args): # the test's output is yt-dlp's, not the server's + pass + + def handle(self): + # yt-dlp hangs up on a file over the cap as soon as it reads its size. + with contextlib.suppress(ConnectionError): + super().handle() + + +def test_yt_dlp_refuses_a_file_over_the_cap_without_raising_and_the_watch_sees_it(tmp_path): + # The boundary `watching_size_cap` reads, held against the yt-dlp this image pins: over + # `max_filesize` its HTTP downloader logs one line and RETURNS — no exception, no file — so + # without the watch the pipeline would only find "no output file" and call it a processing fault. + served = tmp_path / "served" + served.mkdir() + (served / "v.mp4").write_bytes(b"\0" * 200_000) + srv = http.server.ThreadingHTTPServer( + ("127.0.0.1", 0), functools.partial(_QuietFiles, directory=str(served)) + ) + threading.Thread(target=srv.serve_forever, daemon=True).start() + url = f"http://127.0.0.1:{srv.server_port}/v.mp4" + try: + for cap, refused in ((1_000, True), (10_000_000, False)): + out = tmp_path / f"out-{cap}" + opts = formats.build_ydl_opts( + {}, str(out / "%(id)s.%(ext)s"), lambda d: None, max_bytes=cap + ) + with formats.watching_size_cap() as over, yt_dlp.YoutubeDL(opts) as ydl: + ydl.extract_info(url, download=True) + assert over[0] is refused, cap + files = [p for p in out.glob("*") if p.suffix == ".mp4"] if out.exists() else [] + assert bool(files) is not refused, cap + finally: + srv.shutdown() diff --git a/frontend/src/i18n/locales/en/downloads.json b/frontend/src/i18n/locales/en/downloads.json index 1a63481d..c6460e62 100644 --- a/frontend/src/i18n/locales/en/downloads.json +++ b/frontend/src/i18n/locales/en/downloads.json @@ -307,8 +307,8 @@ "body": "Something went wrong with this download. The technical details below may help." }, "quota_exceeded": { - "short": "Storage full", - "body": "This download was stopped because your storage quota is already used up. Delete something from your library, then try again." + "short": "Not enough storage", + "body": "This download doesn't fit in the storage you have left. Delete something from your library, then try again." } }, "catalog": { diff --git a/frontend/src/i18n/locales/hu/downloads.json b/frontend/src/i18n/locales/hu/downloads.json index be4b3e8b..668a41cc 100644 --- a/frontend/src/i18n/locales/hu/downloads.json +++ b/frontend/src/i18n/locales/hu/downloads.json @@ -307,8 +307,8 @@ "body": "Valami hiba történt ennél a letöltésnél. Az alábbi technikai részletek segíthetnek." }, "quota_exceeded": { - "short": "Betelt a tárhely", - "body": "Ez a letöltés leállt, mert a tárhelykereted már elfogyott. Törölj valamit a könyvtáradból, aztán próbáld újra." + "short": "Nincs elég tárhely", + "body": "Ez a letöltés nem fér bele a megmaradt tárhelyedbe. Törölj valamit a könyvtáradból, aztán próbáld újra." } }, "catalog": { diff --git a/frontend/src/lib/downloadFailure.test.ts b/frontend/src/lib/downloadFailure.test.ts index 78ffba27..62a41244 100644 --- a/frontend/src/lib/downloadFailure.test.ts +++ b/frontend/src/lib/downloadFailure.test.ts @@ -81,13 +81,31 @@ describe("failTitleKey", () => { expect(i18n.t(failCopyKeys(download, "body"))).toBe( i18n.t("downloads.errors.fail.cat.quota_exceeded.body"), ); - // Its headline too: space is left, so not "Storage full". + // Its headline too, worded for the converted file rather than the download it is not. const short = i18n.t(failCopyKeys(upload, "short")); expect(short).not.toMatch(/^downloads\./); expect(short).not.toBe(i18n.t("downloads.errors.fail.cat.quota_exceeded.short")); } }); + it("a download refused for storage says it does not fit, true with space left or none", async () => { + // Refused at a full cap before it starts, or by yt-dlp for a file larger than what is left: one + // sentence true of both, so never "full" or "used up". + const download = { + error_code: "quota_exceeded", + error_detail: null, + job_kind: "download", + } as const; + for (const lng of ["en", "hu"]) { + await i18n.changeLanguage(lng); + for (const part of ["short", "body"] as const) { + const text = i18n.t(failCopyKeys(download, part)); + expect(text).not.toMatch(/^downloads\./); + expect(text).not.toMatch(/full|used up|betelt|elfogyott/i); + } + } + }); + it("a clip refused for storage says so of a clip, and not that the quota is used up", async () => { // Refused before it is cut (nothing left) or after (the cut is bigger than what is left): one // sentence true of both, worded for a clip and naming the clip-side way out.