diff --git a/core/video/enrichment/clients.py b/core/video/enrichment/clients.py index 7738c9e9..6c1e08f8 100644 --- a/core/video/enrichment/clients.py +++ b/core/video/enrichment/clients.py @@ -62,6 +62,10 @@ class TMDBClient: if year: params["year" if kind == "movie" else "first_air_date_year"] = year resp = requests.get(self.BASE + path, params=params, timeout=15) + # A non-200 (429 rate-limit, 5xx, timeout-as-error) is a FAILED call, + # not "no match" — raise so the worker records 'error' (retried later) + # instead of burning the item to 'not_found'. + resp.raise_for_status() results = (resp.json() or {}).get("results") or [] if not results: return None @@ -112,30 +116,41 @@ class TVDBClient: logger.exception("TVDB test failed") return False, "Could not reach TVDB" - def _auth(self): - if self._token: + def _auth(self, force=False): + if self._token and not force: return self._token import requests + self._token = None r = requests.post(self.BASE + "/login", json={"apikey": self.api_key}, timeout=15).json() or {} self._token = (r.get("data") or {}).get("token") return self._token - def match(self, kind, title, year, known_id=None): - if kind != "show" or not self.api_key: - return None + def _authed_get(self, path, params=None): + """GET with the bearer token, transparently re-authenticating once if the + cached token has expired (401). Raises on any other non-200 so the worker + records 'error' rather than a false 'not_found'.""" import requests token = self._auth() if not token: return None - headers = {"Authorization": "Bearer " + token} + r = requests.get(self.BASE + path, headers={"Authorization": "Bearer " + token}, + params=params, timeout=15) + if r.status_code == 401 and self._auth(force=True): # token expired → refresh once + r = requests.get(self.BASE + path, headers={"Authorization": "Bearer " + self._token}, + params=params, timeout=15) + r.raise_for_status() + return r.json() or {} + + def match(self, kind, title, year, known_id=None): + if kind != "show" or not self.api_key: + return None tvdb_id = _int(known_id) meta = {} if tvdb_id is None: if not title: return None - r = requests.get(self.BASE + "/search", headers=headers, - params={"query": title, "type": "series"}, timeout=15).json() or {} - results = r.get("data") or [] + r = self._authed_get("/search", {"query": title, "type": "series"}) + results = (r or {}).get("data") or [] if not results: return None top = results[0] @@ -144,9 +159,8 @@ class TVDBClient: else: # Known id from the server → fetch the series directly for its overview. try: - dr = requests.get(self.BASE + "/series/" + str(tvdb_id), - headers=headers, timeout=15).json() or {} - meta["overview"] = (dr.get("data") or {}).get("overview") + dr = self._authed_get("/series/" + str(tvdb_id)) + meta["overview"] = ((dr or {}).get("data") or {}).get("overview") except Exception: logger.exception("TVDB details fetch failed for %s", title or tvdb_id) if tvdb_id is None: diff --git a/core/video/enrichment/worker.py b/core/video/enrichment/worker.py index 750c11e9..3a19f667 100644 --- a/core/video/enrichment/worker.py +++ b/core/video/enrichment/worker.py @@ -110,7 +110,10 @@ class VideoEnrichmentWorker: except Exception: logger.exception("video enrichment %s match failed for %s", self.service, item["title"]) self.stats["errors"] += 1 - self.db.enrichment_apply(self.service, item["kind"], item["id"], matched=False) + # The CALL failed (network/rate-limit/timeout) — record 'error', NOT + # 'not_found', so a transient blip isn't permanently logged as "no + # match". enrichment_next retries 'error' items after retry_days. + self.db.enrichment_apply(self.service, item["kind"], item["id"], matched=False, error=True) return True if result and result.get("id"): self.db.enrichment_apply(self.service, item["kind"], item["id"], matched=True, @@ -124,12 +127,14 @@ class VideoEnrichmentWorker: # ── status (same shape the music enrichment API returns) ────────────────── def get_stats(self) -> dict: breakdown = self.db.enrichment_breakdown(self.service) - pending = sum(b["pending"] for b in breakdown.values()) + # Errored items are outstanding (retried later), so they count as pending + # work — the worker isn't "Complete" while any remain. + pending = sum(b["pending"] + b.get("errors", 0) for b in breakdown.values()) running = self.running and not self.paused and self.enabled idle = running and pending == 0 and self.current_item is None progress = {} for kind, b in breakdown.items(): - total = b["matched"] + b["not_found"] + b["pending"] + total = b["matched"] + b["not_found"] + b.get("errors", 0) + b["pending"] done = b["matched"] + b["not_found"] progress[kind] = {"matched": b["matched"], "total": total, "percent": round(done / total * 100) if total else 0} diff --git a/database/video_database.py b/database/video_database.py index cba0ed6d..58d21978 100644 --- a/database/video_database.py +++ b/database/video_database.py @@ -170,7 +170,8 @@ class VideoDatabase: return _row(row, kind, idc) for kind, (tbl, idc, sc, ac) in kinds.items(): row = conn.execute( - f"SELECT id, title, year, {idc} FROM {tbl} WHERE {sc}='not_found' " + f"SELECT id, title, year, {idc} FROM {tbl} " + f"WHERE {sc} IN ('not_found','error') " f"AND ({ac} IS NULL OR {ac} < ?) ORDER BY {ac} LIMIT 1", (cutoff,)).fetchone() if row: return _row(row, kind, idc) @@ -179,14 +180,22 @@ class VideoDatabase: conn.close() def enrichment_apply(self, service: str, kind: str, item_id: int, matched: bool, - external_id=None, metadata: dict | None = None) -> None: + external_id=None, metadata: dict | None = None, + error: bool = False) -> None: """Record a match result: set match_status + last_attempted, the external - id (when matched), and any whitelisted metadata columns.""" + id (when matched), and any whitelisted metadata columns. + + Status is one of 'matched' / 'not_found' / 'error'. 'error' means the + lookup CALL failed (network/rate-limit/timeout) — distinct from a genuine + 'not_found' so a transient blip isn't permanently recorded as "no match" + (mirrors the music workers). Both 'not_found' and 'error' are retried by + enrichment_next after retry_days.""" spec = _ENRICH.get(service, {}).get(kind) if not spec: return tbl, idc, sc, ac = spec allowed = _ENRICH_META_COLS.get(tbl, set()) + status = "matched" if matched else "error" if error else "not_found" # On legacy DBs tmdb_id/tvdb_id may still carry a UNIQUE index; if a match # would collide with another row's id we drop the id columns and keep the # existing (authoritative) id, still recording status + metadata. @@ -194,7 +203,7 @@ class VideoDatabase: def build(include_ids): sets = [f"{sc}=?", f"{ac}=CURRENT_TIMESTAMP"] - params = ["matched" if matched else "not_found"] + params = [status] if matched and external_id is not None and include_ids: sets.append(f"{idc}=?") params.append(external_id) @@ -231,6 +240,7 @@ class VideoDatabase: out[kind] = { "matched": conn.execute(f"SELECT COUNT(*) FROM {tbl} WHERE {sc}='matched'").fetchone()[0], "not_found": conn.execute(f"SELECT COUNT(*) FROM {tbl} WHERE {sc}='not_found'").fetchone()[0], + "errors": conn.execute(f"SELECT COUNT(*) FROM {tbl} WHERE {sc}='error'").fetchone()[0], "pending": conn.execute(f"SELECT COUNT(*) FROM {tbl} WHERE {sc} IS NULL").fetchone()[0], } return out @@ -247,7 +257,7 @@ class VideoDatabase: if status == "pending": where.append(f"{sc} IS NULL") elif status == "unmatched": - where.append(f"({sc} IS NULL OR {sc}='not_found')") + where.append(f"({sc} IS NULL OR {sc} IN ('not_found','error'))") else: where.append(f"{sc}='not_found'") if search: @@ -282,7 +292,8 @@ class VideoDatabase: if scope == "item" and item_id is not None: cur = conn.execute(f"UPDATE {tbl} SET {sc}=NULL, {ac}=NULL WHERE id=?", (item_id,)) else: - cur = conn.execute(f"UPDATE {tbl} SET {sc}=NULL, {ac}=NULL WHERE {sc}='not_found'") + cur = conn.execute( + f"UPDATE {tbl} SET {sc}=NULL, {ac}=NULL WHERE {sc} IN ('not_found','error')") conn.commit() return cur.rowcount finally: diff --git a/tests/test_video_database.py b/tests/test_video_database.py index 47c6b4e2..4908ac32 100644 --- a/tests/test_video_database.py +++ b/tests/test_video_database.py @@ -387,13 +387,27 @@ def test_enrichment_breakdown_unmatched_retry(db): b = db.upsert_movie("plex", {"server_id": "m2", "title": "B"}) db.enrichment_apply("tmdb", "movie", a, matched=True, external_id=1) db.enrichment_apply("tmdb", "movie", b, matched=False) - assert db.enrichment_breakdown("tmdb")["movie"] == {"matched": 1, "not_found": 1, "pending": 0} + assert db.enrichment_breakdown("tmdb")["movie"] == {"matched": 1, "not_found": 1, "errors": 0, "pending": 0} un = db.enrichment_unmatched("tmdb", "movie", status="not_found") assert [i["title"] for i in un["items"]] == ["B"] and un["total"] == 1 assert db.enrichment_retry("tmdb", "movie", scope="failed") == 1 assert db.enrichment_breakdown("tmdb")["movie"]["pending"] == 1 +def test_error_status_is_distinct_and_retryable_in_ui(db): + a = db.upsert_movie("plex", {"server_id": "m1", "title": "A"}) + db.enrichment_apply("tmdb", "movie", a, matched=False, error=True) + bd = db.enrichment_breakdown("tmdb")["movie"] + assert bd == {"matched": 0, "not_found": 0, "errors": 1, "pending": 0} + # Errored items surface in the modal's "unmatched" list so they can be retried. + assert db.enrichment_unmatched("tmdb", "movie", status="unmatched")["total"] == 1 + # ...but NOT in the strict 'not_found'-only view. + assert db.enrichment_unmatched("tmdb", "movie", status="not_found")["total"] == 0 + # "Retry all failed" re-queues errors too (back to pending/NULL). + assert db.enrichment_retry("tmdb", "movie", scope="failed") == 1 + assert db.enrichment_breakdown("tmdb")["movie"]["pending"] == 1 + + def test_tvdb_is_shows_only(db): # TVDB enriches shows, never movies. The breakdown must not advertise a # movie bucket, and asking for tvdb movies returns empty (never garbage) — diff --git a/tests/test_video_enrichment.py b/tests/test_video_enrichment.py index 947bdb10..908aa284 100644 --- a/tests/test_video_enrichment.py +++ b/tests/test_video_enrichment.py @@ -16,7 +16,7 @@ import pytest from database.video_database import VideoDatabase from core.video.enrichment.worker import VideoEnrichmentWorker from core.video.enrichment.engine import VideoEnrichmentEngine -from core.video.enrichment.clients import TMDBClient +from core.video.enrichment.clients import TMDBClient, TVDBClient class FakeClient: @@ -83,6 +83,52 @@ def test_tmdb_client_with_known_id_skips_the_search(monkeypatch): assert not any("/search/" in u for u in urls) +def test_tmdb_client_raises_on_rate_limit(monkeypatch): + # A 429 must raise (→ worker records 'error'), not silently return no-match. + class _Resp429: + status_code = 429 + def raise_for_status(self): + raise RuntimeError("429 Too Many Requests") + def json(self): + return {} + + fake = types.SimpleNamespace(get=lambda url, **kw: _Resp429()) + monkeypatch.setitem(sys.modules, "requests", fake) + with pytest.raises(Exception): + TMDBClient("KEY").match("movie", "Dune", 2021) + + +def test_tvdb_client_refreshes_expired_token(monkeypatch): + # Cached token returns 401 → the client must re-login once and retry, not fail. + logins = [] + state = {"calls": 0} + + class _Resp: + def __init__(self, code, body): + self.status_code = code + self._body = body + def raise_for_status(self): + if self.status_code >= 400: + raise RuntimeError("HTTP " + str(self.status_code)) + def json(self): + return self._body + + def fake_post(url, **kw): + logins.append(url) + return _Resp(200, {"data": {"token": "tok%d" % len(logins)}}) + + def fake_get(url, **kw): + state["calls"] += 1 + if state["calls"] == 1: + return _Resp(401, {}) # stale token rejected + return _Resp(200, {"data": [{"tvdb_id": 77, "overview": "O"}]}) + + monkeypatch.setitem(sys.modules, "requests", types.SimpleNamespace(get=fake_get, post=fake_post)) + res = TVDBClient("KEY").match("show", "Some Show", 2020) + assert res["id"] == 77 + assert len(logins) == 2 # initial login + one refresh after the 401 + + def test_worker_process_one_not_found(db): db.upsert_movie("plex", {"server_id": "m1", "title": "X"}) w = VideoEnrichmentWorker(db, "tmdb", FakeClient(None)) @@ -96,7 +142,10 @@ def test_worker_process_one_no_items_returns_false(db): assert VideoEnrichmentWorker(db, "tmdb", FakeClient(None)).process_one() is False -def test_worker_match_exception_marks_not_found_and_counts_error(db): +def test_worker_match_exception_marks_error_not_notfound(db): + # A failed CALL must be recorded as 'error' (a transient failure), NOT + # 'not_found' — otherwise a network blip permanently logs the item as "no + # match" and it won't retry for retry_days. Mirrors the music workers. db.upsert_movie("plex", {"server_id": "m1", "title": "X"}) class Boom: @@ -107,7 +156,22 @@ def test_worker_match_exception_marks_not_found_and_counts_error(db): assert w.process_one() is True # doesn't crash the loop assert w.stats["errors"] == 1 with db.connect() as c: - assert c.execute("SELECT tmdb_match_status FROM movies").fetchone()["tmdb_match_status"] == "not_found" + assert c.execute("SELECT tmdb_match_status FROM movies").fetchone()["tmdb_match_status"] == "error" + + +def test_errored_item_is_retried_after_retry_days(db): + # An 'error' row is re-queued by enrichment_next once it's older than the + # retry window (just like 'not_found'), so transient failures recover. + mid = db.upsert_movie("plex", {"server_id": "m1", "title": "X"}) + db.enrichment_apply("tmdb", "movie", mid, matched=False, error=True) + # Just attempted → still inside the retry window → not yet due. + assert db.enrichment_next("tmdb", retry_days=30) is None + # Backdate the attempt → now older than the window → re-queued for retry. + with db.connect() as c: + c.execute("UPDATE movies SET tmdb_last_attempted='2000-01-01 00:00:00' WHERE id=?", (mid,)) + c.commit() + again = db.enrichment_next("tmdb", retry_days=30) + assert again is not None and again["id"] == mid def test_worker_get_stats_shape(db): diff --git a/webui/static/video/video-enrichment-manager.js b/webui/static/video/video-enrichment-manager.js index b2dad8fd..74739c38 100644 --- a/webui/static/video/video-enrichment-manager.js +++ b/webui/static/video/video-enrichment-manager.js @@ -168,8 +168,11 @@ var kinds = Object.keys(bd); host.innerHTML = kinds.map(function (e) { var d = bd[e] || {}; - var total = (d.matched || 0) + (d.not_found || 0) + (d.pending || 0); - var matched = d.matched || 0, nf = d.not_found || 0, pend = d.pending || 0; + // Errored items are outstanding (retried later) — show them with the + // pending bucket so the bar/total stay honest. + var matched = d.matched || 0, nf = d.not_found || 0; + var pend = (d.pending || 0) + (d.errors || 0); + var total = matched + nf + pend; var pct = total ? Math.round(matched / total * 100) : 0; var seg = function (n) { return total ? (n / total) * 100 : 0; }; var active = e === state.kind ? ' em-card--current' : ''; @@ -189,7 +192,7 @@ var m = 0, t = 0; kinds.forEach(function (e) { var d = bd[e] || {}; m += d.matched || 0; - t += (d.matched || 0) + (d.not_found || 0) + (d.pending || 0); + t += (d.matched || 0) + (d.not_found || 0) + (d.pending || 0) + (d.errors || 0); }); overall.innerHTML = t ? '' + (t ? Math.round(m / t * 100) : 0) + '% matched · ' + m + ' of ' + t : '';