diff --git a/python-backend/musicmouse/library/podcast_feeds.py b/python-backend/musicmouse/library/podcast_feeds.py index 9ceedf8..ec3f6f3 100644 --- a/python-backend/musicmouse/library/podcast_feeds.py +++ b/python-backend/musicmouse/library/podcast_feeds.py @@ -19,11 +19,12 @@ and skipped, not a crash, mirroring ``scanner.py``'s own rule. from __future__ import annotations import asyncio +import json import logging import re import shutil from dataclasses import dataclass -from datetime import UTC, datetime +from datetime import UTC, datetime, timedelta from pathlib import Path from typing import Any, Final from urllib.parse import urlsplit @@ -94,6 +95,14 @@ _OG_IMAGE_RE: Final = re.compile( #: as failed - generous, since this runs on a Pi and a long video can take a while. _FFMPEG_TIMEOUT: Final = 600.0 +#: Where a show folder remembers episodes that failed to download, so a permanently +#: dead enclosure (e.g. pulled from the host's CDN) isn't retried every poll. +_FAILED_DOWNLOADS_FILENAME: Final = ".failed-downloads.json" +#: How long a failed episode is left alone before it's given another chance - long +#: enough to stop hammering a dead URL every 6 hours, short enough that a genuinely +#: transient failure (a host outage, a flaky network) still recovers on its own. +_RETRY_BACKOFF: Final = timedelta(days=7) + class AudioExtractionError(Exception): """Raised when a video enclosure could not be turned into an audio file.""" @@ -278,6 +287,40 @@ def missing_episodes(folder: Path, episodes: list[Episode]) -> list[tuple[Episod return out +def _load_failed_downloads(folder: Path) -> dict[str, datetime]: + """Filename -> when it last failed to download, for episodes ``sync_show`` should + leave alone until :data:`_RETRY_BACKOFF` has passed. + + A missing or corrupt record is just an empty one - nothing here is precious enough + to raise over, matching the module's overall rule that bad input is logged and + skipped rather than fatal. + """ + path = folder / _FAILED_DOWNLOADS_FILENAME + try: + raw = json.loads(path.read_text(encoding="utf-8")) + except FileNotFoundError: + return {} + except (OSError, ValueError) as exc: + _log.debug("Could not read %s; treating as empty: %s", path, exc) + return {} + try: + return {str(filename): datetime.fromisoformat(when) for filename, when in raw.items()} + except (AttributeError, TypeError, ValueError) as exc: + _log.debug("Could not parse %s; treating as empty: %s", path, exc) + return {} + + +def _save_failed_downloads(folder: Path, failed: dict[str, datetime]) -> None: + path = folder / _FAILED_DOWNLOADS_FILENAME + if not failed: + path.unlink(missing_ok=True) + return + temp_path = folder / f".{_FAILED_DOWNLOADS_FILENAME}.tmp" + payload = {filename: when.isoformat() for filename, when in failed.items()} + temp_path.write_text(json.dumps(payload), encoding="utf-8") + temp_path.replace(path) + + async def download_episode( client: httpx2.AsyncClient, folder: Path, episode: Episode, filename: str ) -> None: @@ -328,6 +371,8 @@ async def _extract_audio(source_url: str, dest: Path) -> None: "libmp3lame", "-q:a", "2", + "-f", + "mp3", str(dest), stdout=asyncio.subprocess.DEVNULL, stderr=asyncio.subprocess.PIPE, @@ -397,7 +442,9 @@ async def sync_show(client: httpx2.AsyncClient, folder: Path, feed_url: str) -> Returns whether anything changed. Errors - an unreachable feed, a malformed one, a single broken enclosure - are logged and swallowed here so one bad show never stops - the others or takes down the poll loop. + the others or takes down the poll loop. An episode that fails is remembered and left + alone for :data:`_RETRY_BACKOFF` before it's attempted again, so a permanently dead + enclosure doesn't get hammered on every poll. """ try: response = await client.get(feed_url, timeout=_HTTP_TIMEOUT, follow_redirects=True) @@ -407,15 +454,22 @@ async def sync_show(client: httpx2.AsyncClient, folder: Path, feed_url: str) -> return False pending = missing_episodes(folder, parse_feed(response.content)) + failed = _load_failed_downloads(folder) + now = datetime.now(UTC) changed = False for episode, filename in pending: + last_failure = failed.get(filename) + if last_failure is not None and now - last_failure < _RETRY_BACKOFF: + continue try: await download_episode(client, folder, episode, filename) except (httpx2.HTTPError, OSError, AudioExtractionError) as exc: _log.warning( "Could not download episode %r for %s: %s", episode.title, folder.name, exc ) + failed[filename] = now continue + failed.pop(filename, None) _log.info("Downloaded new episode %r for %s", episode.title, folder.name) changed = True @@ -432,6 +486,10 @@ async def sync_show(client: httpx2.AsyncClient, folder: Path, feed_url: str) -> folder.name, exc, ) + + pending_filenames = {filename for _episode, filename in pending} + failed = {filename: when for filename, when in failed.items() if filename in pending_filenames} + _save_failed_downloads(folder, failed) return changed diff --git a/python-backend/tests/test_podcast_feeds.py b/python-backend/tests/test_podcast_feeds.py index 48b4ec5..9618594 100644 --- a/python-backend/tests/test_podcast_feeds.py +++ b/python-backend/tests/test_podcast_feeds.py @@ -2,6 +2,8 @@ from __future__ import annotations +import json +from datetime import UTC, datetime, timedelta from pathlib import Path import httpx2 @@ -491,9 +493,12 @@ async def test_download_episode_extracts_audio_via_ffmpeg( async def communicate(self) -> tuple[bytes, bytes]: return b"", b"" + captured_args: list[object] = [] + async def fake_exec(*args: object, **kwargs: object) -> _SucceedingProcess: # ffmpeg's destination path is the last positional argument; write to it here # to stand in for what the real binary would have produced. + captured_args.extend(args) Path(str(args[-1])).write_bytes(b"extracted-audio") return _SucceedingProcess() @@ -510,3 +515,118 @@ async def test_download_episode_extracts_audio_via_ffmpeg( assert (folder / "20250101 - Video Only.mp3").read_bytes() == b"extracted-audio" assert list(folder.glob(".downloading-*.tmp")) == [] + # The destination path ends in ".tmp", not ".mp3" - ffmpeg can't guess the muxer + # from that extension, so the format must be passed explicitly. + assert captured_args[captured_args.index("-f") + 1] == "mp3" + + +@pytest.mark.asyncio +async def test_sync_show_does_not_retry_a_failed_episode_within_the_backoff( + tmp_path: Path, +) -> None: + folder = tmp_path / "show" + folder.mkdir() + enclosure_requests: list[str] = [] + + def handler(request: httpx2.Request) -> httpx2.Response: + url = str(request.url) + if url == "http://example.com/feed.xml": + return httpx2.Response(200, content=_FEED) + enclosure_requests.append(url) + return httpx2.Response(404) + + async with httpx2.AsyncClient(transport=httpx2.MockTransport(handler)) as client: + await sync_show(client, folder, "http://example.com/feed.xml") + await sync_show(client, folder, "http://example.com/feed.xml") + + # Both episodes 404 on the first pass and get remembered; the second pass must not + # re-request either enclosure while the backoff is still in effect. + assert len(enclosure_requests) == 2 + assert (folder / podcast_feeds._FAILED_DOWNLOADS_FILENAME).exists() + + +@pytest.mark.asyncio +async def test_sync_show_retries_a_failed_episode_once_the_backoff_expires( + tmp_path: Path, +) -> None: + folder = tmp_path / "show" + folder.mkdir() + enclosure_requests: list[str] = [] + + def handler(request: httpx2.Request) -> httpx2.Response: + url = str(request.url) + if url == "http://example.com/feed.xml": + return httpx2.Response(200, content=_FEED) + enclosure_requests.append(url) + return httpx2.Response(404) + + async with httpx2.AsyncClient(transport=httpx2.MockTransport(handler)) as client: + await sync_show(client, folder, "http://example.com/feed.xml") + + # Backdate both failure records past the backoff window, simulating that a + # week has gone by since the first attempt. + record_path = folder / podcast_feeds._FAILED_DOWNLOADS_FILENAME + stale = datetime.now(UTC) - podcast_feeds._RETRY_BACKOFF - timedelta(seconds=1) + record = json.loads(record_path.read_text(encoding="utf-8")) + record_path.write_text( + json.dumps({filename: stale.isoformat() for filename in record}), encoding="utf-8" + ) + + await sync_show(client, folder, "http://example.com/feed.xml") + + assert len(enclosure_requests) == 4 + + +@pytest.mark.asyncio +async def test_sync_show_clears_the_failure_record_once_an_episode_succeeds( + tmp_path: Path, +) -> None: + folder = tmp_path / "show" + folder.mkdir() + should_fail = True + + def handler(request: httpx2.Request) -> httpx2.Response: + url = str(request.url) + if url == "http://example.com/feed.xml": + return httpx2.Response(200, content=_FEED) + if should_fail: + return httpx2.Response(404) + return httpx2.Response(200, content=b"audio-bytes") + + async with httpx2.AsyncClient(transport=httpx2.MockTransport(handler)) as client: + await sync_show(client, folder, "http://example.com/feed.xml") + assert (folder / podcast_feeds._FAILED_DOWNLOADS_FILENAME).exists() + + should_fail = False + # Backdate the record so the retry isn't just skipped by the backoff window. + record_path = folder / podcast_feeds._FAILED_DOWNLOADS_FILENAME + stale = datetime.now(UTC) - podcast_feeds._RETRY_BACKOFF - timedelta(seconds=1) + record = json.loads(record_path.read_text(encoding="utf-8")) + record_path.write_text( + json.dumps({filename: stale.isoformat() for filename in record}), encoding="utf-8" + ) + + changed = await sync_show(client, folder, "http://example.com/feed.xml") + + assert changed is True + assert not (folder / podcast_feeds._FAILED_DOWNLOADS_FILENAME).exists() + + +@pytest.mark.asyncio +async def test_sync_show_survives_a_corrupt_failure_record(tmp_path: Path) -> None: + folder = tmp_path / "show" + folder.mkdir() + (folder / podcast_feeds._FAILED_DOWNLOADS_FILENAME).write_text( + "not valid json", encoding="utf-8" + ) + + def handler(request: httpx2.Request) -> httpx2.Response: + if str(request.url) == "http://example.com/feed.xml": + return httpx2.Response(200, content=_FEED) + return httpx2.Response(200, content=b"audio-bytes") + + async with httpx2.AsyncClient(transport=httpx2.MockTransport(handler)) as client: + changed = await sync_show(client, folder, "http://example.com/feed.xml") + + assert changed is True + assert (folder / "20250101 - Episode One.mp3").exists()