diff --git a/core/metadata/artwork.py b/core/metadata/artwork.py index e8b63987..d405fa01 100644 --- a/core/metadata/artwork.py +++ b/core/metadata/artwork.py @@ -341,8 +341,33 @@ def download_cover_art(album_info: dict, target_dir: str, context: dict = None): if not art_url: logger.warning("No cover art URL available for download.") return - with urllib.request.urlopen(art_url, timeout=10) as response: - image_data = response.read() + # Fetch with one fallback level: if we upgraded a Deezer + # URL above and the CDN happens to refuse the larger size + # for this specific album, retry with the original URL so + # we never regress vs. pre-upgrade behavior. Empirically + # 1900 works for every album tested but defending against + # the edge case keeps the fix strictly non-regressive. + original_url = album_info.get("album_image_url") + if context and not original_url: + album_ctx = get_import_context_album(context) + original_url = album_ctx.get("image_url") or original_url + try: + with urllib.request.urlopen(art_url, timeout=10) as response: + image_data = response.read() + except Exception as fetch_err: + if ( + "dzcdn" in art_url + and original_url + and original_url != art_url + ): + logger.info( + "Deezer CDN refused upgraded cover URL (%s); " + "retrying with original size", fetch_err, + ) + with urllib.request.urlopen(original_url, timeout=10) as response: + image_data = response.read() + else: + raise if not image_data: return diff --git a/core/tag_writer.py b/core/tag_writer.py index 97b110a1..11cb5be1 100644 --- a/core/tag_writer.py +++ b/core/tag_writer.py @@ -209,10 +209,14 @@ def download_cover_art(cover_url: str) -> Optional[Tuple[bytes, str]]: max). Mirrors the same upgrade in ``core.metadata.artwork.download_cover_art`` so the enhanced-library-view "Write Tags to File" feature embeds the same - high-resolution cover the auto post-process flow does. + high-resolution cover the auto post-process flow does. Falls back + to the original URL if the CDN refuses the upgraded size for a + specific album — keeps the fix strictly non-regressive vs. the + pre-upgrade behaviour. """ if not cover_url: return None + original_url = cover_url if 'dzcdn' in cover_url: try: from core.deezer_client import _upgrade_deezer_cover_url @@ -226,6 +230,21 @@ def download_cover_art(cover_url: str) -> Optional[Tuple[bytes, str]]: if image_data: return (image_data, mime_type) except Exception as e: + # Deezer CDN refused upgraded size for this album — retry with + # original URL so we never get less than pre-upgrade behaviour. + if 'dzcdn' in cover_url and cover_url != original_url: + logger.info( + "Deezer CDN refused upgraded cover URL (%s); retrying with original size", e, + ) + try: + with urllib.request.urlopen(original_url, timeout=15) as response: + image_data = response.read() + mime_type = response.info().get_content_type() or 'image/jpeg' + if image_data: + return (image_data, mime_type) + except Exception as fallback_err: + logger.error(f"Error downloading cover art (fallback): {fallback_err}") + return None logger.error(f"Error downloading cover art from {cover_url}: {e}") return None diff --git a/tests/metadata/test_deezer_cover_url_upgrade.py b/tests/metadata/test_deezer_cover_url_upgrade.py index 184d4a6d..caba3dea 100644 --- a/tests/metadata/test_deezer_cover_url_upgrade.py +++ b/tests/metadata/test_deezer_cover_url_upgrade.py @@ -138,3 +138,68 @@ class TestEmptyInputs: def test_none(self): assert _upgrade_deezer_cover_url(None) is None + + +# --------------------------------------------------------------------------- +# Download fallback — if upgraded URL 403s, retry with original +# --------------------------------------------------------------------------- + + +class TestDownloadFallbackOnCdnRefusal: + """If Deezer CDN refuses the upgraded 1900×1900 URL for some + specific album (rare but possible — empirically tested 4 albums + and none hit this, but defending the edge keeps the fix + strictly non-regressive vs. pre-upgrade behaviour).""" + + def test_tag_writer_retries_with_original_on_failure(self, monkeypatch): + """tag_writer.download_cover_art must fall back to the + original URL when the upgraded URL fails.""" + from core import tag_writer + + original_url = 'https://cdn-images.dzcdn.net/images/cover/abc/1000x1000-000000-80-0-0.jpg' + upgraded_url = 'https://cdn-images.dzcdn.net/images/cover/abc/1900x1900-000000-80-0-0.jpg' + + call_log = [] + + class _FakeResponse: + def read(self): return b'cover-bytes' + def info(self): + class _Info: + def get_content_type(_self): return 'image/jpeg' + return _Info() + def __enter__(self): return self + def __exit__(self, *a): pass + + def fake_urlopen(url, timeout=None): + call_log.append(url) + if url == upgraded_url: + raise Exception("403 Forbidden") + return _FakeResponse() + + monkeypatch.setattr('core.tag_writer.urllib.request.urlopen', fake_urlopen) + + result = tag_writer.download_cover_art(original_url) + + assert result == (b'cover-bytes', 'image/jpeg') + # Tried upgraded first, then fell back to original + assert call_log == [upgraded_url, original_url] + + def test_tag_writer_no_fallback_for_non_dzcdn_url(self, monkeypatch): + """Non-Deezer URLs go through unchanged — no upgrade, no + fallback. Fast path preserved.""" + from core import tag_writer + + spotify_url = 'https://i.scdn.co/image/abc' + call_log = [] + + def fake_urlopen(url, timeout=None): + call_log.append(url) + raise Exception("network error") + + monkeypatch.setattr('core.tag_writer.urllib.request.urlopen', fake_urlopen) + + result = tag_writer.download_cover_art(spotify_url) + + assert result is None + # Single attempt — no Deezer fallback path triggered + assert call_log == [spotify_url]