diff --git a/backend/alembic/versions/0084_dl_job_error_detail.py b/backend/alembic/versions/0084_dl_job_error_detail.py new file mode 100644 index 00000000..148aabff --- /dev/null +++ b/backend/alembic/versions/0084_dl_job_error_detail.py @@ -0,0 +1,41 @@ +"""add download_jobs.error_detail — the domain-specific entry of the failure catalog (E11.5 S1) + +Revision ID: 0084_dl_job_error_detail +Revises: 0083_comfort_pins +Create Date: 2026-09-13 + +`error_code` is the generic category (it decides whether a Retry is offered). `error_detail` names +the host-specific catalog entry `downloads.errors.classify` matched on top of it — `facebook.login_wall` +and its siblings — so the UI can say what went wrong ON THAT SITE and what to do about it, instead of +the category's generic sentence. Nullable: most failures have no host-specific entry. + +Backfill: a job's code is written once, at failure time, so a new rule never reaches the rows that +failed before it. The one shape measured on prod (2026-09-13, jobs 9/66/75/145/156/157/182/256/257) is +yt-dlp's `[facebook] : Cannot parse data`, stored as `unknown`. Without a cookie that is Facebook's +login wall. Whether a cookie was attached at the time is not recorded anywhere, so every old row gets +the no-cookie entry — on prod no account had a Facebook cookie, so that is what they all were. +""" + +import sqlalchemy as sa +from alembic import op + +revision: str = "0084_dl_job_error_detail" +down_revision: str | None = "0083_comfort_pins" +branch_labels: str | None = None +depends_on: str | None = None + + +def upgrade() -> None: + op.add_column("download_jobs", sa.Column("error_detail", sa.String(length=48), nullable=True)) + op.execute( + sa.text( + "UPDATE download_jobs SET error_code = 'login_required', " + "error_detail = 'facebook.login_wall' " + "WHERE (error_code IS NULL OR error_code = 'unknown') " + "AND error ILIKE '%[facebook]%cannot parse data%'" + ) + ) + + +def downgrade() -> None: + op.drop_column("download_jobs", "error_detail") diff --git a/backend/app/downloads/credentials.py b/backend/app/downloads/credentials.py index af5b79fe..5594143c 100644 --- a/backend/app/downloads/credentials.py +++ b/backend/app/downloads/credentials.py @@ -104,6 +104,14 @@ def authed_format_sig(spec: dict, user_id: int, host_key: str) -> str: return formats.format_sig(spec, salt=cookie_salt(user_id, host_key)) +def is_authed_asset(asset_format_sig: str, spec: dict, user_id: int, url: str | None) -> bool: + """Whether an asset is `user_id`'s private, cookie-salted identity for `url`'s host — the one + kind of asset a cookie is ever attached to. The worker's cookie gate asks it before a download, + and the failure diagnosis asks it after one (a failure on this asset happened WITH the cookie).""" + host_key = host_key_for_url(url) + return host_key is not None and asset_format_sig == authed_format_sig(spec, user_id, host_key) + + def salt_for_enqueue(db: Session, user_id: int, url: str | None) -> str | None: """At enqueue: the cache salt to apply if this user has a cookie for `url`'s host, else None (a normal shared download). Returns the salt string so the caller passes it straight to @@ -224,7 +232,7 @@ def resolve_cookiefile( cred = get_credential(db, user_id, host_key) if cred is None: return None - if asset_format_sig != authed_format_sig(spec, user_id, host_key): + if not is_authed_asset(asset_format_sig, spec, user_id, url): return None # shared asset — never attach a cookie to a non-private identity raw = decrypt_optional(cred.cookies_enc) if not raw: diff --git a/backend/app/downloads/errors.py b/backend/app/downloads/errors.py index 1d4a7ccb..0d79b480 100644 --- a/backend/app/downloads/errors.py +++ b/backend/app/downloads/errors.py @@ -1,4 +1,5 @@ -"""Classify a raw yt-dlp / ffmpeg failure string into a stable category code. +"""Classify a raw yt-dlp / ffmpeg failure string into a stable category code — and, where a site's +failure has been measured, into that site's entry of the failure catalog. The worker stores the raw exception text (truncated) for technical detail, but a raw `ERROR: [youtube] : Private video. Sign in…` is noise to a user. `classify()` maps that text @@ -6,17 +7,36 @@ to one of a small set of category codes; the frontend turns the code into a loca explanation (and decides whether a retry makes sense). The raw text stays available behind a "details" disclosure. This is the same string-matching approach the worker already trusts for `_RETRIABLE_MARKERS` — kept here as the single source of truth for "what kind of failure is this". + +**The failure catalog** (`_CATALOG`, E11.5). A category is generic: "Sign-in required" cannot tell a +Facebook user that the cookie they already gave was refused, and yt-dlp words several different +Facebook walls identically (`Cannot parse data`). A catalog entry is scoped to ONE host and may also +depend on whether a cookie was attached and on the kind of link, and names a `detail` code the UI has +its own sentence for — why it failed on that site, and what the user can do. `diagnose()` tries the +catalog first and falls back to the category. How an entry is added, every time a failure is reported: + 1. reproduce it on dev (with and without the user's cookie) and keep the verbatim yt-dlp text; + 2. add the entry below, scoped as narrowly as the measurement allows (a marker that is yt-dlp's + generic wording for every site must never match without its host); + 3. pin the real text in `tests/test_download_errors.py`; + 4. add `downloads.errors.fail.catalog..{short,body}` in EN and HU (a contract test checks it; + the group is not called `detail`, which `actionErrors.contract` reads as a raw `HttpError.detail`). """ +from dataclasses import dataclass +from typing import NamedTuple +from urllib.parse import urlparse + +from app.downloads.credentials import host_key_for_url + # Two worker-authored strings that this module owns because both are coupled to `_RULES`. # # QUOTA_FULL_MESSAGE is raised by the worker and must classify back to `quota_exceeded`; its rule # below matches a fragment of it, and test_download_errors pins the pair, because a message and its # rule drifting apart is exactly how a category silently becomes unreachable. # -# NETWORK_REDACTED_MESSAGE is the opposite direction: it REPLACES a message after classification, so -# it deliberately has no rule. Nothing re-classifies a stored `job.error` today; if anything ever -# does, it must carry the original code rather than re-deriving one from this text. +# NETWORK_REDACTED_MESSAGE REPLACES a message after classification (`user_error_text`). It is written +# to the shared asset too, and a second job that inherits that failed asset re-diagnoses the stored +# text (`worker._process_job`), so it has a rule of its own that gives the category back. # # Other worker-raised texts are still inline literals at their raise sites. The ffmpeg ones and # "produced no output file" have markers here; the urlguard refusal deliberately has none and @@ -30,6 +50,8 @@ _RULES: tuple[tuple[str, str], ...] = ( # Our own refusal: the worker found the user already at their storage cap (SEC-13). Placed # first — it is our wording, so it can never be a false match on an upstream message. ("download storage is full", "quota_exceeded"), + # Our own redaction of a transport failure (see NETWORK_REDACTED_MESSAGE) — our wording too. + (NETWORK_REDACTED_MESSAGE.lower(), "network"), # Login / age / members / bot-check — the user must be signed in (we deliberately are not). ("sign in to confirm your age", "login_required"), ("confirm your age", "login_required"), @@ -101,6 +123,62 @@ _RULES: tuple[tuple[str, str], ...] = ( ) +class Diagnosis(NamedTuple): + """What a failure was: the generic `code` (category) and, when a catalog entry matched, its + host-specific `detail`.""" + + code: str + detail: str | None = None + + +@dataclass(frozen=True) +class _Entry: + """One measured failure of one site. It matches when the download URL belongs to `host` + (`credentials.SUPPORTED_HOSTS`), every one of `markers` is in the message, the cookie state is + `cookie` (None: either), and — when `link` is given — one of those fragments is in the URL path.""" + + host: str + markers: tuple[str, ...] + code: str + detail: str + cookie: bool | None = None + link: tuple[str, ...] = () + + +_CATALOG: tuple[_Entry, ...] = ( + # Facebook answers a signed-out client with a page yt-dlp cannot parse — an 18+ reel, a closed + # group's post (measured 2026-09-13: prod jobs 145, 156, 256 download once the owner's cookie is + # attached). "Cannot parse data" is yt-dlp's generic wording for a broken extractor on EVERY site, + # which is why the entry is scoped to Facebook and never becomes a general rule. + _Entry( + host="facebook", + markers=("[facebook]", "cannot parse data"), + code="login_required", + detail="facebook.login_wall", + cookie=False, + ), + # The same text WITH a cookie: an expired / signed-out cookie, or a page that also needs + # impersonation (prod job 257). Telling this user to add cookies would be wrong — they have. + _Entry( + host="facebook", + markers=("[facebook]", "cannot parse data"), + code="login_required", + detail="facebook.cookie_rejected", + cookie=True, + ), + # A group post whose video yt-dlp does not reach from the post's own link, although the video's + # own link downloads (prod job 66, with the cookie). Only for group links: a reel with no formats + # is a different failure. + _Entry( + host="facebook", + markers=("[facebook]", "no video formats found"), + code="format_unavailable", + detail="facebook.group_post_video", + link=("/groups/",), + ), +) + + def classify(msg: str | None) -> str: """Return a category code for a raw failure message (``"unknown"`` if nothing matches).""" if not msg: @@ -112,6 +190,28 @@ def classify(msg: str | None) -> str: return "unknown" +def diagnose( + msg: str | None, *, url: str | None = None, cookie_attached: bool = False +) -> Diagnosis: + """The catalog entry for this failure of this link, else just its category (see the module doc). + + `url` is the address yt-dlp was given; `cookie_attached` whether the acting user's cookie went + with it. Without a URL only the category can be known.""" + host = host_key_for_url(url) if msg and url else None + if host is not None: + low = msg.lower() # type: ignore[union-attr] — `host` is only set when `msg` is truthy + path = urlparse(url).path.lower() + for e in _CATALOG: + if ( + e.host == host + and all(m in low for m in e.markers) + and (e.cookie is None or e.cookie == cookie_attached) + and (not e.link or any(fragment in path for fragment in e.link)) + ): + return Diagnosis(e.code, e.detail) + return Diagnosis(classify(msg)) + + def user_error_text(source_kind: str, msg: str, code: str) -> str: """The failure text to STORE on the job, given the raw message and its category. diff --git a/backend/app/downloads/pipeline.py b/backend/app/downloads/pipeline.py index d1293eb6..a29dcc66 100644 --- a/backend/app/downloads/pipeline.py +++ b/backend/app/downloads/pipeline.py @@ -425,6 +425,7 @@ def _download( ) -> None: staging = _staging_dir(asset_id) cookiefile: Path | None = None + url: str | None = None # read by the failure branch, which can run before it is resolved try: if staging.exists(): shutil.rmtree(staging, ignore_errors=True) @@ -578,6 +579,7 @@ def _download( status=JobStatus.ERROR, error=msg, error_code=dlerrors.classify(msg), + error_detail=None, phase=None, ) db.commit() @@ -588,9 +590,11 @@ def _download( log.info("job %s refused for its owner: %s", job_id, msg) except Exception as exc: # noqa: BLE001 — surface any failure to the user, keep worker alive msg = str(exc)[:500] - code = dlerrors.classify(msg) + # The catalog needs the link and whether the user's cookie went with it: the same yt-dlp + # text is Facebook's login wall without one and a refused cookie with one. + diagnosis = dlerrors.diagnose(msg, url=url, cookie_attached=cookiefile is not None) # SEC-12: redact a transport-level readout for user-supplied URLs (see user_error_text). - msg = dlerrors.user_error_text(source_kind, msg, code) + msg = dlerrors.user_error_text(source_kind, msg, diagnosis.code) with SessionLocal() as db: asset = db.get(MediaAsset, asset_id) if asset: @@ -603,7 +607,8 @@ def _download( job_id, status=JobStatus.ERROR, error=msg, - error_code=code, + error_code=diagnosis.code, + error_detail=diagnosis.detail, phase=None, ) db.commit() diff --git a/backend/app/downloads/service.py b/backend/app/downloads/service.py index 5eaa81b7..8f1aa971 100644 --- a/backend/app/downloads/service.py +++ b/backend/app/downloads/service.py @@ -346,6 +346,7 @@ def resume(db: Session, job: DownloadJob) -> None: job.progress = 0 job.error = None job.error_code = None + job.error_detail = None if job.asset_id: asset = db.get(MediaAsset, job.asset_id) if asset and asset.status == "error": # MediaAsset.status (no enum), not a JobStatus @@ -389,6 +390,8 @@ def redownload(db: Session, job: DownloadJob) -> None: job.progress = 0 job.phase = None job.error = None + job.error_code = None # a rebuild that fails again gets its own diagnosis, never the old one + job.error_detail = None job.speed_bps = None job.eta_s = None # Re-derive the display name from the freshly cleaned title on completion (else an older, un-cleaned diff --git a/backend/app/models.py b/backend/app/models.py index 6ea02966..f29695d9 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -871,6 +871,8 @@ class DownloadJob(Base, TimestampMixin, UpdatedAtMixin): error_code: Mapped[str | None] = mapped_column( String(24) ) # category, see downloads.errors.classify + # The host-specific failure-catalog entry on top of the category (`facebook.login_wall`), or NULL. + error_detail: Mapped[str | None] = mapped_column(String(48)) # see downloads.errors.diagnose queue_pos: Mapped[int] = mapped_column(Integer, default=0, server_default="0") diff --git a/backend/app/routes/downloads.py b/backend/app/routes/downloads.py index 01a56f56..3ef241e3 100644 --- a/backend/app/routes/downloads.py +++ b/backend/app/routes/downloads.py @@ -107,6 +107,7 @@ def _serialize(job: DownloadJob, asset: MediaAsset | None) -> dict: "phase": job.phase, "error": job.error, "error_code": job.error_code, + "error_detail": job.error_detail, "queue_pos": job.queue_pos, "created_at": iso(job.created_at), "source_kind": job.source_kind, diff --git a/backend/app/worker.py b/backend/app/worker.py index cb617259..ebc0c1d1 100644 --- a/backend/app/worker.py +++ b/backend/app/worker.py @@ -32,7 +32,8 @@ from app import sysconfig from app.config import settings from app.db import SessionLocal from app.downloads import edit as editmod -from app.downloads import quota, service +from app.downloads import credentials, formats, quota, service +from app.downloads import errors as dlerrors from app.downloads.jobdb import ( _edit_staging_dir, _requeue_if_running, @@ -399,6 +400,19 @@ def _process_job(job_id: int) -> None: asset_id = asset.id if asset else None asset_status = asset.status if asset else None asset_error = asset.error if asset else None + if asset_status == "error": + # A job that meets an already-failed asset gets the SAME explanation the job that failed + # it got — not a bare "Download failed". Only the text is on the asset, so it is + # diagnosed again, for this link, with the cookie state of that asset: a user's private + # (cookie-salted) asset failed with their cookie attached, the shared one without. + url = service.source_url(job.source_kind, job.source_ref) + diagnosis = dlerrors.diagnose( + asset_error, + url=url, + cookie_attached=credentials.is_authed_asset( + asset.format_sig, formats.normalize(job.profile_snapshot), job.user_id, url + ), + ) if asset is None: _set_job(job_id, status="error", error="No media asset", phase=None) @@ -408,7 +422,14 @@ def _process_job(job_id: int) -> None: _finish_from_cache(job_id, asset_id) return if asset_status == "error": - _set_job(job_id, status="error", error=asset_error or "Download failed", phase=None) + _set_job( + job_id, + status="error", + error=asset_error or "Download failed", + error_code=diagnosis.code, + error_detail=diagnosis.detail, + phase=None, + ) return if asset_status == "downloading": # Another worker won this asset between our claim and here (a narrow race — the claim SQL diff --git a/backend/tests/test_download_errors.py b/backend/tests/test_download_errors.py index 680d2d13..b467dee6 100644 --- a/backend/tests/test_download_errors.py +++ b/backend/tests/test_download_errors.py @@ -64,6 +64,71 @@ def test_our_own_worker_refusals_classify_as_themselves(): assert classify(dlerrors.QUOTA_FULL_MESSAGE) == "quota_exceeded" +def test_the_redacted_network_text_classifies_back_to_network(): + # The redacted text is also what a failed asset keeps, and a second job that meets that asset is + # diagnosed from it — without its own rule it would read "Download failed". + assert classify(dlerrors.NETWORK_REDACTED_MESSAGE) == "network" + + +# Verbatim yt-dlp 2026.07.04 output from prod jobs, measured on dev 2026-09-13 (E11.5). +_FB_PARSE = ( + "ERROR: [facebook] 1736360027626346: Cannot parse data; please report this issue on " + "https://github.com/yt-dlp/yt-dlp/issues?q= , filling out the appropriate issue template." +) +_FB_NO_FORMATS = ( + "ERROR: [facebook] 1755988225525547: No video formats found!; please report this issue on " + "https://github.com/yt-dlp/yt-dlp/issues?q= , filling out the appropriate issue template." +) +_REEL = "https://www.facebook.com/reel/1736360027626346/?fs=e&fs=e" +_GROUP_POST = "https://www.facebook.com/groups/983554479435596/permalink/1755988225525547/?" + + +def test_facebook_cannot_parse_data_is_the_login_wall_without_a_cookie(): + for url in ( + _REEL, + _GROUP_POST, + "https://fb.watch/Ia_iJYI2Vw/?fs=e", + "https://m.facebook.com/x", + ): + assert dlerrors.diagnose(_FB_PARSE, url=url) == ("login_required", "facebook.login_wall") + + +def test_facebook_cannot_parse_data_with_a_cookie_is_the_cookie_refused(): + # Telling a user who already gave a cookie to add one is the failure this entry exists to avoid. + assert dlerrors.diagnose(_FB_PARSE, url=_REEL, cookie_attached=True) == ( + "login_required", + "facebook.cookie_rejected", + ) + + +def test_a_facebook_group_post_without_formats_names_the_link(): + assert dlerrors.diagnose(_FB_NO_FORMATS, url=_GROUP_POST, cookie_attached=True) == ( + "format_unavailable", + "facebook.group_post_video", + ) + # A reel with no formats is another failure: only the category. + assert dlerrors.diagnose(_FB_NO_FORMATS, url=_REEL, cookie_attached=True) == ( + "format_unavailable", + None, + ) + + +def test_a_catalog_entry_never_leaves_its_host(): + # "Cannot parse data" is yt-dlp's wording for a broken extractor on every site. + other = _FB_PARSE.replace("[facebook]", "[vimeo]") + assert dlerrors.diagnose(other, url="https://vimeo.com/1") == ("unknown", None) + # The Facebook text for a link that is not Facebook's (a look-alike host) is not the Facebook wall. + assert dlerrors.diagnose(_FB_PARSE, url="https://facebook.com.evil/reel/1") == ("unknown", None) + # No link known: the category only. + assert dlerrors.diagnose(_FB_PARSE) == ("unknown", None) + + +def test_diagnose_falls_back_to_the_category(): + msg = "ERROR: [facebook] 123: You must be logged in to see this post" + assert dlerrors.diagnose(msg, url=_REEL) == ("login_required", None) + assert dlerrors.diagnose(None, url=_REEL) == ("unknown", None) + + def test_a_url_jobs_network_failure_is_redacted_but_youtube_keeps_its_text(): # SEC-12: the raw transport text for a user-chosen host:port is a port-scanner readout, so it is # replaced for `url` jobs. Everything else must survive verbatim, or a real diagnosis is lost. diff --git a/backend/tests/test_download_failure_catalog.py b/backend/tests/test_download_failure_catalog.py new file mode 100644 index 00000000..9ae95199 --- /dev/null +++ b/backend/tests/test_download_failure_catalog.py @@ -0,0 +1,206 @@ +"""DB-backed tests for the download failure catalog (E11.5 S1) at the places a failure is WRITTEN. + +`test_download_errors` pins which entry a message matches; these pin that every writer hands the +matcher what it needs — the link and whether the user's cookie went with it — and that a stored entry +never outlives the failure it describes: +- the download child's failure branch (`pipeline._download`); +- a second job meeting an already-failed asset (`worker._process_job`), which only has the text; +- Retry / Re-download, which requeue the job; +- the `0084` backfill of the rows that failed before the catalog existed. +""" + +from alembic import command +from alembic.config import Config +from sqlalchemy import text + +from app.config import settings +from app.db import engine +from app.downloads import credentials, formats, pipeline, service +from app.models import DownloadJob, DownloadQuota, MediaAsset, User +from app.worker import _process_job + +_REEL = "https://www.facebook.com/reel/1736360027626346/?fs=e&fs=e" +# Verbatim yt-dlp 2026.07.04, prod job 256. +_FB_PARSE = ( + "ERROR: [facebook] 1736360027626346: Cannot parse data; please report this issue on " + "https://github.com/yt-dlp/yt-dlp/issues?q= , filling out the appropriate issue template." +) + + +def _user(db, email): + u = User(email=email) + db.add(u) + db.flush() + db.add( + DownloadQuota( + user_id=u.id, max_bytes=10**12, max_concurrent=5, max_jobs=10**6, unlimited=True + ) + ) + return u + + +def _asset_and_job(db, user, *, sig, asset_status, job_status, error=None): + a = MediaAsset( + source_kind="url", + source_ref=_REEL, + format_sig=sig, + status=asset_status, + error=error, + ref_count=1, + ) + db.add(a) + db.flush() + j = DownloadJob( + user_id=user.id, + asset_id=a.id, + source_kind="url", + source_ref=_REEL, + status=job_status, + queue_pos=0, + profile_snapshot={}, + ) + db.add(j) + db.commit() + return a, j + + +def _fail_the_child(monkeypatch, tmp_path, *, cookie: bool): + monkeypatch.setattr(settings, "download_root", str(tmp_path)) + monkeypatch.setattr(pipeline.urlguard, "check_url", lambda url: None) + cookiefile = tmp_path / "cookies.txt" + cookiefile.write_text("# Netscape HTTP Cookie File\n") + monkeypatch.setattr( + pipeline.credentials, + "resolve_cookiefile", + lambda *a, **k: cookiefile if cookie else None, + ) + + def extract(*a, **k): + raise RuntimeError(_FB_PARSE) + + monkeypatch.setattr(pipeline, "_extract_with_fallback", extract) + + +def test_the_download_child_diagnoses_with_the_link_and_no_cookie(db, monkeypatch, tmp_path): + u = _user(db, "fb-nocookie@example.com") + a, j = _asset_and_job(db, u, sig="shared", asset_status="downloading", job_status="running") + _fail_the_child(monkeypatch, tmp_path, cookie=False) + + pipeline._download(j.id, a.id, "url", _REEL, {}, u.id) + + db.expire_all() + job = db.get(DownloadJob, j.id) + assert (job.status, job.error_code, job.error_detail) == ( + "error", + "login_required", + "facebook.login_wall", + ) + + +def test_the_download_child_knows_the_cookie_went_with_it(db, monkeypatch, tmp_path): + # The same text with the user's cookie attached: telling them to add cookies would be wrong. + u = _user(db, "fb-cookie@example.com") + a, j = _asset_and_job(db, u, sig="private", asset_status="downloading", job_status="running") + _fail_the_child(monkeypatch, tmp_path, cookie=True) + + pipeline._download(j.id, a.id, "url", _REEL, {}, u.id) + + db.expire_all() + job = db.get(DownloadJob, j.id) + assert (job.error_code, job.error_detail) == ("login_required", "facebook.cookie_rejected") + + +def test_a_job_meeting_a_failed_shared_asset_gets_the_same_explanation(db): + # Before the catalog this path wrote the text and NO code: "Download failed" for the second + # job, "Facebook sign-in required" for the first. + u = _user(db, "fb-second@example.com") + _, j = _asset_and_job( + db, u, sig="shared", asset_status="error", job_status="running", error=_FB_PARSE + ) + + _process_job(j.id) + + db.expire_all() + job = db.get(DownloadJob, j.id) + assert (job.status, job.error_code, job.error_detail) == ( + "error", + "login_required", + "facebook.login_wall", + ) + + +def test_a_job_meeting_its_users_failed_private_asset_knows_the_cookie_was_used(db): + u = _user(db, "fb-private@example.com") + sig = credentials.authed_format_sig(formats.normalize({}), u.id, "facebook") + _, j = _asset_and_job( + db, u, sig=sig, asset_status="error", job_status="running", error=_FB_PARSE + ) + + _process_job(j.id) + + db.expire_all() + job = db.get(DownloadJob, j.id) + assert (job.error_code, job.error_detail) == ("login_required", "facebook.cookie_rejected") + + +def test_retry_and_redownload_drop_the_old_explanation(db): + u = _user(db, "fb-requeue@example.com") + for requeue in (service.resume, service.redownload): + _, j = _asset_and_job( + db, u, sig=f"s-{requeue.__name__}", asset_status="error", job_status="error" + ) + j.error, j.error_code, j.error_detail = _FB_PARSE, "login_required", "facebook.login_wall" + db.commit() + + requeue(db, j) + + db.refresh(j) + assert (j.status, j.error_code, j.error_detail) == ("queued", None, None), requeue.__name__ + + +def test_0084_backfills_only_the_facebook_parse_failures_it_measured(db): + cfg = Config("alembic.ini") + command.downgrade(cfg, "0083_comfort_pins") + upgraded = False + try: + with engine.begin() as conn: + uid = conn.execute( + text("INSERT INTO users (email) VALUES ('backfill@example.com') RETURNING id") + ).scalar_one() + rows = { + "fb_unknown": ("error", "unknown", _FB_PARSE), + "fb_null": ("canceled", None, _FB_PARSE), + "other_site": ("error", "unknown", _FB_PARSE.replace("[facebook]", "[vimeo]")), + "fb_other_code": ("error", "network", _FB_PARSE), + } + ids = {} + for name, (status, code, error) in rows.items(): + ids[name] = conn.execute( + text( + "INSERT INTO download_jobs (user_id, source_kind, source_ref, " + "profile_snapshot, status, error_code, error) " + "VALUES (:u, 'url', :ref, '{}', :s, :c, :e) RETURNING id" + ), + {"u": uid, "ref": _REEL, "s": status, "c": code, "e": error}, + ).scalar_one() + command.upgrade(cfg, "head") + upgraded = True + with engine.connect() as conn: + got = { + name: tuple( + conn.execute( + text("SELECT error_code, error_detail FROM download_jobs WHERE id = :i"), + {"i": i}, + ).one() + ) + for name, i in ids.items() + } + assert got == { + "fb_unknown": ("login_required", "facebook.login_wall"), + "fb_null": ("login_required", "facebook.login_wall"), + "other_site": ("unknown", None), + "fb_other_code": ("network", None), + } + finally: + if not upgraded: + command.upgrade(cfg, "head") diff --git a/frontend/src/components/LibraryPlayer.dom.test.tsx b/frontend/src/components/LibraryPlayer.dom.test.tsx index aaa05ede..38e119b6 100644 --- a/frontend/src/components/LibraryPlayer.dom.test.tsx +++ b/frontend/src/components/LibraryPlayer.dom.test.tsx @@ -24,6 +24,7 @@ const baseJob: DownloadJob = { phase: null, error: null, error_code: null, + error_detail: null, queue_pos: 0, created_at: null, source_kind: "youtube", diff --git a/frontend/src/components/downloadModals.tsx b/frontend/src/components/downloadModals.tsx index 7c54a069..0e618af8 100644 --- a/frontend/src/components/downloadModals.tsx +++ b/frontend/src/components/downloadModals.tsx @@ -8,6 +8,7 @@ import { api, type DownloadJob } from "../lib/api"; import { inputCls } from "./ui/form"; import { Button } from "./ui/Button"; import { IconBtn } from "./downloadRow"; +import { failCopyKeys } from "../lib/downloadFailure"; // Per-item user modals split out of DownloadCenter (self-contained, prop-driven): rename/metadata // editing and the failure-detail window. @@ -142,8 +143,8 @@ export function ErrorModal({ return (
-
{t(`downloads.errors.fail.cat.${code}.short`)}
-

