diff --git a/core/metadata/art_apply.py b/core/metadata/art_apply.py index 448ff0f2..f81a35cd 100644 --- a/core/metadata/art_apply.py +++ b/core/metadata/art_apply.py @@ -168,11 +168,20 @@ def apply_art_to_album_files( target_dir = folder or (os.path.dirname(paths[0]) if paths else None) if target_dir and os.path.isdir(target_dir): + # download_cover_art swallows its own write errors, so a read-only mount + # (EROFS) on the cover.jpg sidecar would otherwise go undetected here — + # the exact gap that left cover-only albums reporting success on a + # read-only filesystem (Sokhi: tracks already had embedded art, so the + # embed loop above never tripped EROFS). Pass a capture dict and read + # the read-only flag it sets back. + cover_ctx = context if isinstance(context, dict) else {} try: - download_cover_art(album_info, target_dir, context) + download_cover_art(album_info, target_dir, cover_ctx) result["cover_written"] = folder_has_cover_sidecar(target_dir) except Exception as exc: if getattr(exc, "errno", None) == errno.EROFS: result["read_only_fs"] = True logger.warning("cover.jpg write failed for %s: %s", target_dir, exc) + if cover_ctx.get("_cover_read_only"): + result["read_only_fs"] = True return result diff --git a/core/metadata/artwork.py b/core/metadata/artwork.py index 3698a064..091ee1c4 100644 --- a/core/metadata/artwork.py +++ b/core/metadata/artwork.py @@ -2,6 +2,7 @@ from __future__ import annotations +import errno import os import re import time @@ -566,4 +567,14 @@ def download_cover_art(album_info: dict, target_dir: str, context: dict = None): handle.write(image_data) logger.info("Cover art downloaded to: %s", cover_path) except Exception as exc: - logger.error("Error downloading cover.jpg: %s", exc) + # A read-only mount (EROFS) is a "can't write" condition the caller + # needs to surface (cover-art filler #804/Tim/Sokhi) — but we must NOT + # re-raise (import callers aren't wrapped here). Record it on the + # context so callers that care can detect it, instead of just spamming + # the log with a swallowed error. + if getattr(exc, "errno", None) == errno.EROFS: + if isinstance(context, dict): + context["_cover_read_only"] = True + logger.warning("cover.jpg write blocked — read-only filesystem: %s", cover_path) + else: + logger.error("Error downloading cover.jpg: %s", exc) diff --git a/tests/test_art_apply.py b/tests/test_art_apply.py index 2be5d9e4..3a590ef1 100644 --- a/tests/test_art_apply.py +++ b/tests/test_art_apply.py @@ -194,6 +194,27 @@ def test_apply_flags_erofs_from_actual_write(tmp_path, monkeypatch): assert res['failed'] == 2 # first EROFS fails it + bails the rest +def test_cover_only_read_only_is_detected(tmp_path, monkeypatch): + """The gap that left Sokhi stuck (#804): tracks already have art so the + embed loop is skipped, but the cover.jpg write hits a read-only mount. + download_cover_art swallows that EROFS onto the context — apply must still + surface read_only_fs (not report a silent success).""" + f = tmp_path / 'a.flac'; f.write_bytes(b'') + audio = SimpleNamespace(pictures=['pic'], tags={'ok': 1}) # already has art → embed skipped + monkeypatch.setattr(aa, 'get_mutagen_symbols', lambda: _fake_symbols(audio)) + + def _fake_download(album_info, target_dir, ctx): + # mimic download_cover_art's EROFS handling: record on context, swallow + if isinstance(ctx, dict): + ctx['_cover_read_only'] = True + monkeypatch.setattr(aa, 'download_cover_art', _fake_download) + + res = aa.apply_art_to_album_files([str(f)], {}, {}, folder=str(tmp_path)) + + assert res['embedded'] == 0 and res['skipped'] == 1 # nothing embedded + assert res['read_only_fs'] is True # but read-only surfaced + + def test_apply_normal_failure_not_flagged_read_only(tmp_path, monkeypatch): def _boom(): raise PermissionError(13, 'Permission denied')