Fix podcast episode downloads: ffmpeg muxer bug and dead-URL retry storm
_extract_audio wrote transcoded audio to a .tmp temp path, so ffmpeg couldn't guess a muxer from the filename and aborted on every video episode whose source was still reachable. Pass -f mp3 explicitly instead of relying on the extension. Separately, a permanently 404'd episode was retried on every startup and every 6-hour poll forever, since nothing remembered past failures. Track failed downloads per show folder with a 7-day backoff before retrying. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -19,11 +19,12 @@ and skipped, not a crash, mirroring ``scanner.py``'s own rule.
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import asyncio
|
import asyncio
|
||||||
|
import json
|
||||||
import logging
|
import logging
|
||||||
import re
|
import re
|
||||||
import shutil
|
import shutil
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
from datetime import UTC, datetime
|
from datetime import UTC, datetime, timedelta
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from typing import Any, Final
|
from typing import Any, Final
|
||||||
from urllib.parse import urlsplit
|
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.
|
#: as failed - generous, since this runs on a Pi and a long video can take a while.
|
||||||
_FFMPEG_TIMEOUT: Final = 600.0
|
_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):
|
class AudioExtractionError(Exception):
|
||||||
"""Raised when a video enclosure could not be turned into an audio file."""
|
"""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
|
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(
|
async def download_episode(
|
||||||
client: httpx2.AsyncClient, folder: Path, episode: Episode, filename: str
|
client: httpx2.AsyncClient, folder: Path, episode: Episode, filename: str
|
||||||
) -> None:
|
) -> None:
|
||||||
@@ -328,6 +371,8 @@ async def _extract_audio(source_url: str, dest: Path) -> None:
|
|||||||
"libmp3lame",
|
"libmp3lame",
|
||||||
"-q:a",
|
"-q:a",
|
||||||
"2",
|
"2",
|
||||||
|
"-f",
|
||||||
|
"mp3",
|
||||||
str(dest),
|
str(dest),
|
||||||
stdout=asyncio.subprocess.DEVNULL,
|
stdout=asyncio.subprocess.DEVNULL,
|
||||||
stderr=asyncio.subprocess.PIPE,
|
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
|
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
|
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:
|
try:
|
||||||
response = await client.get(feed_url, timeout=_HTTP_TIMEOUT, follow_redirects=True)
|
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
|
return False
|
||||||
|
|
||||||
pending = missing_episodes(folder, parse_feed(response.content))
|
pending = missing_episodes(folder, parse_feed(response.content))
|
||||||
|
failed = _load_failed_downloads(folder)
|
||||||
|
now = datetime.now(UTC)
|
||||||
changed = False
|
changed = False
|
||||||
for episode, filename in pending:
|
for episode, filename in pending:
|
||||||
|
last_failure = failed.get(filename)
|
||||||
|
if last_failure is not None and now - last_failure < _RETRY_BACKOFF:
|
||||||
|
continue
|
||||||
try:
|
try:
|
||||||
await download_episode(client, folder, episode, filename)
|
await download_episode(client, folder, episode, filename)
|
||||||
except (httpx2.HTTPError, OSError, AudioExtractionError) as exc:
|
except (httpx2.HTTPError, OSError, AudioExtractionError) as exc:
|
||||||
_log.warning(
|
_log.warning(
|
||||||
"Could not download episode %r for %s: %s", episode.title, folder.name, exc
|
"Could not download episode %r for %s: %s", episode.title, folder.name, exc
|
||||||
)
|
)
|
||||||
|
failed[filename] = now
|
||||||
continue
|
continue
|
||||||
|
failed.pop(filename, None)
|
||||||
_log.info("Downloaded new episode %r for %s", episode.title, folder.name)
|
_log.info("Downloaded new episode %r for %s", episode.title, folder.name)
|
||||||
changed = True
|
changed = True
|
||||||
|
|
||||||
@@ -432,6 +486,10 @@ async def sync_show(client: httpx2.AsyncClient, folder: Path, feed_url: str) ->
|
|||||||
folder.name,
|
folder.name,
|
||||||
exc,
|
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
|
return changed
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -2,6 +2,8 @@
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import json
|
||||||
|
from datetime import UTC, datetime, timedelta
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
import httpx2
|
import httpx2
|
||||||
@@ -491,9 +493,12 @@ async def test_download_episode_extracts_audio_via_ffmpeg(
|
|||||||
async def communicate(self) -> tuple[bytes, bytes]:
|
async def communicate(self) -> tuple[bytes, bytes]:
|
||||||
return b"", b""
|
return b"", b""
|
||||||
|
|
||||||
|
captured_args: list[object] = []
|
||||||
|
|
||||||
async def fake_exec(*args: object, **kwargs: object) -> _SucceedingProcess:
|
async def fake_exec(*args: object, **kwargs: object) -> _SucceedingProcess:
|
||||||
# ffmpeg's destination path is the last positional argument; write to it here
|
# ffmpeg's destination path is the last positional argument; write to it here
|
||||||
# to stand in for what the real binary would have produced.
|
# to stand in for what the real binary would have produced.
|
||||||
|
captured_args.extend(args)
|
||||||
Path(str(args[-1])).write_bytes(b"extracted-audio")
|
Path(str(args[-1])).write_bytes(b"extracted-audio")
|
||||||
return _SucceedingProcess()
|
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 (folder / "20250101 - Video Only.mp3").read_bytes() == b"extracted-audio"
|
||||||
assert list(folder.glob(".downloading-*.tmp")) == []
|
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()
|
||||||
|
|||||||
Reference in New Issue
Block a user