From: Mikhail Nevskiy <139659391+trudenboy@users.noreply.github.com> Date: Mon, 9 Feb 2026 08:42:59 +0000 (+0300) Subject: fix(yandex_music): fix playlist loading and missing album cover art (#3099) X-Git-Url: https://git.kitaultman.com/?a=commitdiff_plain;h=710d84e9e18ad4f7a1386c93311add687c4caa61;p=music-assistant-server.git fix(yandex_music): fix playlist loading and missing album cover art (#3099) * fix(yandex_music): fix playlist tracks not loading in UI - Return empty list for page > 0 since Yandex Music API returns all tracks in a single call; without this the controller pagination loop never terminates - Add fetch_tracks_async() fallback when API returns playlist metadata without tracks - Raise ResourceTemporarilyUnavailable instead of returning empty list when tracks are expected but unavailable, preventing cache of empty results - Fetch full track details in batches of 50 to reduce timeout risk - Retry get_tracks once on NetworkError in api_client - Raise ResourceTemporarilyUnavailable on NetworkError in get_playlist to prevent caching None as empty result - Suppress yandex_music library DEBUG logs to avoid huge API dumps Co-authored-by: Cursor * chore: apply ruff fixes and update snapshot - Remove unused noqa comment (RUF100) in provider.py - Update test_parsers snapshot for upstream model changes (year/style fields) Co-authored-by: Cursor * fix(yandex_music): fix missing album cover art in library - In parse_track(), use parse_album() instead of ItemMapping for the track's album reference so albums are created with full metadata (including cover_uri) when tracks are synced to the library - In get_liked_albums(), fetch full album details via client.albums() in batches of 50, since the users_likes_albums endpoint returns minimal data without cover_uri Co-authored-by: Cursor * chore(yandex_music): regenerate parser snapshots after models bump Co-authored-by: Cursor * Update music_assistant/providers/yandex_music/api_client.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update music_assistant/providers/yandex_music/provider.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Add tests for api_client batching/retry and playlist edge cases Cover the 3 unresolved Copilot review comments from PR #3099: - get_liked_albums() batch fetching and NetworkError fallback - get_tracks() retry-on-NetworkError logic - get_playlist_tracks() page>0, fetch_tracks_async fallback, empty batch error Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: Михаил Невский Co-authored-by: Cursor Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 --- diff --git a/music_assistant/providers/yandex_music/api_client.py b/music_assistant/providers/yandex_music/api_client.py index e1ba5948..5293068a 100644 --- a/music_assistant/providers/yandex_music/api_client.py +++ b/music_assistant/providers/yandex_music/api_client.py @@ -99,16 +99,44 @@ class YandexMusicClient: raise ResourceTemporarilyUnavailable("Failed to fetch liked tracks") from err async def get_liked_albums(self) -> list[YandexAlbum]: - """Get user's liked albums. + """Get user's liked albums with full details (including cover art). - :return: List of liked album objects. + The users_likes_albums endpoint returns minimal album data without + cover_uri, so we fetch full album details in batches afterwards. + + :return: List of liked album objects with full details. """ client = self._ensure_connected() try: result = await client.users_likes_albums() if result is None: return [] - return [like.album for like in result if like.album is not None] + album_ids = [ + str(like.album.id) for like in result if like.album is not None and like.album.id + ] + if not album_ids: + return [] + # Fetch full album details in batches to get cover_uri and other metadata + batch_size = 50 + full_albums: list[YandexAlbum] = [] + for i in range(0, len(album_ids), batch_size): + batch = album_ids[i : i + batch_size] + try: + batch_result = await client.albums(batch) + if batch_result: + full_albums.extend(batch_result) + except (BadRequestError, NetworkError) as batch_err: + LOGGER.warning("Error fetching album details batch: %s", batch_err) + # Fall back to minimal data for this batch + batch_set = set(batch) + for like in result: + if ( + like.album is not None + and like.album.id + and str(like.album.id) in batch_set + ): + full_albums.append(like.album) + return full_albums except (BadRequestError, NetworkError) as err: LOGGER.error("Error fetching liked albums: %s", err) raise ResourceTemporarilyUnavailable("Failed to fetch liked albums") from err @@ -186,12 +214,22 @@ class YandexMusicClient: :param track_ids: List of track IDs. :return: List of track objects. + :raises ResourceTemporarilyUnavailable: On network errors after retry. """ client = self._ensure_connected() try: result = await client.tracks(track_ids) return result or [] - except (BadRequestError, NetworkError) as err: + except NetworkError as err: + # Retry once on network errors (timeout, disconnect, etc.) + LOGGER.warning("Network error fetching tracks, retrying once: %s", err) + try: + result = await client.tracks(track_ids) + return result or [] + except NetworkError as retry_err: + LOGGER.error("Error fetching tracks (retry failed): %s", retry_err) + raise ResourceTemporarilyUnavailable("Failed to fetch tracks") from retry_err + except BadRequestError as err: LOGGER.error("Error fetching tracks: %s", err) return [] @@ -292,6 +330,7 @@ class YandexMusicClient: :param user_id: User ID (owner of the playlist). :param playlist_id: Playlist ID (kind). :return: Playlist object or None if not found. + :raises ResourceTemporarilyUnavailable: On network errors. """ client = self._ensure_connected() try: @@ -299,7 +338,10 @@ class YandexMusicClient: if isinstance(result, list): return result[0] if result else None return result - except (BadRequestError, NetworkError) as err: + except NetworkError as err: + LOGGER.warning("Network error fetching playlist %s/%s: %s", user_id, playlist_id, err) + raise ResourceTemporarilyUnavailable("Failed to fetch playlist") from err + except BadRequestError as err: LOGGER.error("Error fetching playlist %s/%s: %s", user_id, playlist_id, err) return None diff --git a/music_assistant/providers/yandex_music/parsers.py b/music_assistant/providers/yandex_music/parsers.py index 7c6d5202..c72728f0 100644 --- a/music_assistant/providers/yandex_music/parsers.py +++ b/music_assistant/providers/yandex_music/parsers.py @@ -253,17 +253,13 @@ def parse_track(provider: YandexMusicProvider, track_obj: YandexTrack) -> Track: for artist in track_obj.artists: track.artists.append(parse_artist(provider, artist)) - # Parse album (minimal data) + # Parse album (full data so album gets cover art in the library) if track_obj.albums and len(track_obj.albums) > 0: - album = track_obj.albums[0] - track.album = provider.get_item_mapping( - media_type="album", - key=str(album.id), - name=album.title or "Unknown Album", - ) - # Get image from album if available - if album.cover_uri: - image_url = _get_image_url(album.cover_uri) + album_obj = track_obj.albums[0] + track.album = parse_album(provider, album_obj) + # Also set track image from album cover if available + if album_obj.cover_uri: + image_url = _get_image_url(album_obj.cover_uri) if image_url: track.metadata.images = UniqueList( [ diff --git a/music_assistant/providers/yandex_music/provider.py b/music_assistant/providers/yandex_music/provider.py index 55a1b474..21bc07a6 100644 --- a/music_assistant/providers/yandex_music/provider.py +++ b/music_assistant/providers/yandex_music/provider.py @@ -2,6 +2,7 @@ from __future__ import annotations +import logging from typing import TYPE_CHECKING from music_assistant_models.enums import MediaType @@ -10,6 +11,7 @@ from music_assistant_models.errors import ( LoginFailed, MediaNotFoundError, ProviderUnavailableError, + ResourceTemporarilyUnavailable, ) from music_assistant_models.media_items import ( Album, @@ -63,6 +65,8 @@ class YandexMusicProvider(MusicProvider): self._client = YandexMusicClient(str(token)) await self._client.connect() + # Suppress yandex_music library DEBUG dumps (full API request/response JSON) + logging.getLogger("yandex_music").setLevel(self.logger.level + 10) self._streaming = YandexMusicStreamingManager(self) self.logger.info("Successfully connected to Yandex Music") @@ -254,6 +258,11 @@ class YandexMusicProvider(MusicProvider): :param page: Page number for pagination. :return: List of Track objects. """ + # Yandex Music API returns all playlist tracks in one call (no server-side pagination). + # Return empty list for page > 0 so the controller pagination loop terminates. + if page > 0: + return [] + # Parse the playlist ID (format: owner_id:kind) if PLAYLIST_ID_SPLITTER in prov_playlist_id: owner_id, kind = prov_playlist_id.split(PLAYLIST_ID_SPLITTER, 1) @@ -262,21 +271,62 @@ class YandexMusicProvider(MusicProvider): kind = prov_playlist_id playlist = await self.client.get_playlist(owner_id, kind) - if not playlist or not playlist.tracks: + if not playlist: + return [] + + # API sometimes returns playlist without tracks; fetch them explicitly if needed + tracks_list = playlist.tracks or [] + track_count = getattr(playlist, "track_count", None) or 0 + if not tracks_list and track_count > 0: + self.logger.debug( + "Playlist %s/%s: track_count=%s but no tracks in response, " + "calling fetch_tracks_async", + owner_id, + kind, + track_count, + ) + try: + tracks_list = await playlist.fetch_tracks_async() + except Exception as err: + self.logger.warning("fetch_tracks_async failed for %s/%s: %s", owner_id, kind, err) + if not tracks_list: + raise ResourceTemporarilyUnavailable( + "Playlist tracks not available; try again later" + ) + + if not tracks_list: return [] # Yandex returns TrackShort objects, we need to fetch full track info track_ids = [ str(track.track_id) if hasattr(track, "track_id") else str(track.id) - for track in playlist.tracks + for track in tracks_list if track ] - if not track_ids: return [] - # Fetch full track details - full_tracks = await self.client.get_tracks(track_ids) + # Fetch full track details in batches to avoid timeouts + batch_size = 50 + full_tracks = [] + for i in range(0, len(track_ids), batch_size): + batch = track_ids[i : i + batch_size] + batch_result = await self.client.get_tracks(batch) + if not batch_result: + self.logger.warning( + "Received empty result for playlist %s tracks batch %s-%s", + prov_playlist_id, + i, + i + len(batch) - 1, + ) + raise ResourceTemporarilyUnavailable( + "Playlist tracks not fully available; try again later" + ) + full_tracks.extend(batch_result) + + if track_ids and not full_tracks: + raise ResourceTemporarilyUnavailable("Failed to load track details; try again later") + tracks = [] for track in full_tracks: try: diff --git a/tests/providers/yandex_music/__snapshots__/test_parsers.ambr b/tests/providers/yandex_music/__snapshots__/test_parsers.ambr index 3ef6a077..f4e45fce 100644 --- a/tests/providers/yandex_music/__snapshots__/test_parsers.ambr +++ b/tests/providers/yandex_music/__snapshots__/test_parsers.ambr @@ -390,15 +390,69 @@ # name: test_parse_track_snapshot[with_artist_and_album] dict({ 'album': dict({ - 'available': True, + 'album_type': 'album', + 'artists': list([ + ]), + 'date_added': None, 'external_ids': list([ ]), - 'image': None, + 'favorite': False, 'is_playable': True, 'item_id': '20', 'media_type': 'album', + 'metadata': dict({ + 'chapters': None, + 'copyright': None, + 'description': None, + 'explicit': None, + 'genres': None, + 'grouping': None, + 'images': list([ + dict({ + 'path': 'https://avatars.yandex.net/get-music-content/aaa/bbb/1000x1000', + 'provider': 'yandex_music_instance', + 'remotely_accessible': True, + 'type': 'thumb', + }), + ]), + 'label': None, + 'languages': None, + 'last_refresh': None, + 'links': None, + 'lrc_lyrics': None, + 'lyrics': None, + 'mood': None, + 'performers': None, + 'popularity': None, + 'preview': None, + 'release_date': None, + 'review': None, + 'style': None, + }), 'name': 'Track Album', + 'position': None, 'provider': 'yandex_music_instance', + 'provider_mappings': list([ + dict({ + 'audio_format': dict({ + 'bit_depth': 16, + 'bit_rate': 0, + 'channels': 2, + 'codec_type': '?', + 'content_type': '?', + 'output_format_str': '?', + 'sample_rate': 44100, + }), + 'available': False, + 'details': None, + 'in_library': None, + 'is_unique': None, + 'item_id': '20', + 'provider_domain': 'yandex_music', + 'provider_instance': 'yandex_music_instance', + 'url': 'https://music.yandex.ru/album/20', + }), + ]), 'sort_name': 'track album', 'translation_key': None, 'uri': 'yandex_music_instance://album/20', diff --git a/tests/providers/yandex_music/test_api_client.py b/tests/providers/yandex_music/test_api_client.py new file mode 100644 index 00000000..43e97ba0 --- /dev/null +++ b/tests/providers/yandex_music/test_api_client.py @@ -0,0 +1,103 @@ +"""Unit tests for YandexMusicClient (api_client.py).""" + +from __future__ import annotations + +from unittest import mock + +import pytest +from music_assistant_models.errors import ResourceTemporarilyUnavailable +from yandex_music.exceptions import NetworkError + +from music_assistant.providers.yandex_music.api_client import YandexMusicClient + + +def _make_client() -> tuple[YandexMusicClient, mock.AsyncMock]: + """Create a YandexMusicClient with a mocked underlying ClientAsync. + + :return: Tuple of (YandexMusicClient, mock_underlying_client). + """ + client = YandexMusicClient(token="fake_token") + mock_underlying = mock.AsyncMock() + client._client = mock_underlying + client._user_id = 12345 + return client, mock_underlying + + +# -- get_liked_albums: batching ------------------------------------------------- + + +async def test_get_liked_albums_batching() -> None: + """Albums are fetched in batch via client.albums() for full metadata.""" + client, underlying = _make_client() + + # Build 3 minimal "like" objects with album stubs (no cover_uri) + likes = [] + for album_id in (1, 2, 3): + album_stub = type("Album", (), {"id": album_id, "cover_uri": None})() + like = type("Like", (), {"album": album_stub})() + likes.append(like) + + # Full album objects returned by client.albums() + full_albums = [ + type("Album", (), {"id": aid, "cover_uri": f"cover_{aid}"})() for aid in (1, 2, 3) + ] + + underlying.users_likes_albums = mock.AsyncMock(return_value=likes) + underlying.albums = mock.AsyncMock(return_value=full_albums) + + result = await client.get_liked_albums() + + underlying.albums.assert_awaited_once_with(["1", "2", "3"]) + assert result == full_albums + assert all(a.cover_uri is not None for a in result) + + +async def test_get_liked_albums_batch_fallback_on_network_error() -> None: + """When client.albums() fails, fallback returns minimal album data from likes.""" + client, underlying = _make_client() + + album_stub_1 = type("Album", (), {"id": 10, "cover_uri": None})() + album_stub_2 = type("Album", (), {"id": 20, "cover_uri": None})() + likes = [ + type("Like", (), {"album": album_stub_1})(), + type("Like", (), {"album": album_stub_2})(), + ] + + underlying.users_likes_albums = mock.AsyncMock(return_value=likes) + underlying.albums = mock.AsyncMock(side_effect=NetworkError("timeout")) + + result = await client.get_liked_albums() + + # Should fall back to the minimal album objects from likes + assert len(result) == 2 + assert {a.id for a in result} == {10, 20} + + +# -- get_tracks: retry on NetworkError ------------------------------------------- + + +async def test_get_tracks_retry_on_network_error_then_success() -> None: + """First call fails with NetworkError; retry succeeds.""" + client, underlying = _make_client() + + track = type("Track", (), {"id": 400, "title": "Test Track"})() + underlying.tracks = mock.AsyncMock(side_effect=[NetworkError("timeout"), [track]]) + + result = await client.get_tracks(["400"]) + + assert result == [track] + assert underlying.tracks.await_count == 2 + + +async def test_get_tracks_retry_on_network_error_both_fail() -> None: + """Both attempts fail with NetworkError → ResourceTemporarilyUnavailable.""" + client, underlying = _make_client() + + underlying.tracks = mock.AsyncMock( + side_effect=[NetworkError("timeout"), NetworkError("timeout again")] + ) + + with pytest.raises(ResourceTemporarilyUnavailable): + await client.get_tracks(["400"]) + + assert underlying.tracks.await_count == 2 diff --git a/tests/providers/yandex_music/test_integration.py b/tests/providers/yandex_music/test_integration.py index 85a4f77a..2150c3f4 100644 --- a/tests/providers/yandex_music/test_integration.py +++ b/tests/providers/yandex_music/test_integration.py @@ -10,6 +10,7 @@ from unittest import mock import pytest from music_assistant_models.enums import ContentType, MediaType, StreamType +from music_assistant_models.errors import ResourceTemporarilyUnavailable from yandex_music import Album as YandexAlbum from yandex_music import Artist as YandexArtist from yandex_music import Playlist as YandexPlaylist @@ -360,3 +361,75 @@ async def test_browse(mass: MusicAssistant) -> None: artists_items = await prov.browse(path=artists_path) assert artists_items is not None assert isinstance(artists_items, (list, tuple)) + + +# -- Playlist edge-case tests -------------------------------------------------- + + +@pytest.mark.usefixtures("yandex_music_provider") +async def test_get_playlist_tracks_page_gt_zero_returns_empty(mass: MusicAssistant) -> None: + """Page > 0 returns empty list (Yandex returns all tracks in one call).""" + prov = _get_yandex_provider(mass) + assert prov is not None + # Use a different playlist ID to avoid cache collision with test_get_playlist_tracks + result = await prov.get_playlist_tracks("12345:99", page=1) + assert result == [] + + +@pytest.mark.usefixtures("yandex_music_provider") +async def test_get_playlist_tracks_fetch_tracks_async_fallback(mass: MusicAssistant) -> None: + """When playlist.tracks is None but track_count > 0, fetch_tracks_async is used.""" + prov = _get_yandex_provider(mass) + assert prov is not None + + _, _, track, _ = _load_yandex_objects() + + # Build a playlist object with tracks=None and track_count=5 + track_short = type("TrackShort", (), {"track_id": 400, "id": 400})() + playlist_no_tracks = type( + "Playlist", + (), + { + "owner": type("Owner", (), {"uid": 12345})(), + "kind": 77, + "title": "Fallback Playlist", + "tracks": None, + "track_count": 5, + "fetch_tracks_async": mock.AsyncMock(return_value=[track_short]), + }, + )() + + prov.client.get_playlist = mock.AsyncMock(return_value=playlist_no_tracks) # type: ignore[attr-defined] + prov.client.get_tracks = mock.AsyncMock(return_value=[track]) # type: ignore[attr-defined] + + result = await prov.get_playlist_tracks("12345:77", page=0) + assert isinstance(result, list) + assert len(result) >= 1 + playlist_no_tracks.fetch_tracks_async.assert_awaited_once() + + +@pytest.mark.usefixtures("yandex_music_provider") +async def test_get_playlist_tracks_empty_batch_raises(mass: MusicAssistant) -> None: + """Empty batch result from get_tracks raises ResourceTemporarilyUnavailable.""" + prov = _get_yandex_provider(mass) + assert prov is not None + + # Build a playlist with tracks that have track_ids + track_short = type("TrackShort", (), {"track_id": 400, "id": 400})() + playlist_with_tracks = type( + "Playlist", + (), + { + "owner": type("Owner", (), {"uid": 12345})(), + "kind": 88, + "title": "Batch Fail Playlist", + "tracks": [track_short], + "track_count": 1, + }, + )() + + prov.client.get_playlist = mock.AsyncMock(return_value=playlist_with_tracks) # type: ignore[attr-defined] + prov.client.get_tracks = mock.AsyncMock(return_value=[]) # type: ignore[attr-defined] + + with pytest.raises(ResourceTemporarilyUnavailable): + await prov.get_playlist_tracks("12345:88", page=0)