Repository: beetbox/beets
Pull request: Improve ListenBrainz error handling and simplify playlist handling
## Description

Fixes #5459

## To Do
- [X] Changelog. (Add an entry to `docs/changelog.rst` to the bottom of one of the lists near the top of the document.)
- [ ] Tests. (Very much encouraged but not strictly required.)


Problem statement:
Listenbrainz broken - AttributeError
Listenbrainz is currently broken. I get this error:

```bash
Traceback (most recent call last):
  File "/home/arsaboo/.local/bin/beet", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/home/arsaboo/.local/lib/python3.12/site-packages/beets/ui/__init__.py", line 1862, in main
    _raw_main(args)
  File "/home/arsaboo/.local/lib/python3.12/site-packages/beets/ui/__init__.py", line 1849, in _raw_main
    subcommand.func(lib, suboptions, subargs)
  File "/home/arsaboo/.local/lib/python3.12/site-packages/beetsplug/plexsync.py", line 342, in func_playlist_import
    self._plex_import_playlist(opts.playlist, opts.url, opts.listenbrainz)
  File "/home/arsaboo/.local/lib/python3.12/site-packages/beetsplug/plexsync.py", line 947, in _plex_import_playlist
    weekly_jams = lb.get_weekly_jams()
                  ^^^^^^^^^^^^^^^^^^^^
  File "/home/arsaboo/.local/lib/python3.12/site-packages/beetsplug/listenbrainz.py", line 258, in get_weekly_jams
    return self.get_weekly_playlist(1)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/arsaboo/.local/lib/python3.12/site-packages/beetsplug/listenbrainz.py", line 250, in get_weekly_playlist
    return self.get_tracks_from_playlist(playlist)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/arsaboo/.local/lib/python3.12/site-packages/beetsplug/listenbrainz.py", line 205, in get_tracks_from_playlist
    "identifier": track.get("identifier").split("/")[-1],
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'list' object has no attribute 'split'
```

I will submit a PR soon to fix this.

Hints from issue discussion:

Commit message: Update changelog.rst

