diff --git a/backend/app/downloads/edit.py b/backend/app/downloads/edit.py index b52064f..cff449a 100644 --- a/backend/app/downloads/edit.py +++ b/backend/app/downloads/edit.py @@ -79,13 +79,17 @@ def normalize_edit_spec(spec: dict | None) -> dict: # Single TRIM (one output file; the frontend fans a "separate" split out into N of these). trim = spec.get("trim") or {} if trim: - start = max(0.0, float(trim.get("start_s") or 0.0)) - end_raw = trim.get("end_s") + # Guard the numeric coercion like the crop/segments branches do — a malformed value must + # drop the trim (→ empty spec → 400 EditError), not raise an unhandled 500. + try: + start = max(0.0, float(trim.get("start_s") or 0.0)) + end_raw = trim.get("end_s") + end = float(end_raw) if end_raw is not None else None + except (TypeError, ValueError): + start, end = 0.0, None t: dict = {"start_s": round(start, 3)} - if end_raw is not None: - end = float(end_raw) - if end > start: - t["end_s"] = round(end, 3) + if end is not None and end > start: + t["end_s"] = round(end, 3) # A trim with only a start (open-ended) is valid (cut to the end). if t.get("start_s") or "end_s" in t: out["trim"] = t diff --git a/backend/app/downloads/formats.py b/backend/app/downloads/formats.py index ef38ef9..8d79266 100644 --- a/backend/app/downloads/formats.py +++ b/backend/app/downloads/formats.py @@ -62,7 +62,12 @@ def normalize(spec: dict) -> dict: if s["mode"] not in ("av", "v", "a"): s["mode"] = "av" if s["max_height"] is not None: - s["max_height"] = int(s["max_height"]) + # A malformed max_height (non-numeric inline spec / stored profile) falls back to "best" + # rather than raising an unhandled 500 at enqueue. + try: + s["max_height"] = int(s["max_height"]) + except (TypeError, ValueError): + s["max_height"] = None for b in ("embed_subs", "embed_chapters", "embed_thumbnail", "sponsorblock"): s[b] = bool(s[b]) # Audio-only files can't embed subtitles/thumbnails as video; keep the flags meaningful. diff --git a/backend/app/downloads/service.py b/backend/app/downloads/service.py index b20304b..9436c14 100644 --- a/backend/app/downloads/service.py +++ b/backend/app/downloads/service.py @@ -46,6 +46,14 @@ def get_or_create_asset( ) asset = db.execute(stmt).scalar_one_or_none() if asset is not None: + # A previously-FAILED shared asset must be re-attempted for the new job about to hold it — + # reset it to pending (+clear the error) so the worker actually re-downloads instead of + # short-circuiting the new job with the asset's stale error. (Mirrors resume_download; without + # this a fresh enqueue of a once-failed (source,format) pair is permanently poisoned, since + # errored assets carry no expires_at and GC never clears them.) + if asset.status == "error": + asset.status = "pending" + asset.error = None return asset asset = MediaAsset( source_kind=source_kind, diff --git a/backend/app/routes/downloads.py b/backend/app/routes/downloads.py index ee5490e..066249c 100644 --- a/backend/app/routes/downloads.py +++ b/backend/app/routes/downloads.py @@ -15,6 +15,7 @@ from fastapi.responses import FileResponse from sqlalchemy import func, select from sqlalchemy.orm import Session +from app import sysconfig from app.auth import admin_user, require_human from app.config import settings from app.db import get_db @@ -499,9 +500,18 @@ def _release_asset(db: Session, job: DownloadJob) -> None: """Drop this job's hold on its asset when it leaves a holding state. Once NO job holds the asset anymore, delete the file + row immediately — a deleted download should free its disk, and the shared cache only needs to span *overlapping* holders (a later re-add just downloads - again). Idempotent: a job that's already left holding (canceled/error) is a no-op, so delete - after cancel doesn't double-release.""" - if not job.asset_id or job.status not in ("queued", "running", "paused", "done"): + again). `error` counts as holding: enqueue always +1'd ref_count and the worker never + decrements on failure (ref_count is the route's job), so an errored job releases here on + delete — else its +1 leaks and a later resume→ready keeps the file pinned past its last holder. + Idempotent for the cancel path: cancel already released while the job was holding, then set it + `canceled` (∉ the set below), so delete-after-cancel is a no-op and never double-releases. + + We DON'T delete an errored asset row here even at ref==0: another request may be concurrently + re-enqueuing the same (source,format) — get_or_create_asset would reset that row to `pending` and + +1 it, and deleting it under that would FK-null the new job's asset (a lost update on ref_count). + A ready asset is still freed at ref==0 (its file is the point); an orphaned errored row is cheap + (no file) and gets reused+reset by the next enqueue, or reclaimed by a later GC pass.""" + if not job.asset_id or job.status not in ("queued", "running", "paused", "done", "error"): return asset = db.get(MediaAsset, job.asset_id) if asset is None: @@ -876,7 +886,7 @@ def admin_storage( "ready_files": ready[0], "total_bytes": int(ready[1]), "total_assets": total_assets, - "total_cap_bytes": settings.download_total_max_bytes, + "total_cap_bytes": sysconfig.get_int(db, "download_total_max_bytes"), "per_user": [ {"user_id": uid, "email": emails.get(uid), "footprint_bytes": quota.footprint(db, uid)} for uid, _ in per_user