{t(`downloads.errors.fail.cat.${code}.body`)}

+
{t(failCopyKeys(job, "short"))}
+

{t(failCopyKeys(job, "body"))}

{job.error && (
) : null} diff --git a/frontend/src/i18n/backendIds.contract.test.ts b/frontend/src/i18n/backendIds.contract.test.ts index 68d1ddef..3b2eb699 100644 --- a/frontend/src/i18n/backendIds.contract.test.ts +++ b/frontend/src/i18n/backendIds.contract.test.ts @@ -67,6 +67,13 @@ const adminRefusals = [ ...backend("routes/admin.py").matchAll(/\{\s*"code":\s*"(\w+)",\s*"reason":\s*"(\w+)"/g), ].filter((m) => adminCodes.includes(m[1]!)); const adminReasons = [...new Set(adminRefusals.map((m) => m[2]!))]; +// The download failure catalog's host-specific entries → `downloads.errors.fail.catalog.`. +// A code holds a dot (`facebook.login_wall`), so the reverse check strips the prefix and the part +// rather than splitting on dots. +const failDetails = [ + ...new Set([...backend("downloads/errors.py").matchAll(/detail="([\w.]+)"/g)].map((m) => m[1]!)), +]; +const DETAIL = "downloads.errors.fail.catalog."; /** The ids under `prefix` a locale names, `prefix.` or `prefix..`. */ const idsUnder = (keys: Set, prefix: string) => @@ -83,6 +90,7 @@ describe("backend ids have their names in both languages", () => { expect(auditActions).toEqual(expect.arrayContaining(["config_set", "audit_clear"])); expect(quotaActions.length).toBeGreaterThan(0); expect(adminCodes).toEqual(expect.arrayContaining(["activate", "setpw"])); + expect(failDetails).toEqual(expect.arrayContaining(["facebook.login_wall"])); // Every code the frontend localizes is raised somewhere the scan can see. expect(adminCodes.filter((c) => !adminRefusals.some((m) => m[1] === c))).toEqual([]); }); @@ -117,6 +125,18 @@ describe("backend ids have their names in both languages", () => { expect(quotaActions.filter((a) => !idsUnder(keys, "quotaActions").has(a))).toEqual([]); }); + it(`${lang}: every download failure-catalog entry`, () => { + expect( + missing(failDetails.flatMap((d) => [`${DETAIL}${d}.short`, `${DETAIL}${d}.body`])), + ).toEqual([]); + const named = new Set( + [...keys] + .filter((k) => k.startsWith(DETAIL)) + .map((k) => k.slice(DETAIL.length).replace(/\.(short|body)$/, "")), + ); + expect([...named].filter((d) => !failDetails.includes(d))).toEqual([]); + }); + it(`${lang}: every admin refusal reason`, () => { expect(missing(adminReasons.map((r) => `errors.admin.${r}`))).toEqual([]); expect([...idsUnder(keys, "errors.admin")].filter((r) => !adminReasons.includes(r))).toEqual( diff --git a/frontend/src/i18n/locales/en/downloads.json b/frontend/src/i18n/locales/en/downloads.json index e0cc6b04..1c21a6c4 100644 --- a/frontend/src/i18n/locales/en/downloads.json +++ b/frontend/src/i18n/locales/en/downloads.json @@ -287,6 +287,22 @@ "short": "Storage full", "body": "This download was stopped because your storage quota is already used up. Delete something from your library, then try again." } + }, + "catalog": { + "facebook": { + "login_wall": { + "short": "Facebook sign-in required", + "body": "Facebook only shows this to signed-in users (e.g. 18+ content or a closed group). Add your Facebook cookies under Settings → Account → Download authentication, then press Retry." + }, + "cookie_rejected": { + "short": "Facebook didn't accept the cookies", + "body": "The cookies have probably expired, or you signed out in that browser. Export them again, upload them, and retry. If it still fails, the content has been removed or isn't visible to you." + }, + "group_post_video": { + "short": "The video isn't reachable from the post link", + "body": "This is the link of a group post, not of the video. Open the video on Facebook and paste its link." + } + } } }, "cookies": { diff --git a/frontend/src/i18n/locales/hu/downloads.json b/frontend/src/i18n/locales/hu/downloads.json index a396d465..c1883b52 100644 --- a/frontend/src/i18n/locales/hu/downloads.json +++ b/frontend/src/i18n/locales/hu/downloads.json @@ -287,6 +287,22 @@ "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." } + }, + "catalog": { + "facebook": { + "login_wall": { + "short": "Facebook-bejelentkezés szükséges", + "body": "A Facebook ezt csak bejelentkezve mutatja (pl. 18+ tartalom vagy zárt csoport). Add meg a Facebook-cookie-kat a Beállítások → Fiók → Letöltés-hitelesítés alatt, majd nyomd meg az Újra gombot." + }, + "cookie_rejected": { + "short": "A Facebook nem fogadta el a cookie-t", + "body": "A cookie valószínűleg lejárt, vagy kijelentkeztél abban a böngészőben. Exportáld újra, töltsd fel, és próbáld újra. Ha így sem megy, a tartalom törölt, vagy számodra nem látható." + }, + "group_post_video": { + "short": "A videó nem érhető el a poszt linkjéről", + "body": "Ez a csoportposzt linkje, nem a videóé. Nyisd meg a videót a Facebookon, és annak a linkjét másold be." + } + } } }, "cookies": { diff --git a/frontend/src/lib/api/downloads.ts b/frontend/src/lib/api/downloads.ts index ed8ca53c..1e07e265 100644 --- a/frontend/src/lib/api/downloads.ts +++ b/frontend/src/lib/api/downloads.ts @@ -76,6 +76,7 @@ export interface DownloadJob { phase: string | null; error: string | null; // raw yt-dlp/ffmpeg text (technical detail) error_code: string | null; // failure category, see backend downloads.errors.classify + error_detail: string | null; // host-specific failure-catalog entry, see downloads.errors.diagnose queue_pos: number; created_at: string | null; source_kind: string; diff --git a/frontend/src/lib/downloadFailure.test.ts b/frontend/src/lib/downloadFailure.test.ts new file mode 100644 index 00000000..ed290cef --- /dev/null +++ b/frontend/src/lib/downloadFailure.test.ts @@ -0,0 +1,43 @@ +// @vitest-environment jsdom +// +// A failed download's explanation: the failure-catalog entry first, its category as the fallback +// (E11.5 S1). The second case runs the REAL i18next with the real locales, because the fallback is +// i18next's list-key lookup, not code of ours — an older client meeting a detail it has no copy for +// must still say the category, not print a key. +import { describe, expect, it } from "vitest"; +import i18n from "../i18n"; +import { failCopyKeys } from "./downloadFailure"; + +describe("failCopyKeys", () => { + it("puts a catalog entry before its category, and is the category alone without one", () => { + expect( + failCopyKeys({ error_code: "login_required", error_detail: "facebook.login_wall" }, "body"), + ).toEqual([ + "downloads.errors.fail.catalog.facebook.login_wall.body", + "downloads.errors.fail.cat.login_required.body", + ]); + expect(failCopyKeys({ error_code: "network", error_detail: null }, "short")).toEqual([ + "downloads.errors.fail.cat.network.short", + ]); + expect(failCopyKeys({ error_code: null, error_detail: null }, "short")).toEqual([ + "downloads.errors.fail.cat.unknown.short", + ]); + }); + + it("reads the entry's copy, and the category's for an entry this client does not know", async () => { + await i18n.changeLanguage("en"); + expect( + i18n.t( + failCopyKeys( + { error_code: "login_required", error_detail: "facebook.login_wall" }, + "short", + ), + ), + ).toBe("Facebook sign-in required"); + expect( + i18n.t( + failCopyKeys({ error_code: "login_required", error_detail: "facebook.newer" }, "short"), + ), + ).toBe("Sign-in required"); + }); +}); diff --git a/frontend/src/lib/downloadFailure.ts b/frontend/src/lib/downloadFailure.ts new file mode 100644 index 00000000..58c77fd1 --- /dev/null +++ b/frontend/src/lib/downloadFailure.ts @@ -0,0 +1,17 @@ +import type { DownloadJob } from "./api"; + +// The i18n keys that explain a failed download, most specific first — the list form `t()` takes, so +// the first key a locale has wins. A failure-catalog entry (`error_detail`, e.g. `facebook.login_wall`) +// names what went wrong on that site and what to do; the category (`error_code`) is the generic +// sentence, and the fallback when an older client meets a detail it has no copy for. One helper for +// the queue row's headline and the failure window, so the two can never explain the same job +// differently. +export function failCopyKeys( + job: Pick, + part: "short" | "body", +): string[] { + const category = `downloads.errors.fail.cat.${job.error_code || "unknown"}.${part}`; + return job.error_detail + ? [`downloads.errors.fail.catalog.${job.error_detail}.${part}`, category] + : [category]; +}