Patch to review:
diff --git a/beetsplug/listenbrainz.py b/beetsplug/listenbrainz.py
index 4855481f86..d1efd2275e 100644
--- a/beetsplug/listenbrainz.py
+++ b/beetsplug/listenbrainz.py
@@ -116,13 +116,9 @@ def get_tracks_from_listens(self, listens):
                 mbid = self.get_mb_recording_id(track)
             tracks.append(
                 {
-                    "album": {
-                        "name": track["track_metadata"].get("release_name")
-                    },
+                    "album": {"name": track["track_metadata"].get("release_name")},
                     "name": track["track_metadata"].get("track_name"),
-                    "artist": {
-                        "name": track["track_metadata"].get("artist_name")
-                    },
+                    "artist": {"name": track["track_metadata"].get("artist_name")},
                     "mbid": mbid,
                     "release_mbid": mbid_mapping.get("release_mbid"),
                     "listened_at": track.get("listened_at"),
@@ -148,9 +144,6 @@ def get_playlists_createdfor(self, username):
         return self._make_request(url)
 
     def get_listenbrainz_playlists(self):
-        """Returns a list of playlists created by ListenBrainz."""
-        import re
-
         resp = self.get_playlists_createdfor(self.username)
         playlists = resp.get("playlists")
         listenbrainz_playlists = []
@@ -159,35 +152,24 @@ def get_listenbrainz_playlists(self):
             playlist_info = playlist.get("playlist")
             if playlist_info.get("creator") == "listenbrainz":
                 title = playlist_info.get("title")
-                match = re.search(
-                    r"(Missed Recordings of \d{4}|Discoveries of \d{4})", title
-                )
-                if "Exploration" in title:
-                    playlist_type = "Exploration"
-                elif "Jams" in title:
-                    playlist_type = "Jams"
-                elif match:
-                    playlist_type = match.group(1)
-                else:
-                    playlist_type = None
-                if "week of " in title:
+                self._log.debug(f"Playlist title: {title}")
+                playlist_type = "Exploration" if "Exploration" in title else "Jams"
+                if "week of" in title:
                     date_str = title.split("week of ")[1].split(" ")[0]
-                    date = datetime.datetime.strptime(
-                        date_str, "%Y-%m-%d"
-                    ).date()
+                    date = datetime.datetime.strptime(date_str, "%Y-%m-%d").date()
                 else:
-                    date = None
+                    continue
                 identifier = playlist_info.get("identifier")
                 id = identifier.split("/")[-1]
-                if playlist_type in ["Jams", "Exploration"]:
-                    listenbrainz_playlists.append(
-                        {
-                            "type": playlist_type,
-                            "date": date,
-                            "identifier": id,
-                            "title": title,
-                        }
-                    )
+                listenbrainz_playlists.append(
+                    {"type": playlist_type, "date": date, "identifier": id}
+                )
+        listenbrainz_playlists = sorted(listenbrainz_playlists, key=lambda x: x["type"])
+        listenbrainz_playlists = sorted(
+            listenbrainz_playlists, key=lambda x: x["date"], reverse=True
+        )
+        for playlist in listenbrainz_playlists:
+            self._log.debug(f'Playlist: {playlist["type"]} - {playlist["date"]}')
         return listenbrainz_playlists
 
     def get_playlist(self, identifier):
@@ -199,17 +181,21 @@ def get_tracks_from_playlist(self, playlist):
         """This function returns a list of tracks in the playlist."""
         tracks = []
         for track in playlist.get("playlist").get("track"):
-            tracks.append(
-                {
-                    "artist": track.get("creator"),
-                    "identifier": track.get("identifier").split("/")[-1],
-                    "title": track.get("title"),
-                }
-            )
+            identifier = track.get("identifier")
+            if isinstance(identifier, list):
+                identifier = identifier[0]
+            artist = track.get(
+                "creator", "Unknown artist"
+            )  # Set a default value if 'creator' key is not present
+            track_dict = {
+                "artist": artist,
+                "identifier": identifier.split("/")[-1],
+                "title": track.get("title"),
+            }
+            tracks.append(track_dict)
         return self.get_track_info(tracks)
 
     def get_track_info(self, tracks):
-        """Returns a list of track info."""
         track_info = []
         for track in tracks:
             identifier = track.get("identifier")
@@ -242,25 +228,32 @@ def get_track_info(self, tracks):
             )
         return track_info
 
-    def get_weekly_playlist(self, index):
-        """Returns a list of weekly playlists based on the index."""
+    def get_weekly_playlist(self, playlist_type, most_recent=True):
+        # Fetch all playlists
         playlists = self.get_listenbrainz_playlists()
-        playlist = self.get_playlist(playlists[index].get("identifier"))
-        self._log.info(f"Getting {playlist.get('playlist').get('title')}")
+        # Filter playlists by type
+        filtered_playlists = [p for p in playlists if p["type"] == playlist_type]
+        # Sort playlists by date in descending order
+        sorted_playlists = sorted(
+            filtered_playlists, key=lambda x: x["date"], reverse=True
+        )
+        # Select the most recent or older playlist based on the most_recent flag
+        selected_playlist = sorted_playlists[0] if most_recent else sorted_playlists[1]
+        self._log.debug(
+            f"Selected playlist: {selected_playlist['type']} - {selected_playlist['date']}"
+        )
+        # Fetch and return tracks from the selected playlist
+        playlist = self.get_playlist(selected_playlist.get("identifier"))
         return self.get_tracks_from_playlist(playlist)
 
     def get_weekly_exploration(self):
-        """Returns a list of weekly exploration."""
-        return self.get_weekly_playlist(0)
+        return self.get_weekly_playlist("Exploration", most_recent=True)
 
     def get_weekly_jams(self):
-        """Returns a list of weekly jams."""
-        return self.get_weekly_playlist(1)
+        return self.get_weekly_playlist("Jams", most_recent=True)
 
     def get_last_weekly_exploration(self):
-        """Returns a list of weekly exploration."""
-        return self.get_weekly_playlist(3)
+        return self.get_weekly_playlist("Exploration", most_recent=False)
 
     def get_last_weekly_jams(self):
-        """Returns a list of weekly jams."""
-        return self.get_weekly_playlist(3)
+        return self.get_weekly_playlist("Jams", most_recent=False)
diff --git a/docs/changelog.rst b/docs/changelog.rst
index a2e15dd317..b159e9bb63 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -253,6 +253,8 @@ New features:
 
 Bug fixes:
 
+* Improve ListenBrainz error handling.
+  :bug:`5459`
 * :doc:`/plugins/deezer`: Improve requests error handling.
 * :doc:`/plugins/lastimport`: Improve error handling in the `process_tracks` function and enable it to be used with other plugins.
 * :doc:`/plugins/spotify`: Improve handling of ConnectionError.


Code review comments:
diff_hunk: @@ -199,17 +181,21 @@ def get_tracks_from_playlist(self, playlist):
         """This function returns a list of tracks in the playlist."""
         tracks = []
         for track in playlist.get("playlist").get("track"):
-            tracks.append(
-                {
-                    "artist": track.get("creator"),
-                    "identifier": track.get("identifier").split("/")[-1],
-                    "title": track.get("title"),
-                }
-            )
+            identifier = track.get("identifier")
+            if isinstance(identifier, list):
+                identifier = identifier[0]
+            artist = track.get(
+                "creator", "Unknown artist"
+            )  # Set a default value if 'creator' key is not present
+            track_dict = {
+                "artist": artist,
+                "identifier": identifier.split("/")[-1],
+                "title": track.get("title"),
+            }
+            tracks.append(track_dict)
path: beetsplug/listenbrainz.py
text: @user1:
I find the original implementation slightly cleaner to read. Consider something like below
```suggestion
            identifier = track.get("identifier")
            if isinstance(identifier, list):
                identifier = identifier[0]

            tracks.append(
                {
                    "artist": track.get("creator", "Unknown artist"),
                    "identifier": identifier.split("/")[-1],
                    "title": track.get("title"),
                }
            )
```