fix: index albums through the endpoint Lidarr does not throw from

Indexing fetched albums one artist at a time, and `GET /api/v1/album?artistId=`
is Lidarr's unguarded path. It maps straight from the album service with no
hydration: the mapper then dereferences model.Images and model.SecondaryTypes
without a null check, follows model.Artist?.Value where only the first link is
guarded, and selects the monitored release with SingleOrDefault, which throws
outright when an album has two of them. Any of those is a 500 that aborts the
whole index.

The unfiltered `GET /api/v1/album` builds its own artist and release lookups and
skips an album whose metadata is missing rather than dereferencing it. Use that
instead, once, and group by artistId locally. It is the defensive path and it
costs N fewer requests.

Tracks and files have no unfiltered endpoint -- Lidarr rejects a call with no
filter at all -- so those stay per artist. A failure on one artist now skips
that artist rather than ending the run, but the count is recorded in the store
and the coverage report leads with it: a missing artist makes their played music
look unplayed, which is precisely the error that costs music later, so an
incomplete index must not be culled against.

Errors now carry the request URL and whatever the server put in the body. The
original report of this failure was "album: HTTP 500", which points at the URL
and the credentials -- neither of which was at fault.

README.md

@@ -42,6 +42,21 @@ It leans towards collapsing too much. A false match makes something look
 played; a missed match makes something look abandoned. Only one of those
 deletes music.
 
+### Indexing quirks
+
+Albums are fetched from the **unfiltered** `GET /api/v1/album`, not one call per
+artist. `?artistId=` is Lidarr's unguarded path: it dereferences the album's
+artist metadata with no null check and picks the monitored release with
+`SingleOrDefault`, so it returns a 500 for an album with broken metadata or two
+monitored releases. The unfiltered endpoint skips such albums instead, and costs
+N fewer requests.
+
+Tracks and files have no unfiltered endpoint — Lidarr rejects a call with no
+filter — so they stay per artist. If one artist cannot be served, that artist is
+skipped and the run continues, but the count is recorded and the coverage report
+says so loudly. A missing artist makes their played music look cold, so an
+incomplete index must never be culled against.
+
 ### Reading the coverage report
 
 Matched against unmatched is the wrong comparison — most unmatched listening is

music_curator.py

@@ -665,13 +665,36 @@ class Lidarr:
         try:
             body = self.transport(url, timeout=self.timeout, headers={"X-Api-Key": self.api_key})
         except urllib.error.HTTPError as error:
-            raise LidarrError(f"{path}: HTTP {error.code}") from error
+            detail = error_detail(error)
+            raise LidarrError(f"GET {url}: HTTP {error.code}{': ' + detail if detail else ''}")
         except (urllib.error.URLError, TimeoutError) as error:
-            raise LidarrError(f"{path}: {error}") from error
+            raise LidarrError(f"GET {url}: {error}") from error
         try:
             return json.loads(body)
         except json.JSONDecodeError as error:
-            raise LidarrError(f"{path}: malformed response") from error
+            raise LidarrError(f"GET {url}: malformed response") from error
+
+
+def error_detail(error, limit=300):
+    """Return whatever the server said about a failure, for the log.
+
+    A bare status code sends you looking in the wrong place; Lidarr puts the
+    actual exception in the body.
+    """
+    try:
+        body = error.read().decode("utf-8", "replace").strip()
+    except Exception:  # noqa: BLE001 - diagnostics must never raise
+        return ""
+    if not body:
+        return ""
+    try:
+        payload = json.loads(body)
+    except json.JSONDecodeError:
+        return body[:limit]
+    for key in ("message", "description", "error"):
+        if payload.get(key):
+            return str(payload[key])[:limit]
+    return body[:limit]
 
 
 def parse_added(value):
@@ -693,6 +716,15 @@ def index_library(client, store):
     """
     artists = client.get("artist")
     artist_rows, album_rows, track_rows = [], [], []
+    skipped = []
+
+    # Albums come back in one unfiltered call rather than one per artist. That
+    # endpoint is the only one Lidarr hydrates defensively -- it skips an album
+    # whose artist metadata is missing, where `?artistId=` dereferences it and
+    # returns a 500 -- and it costs N fewer requests into the bargain.
+    albums_by_artist = {}
+    for album in client.get("album"):
+        albums_by_artist.setdefault(album.get("artistId"), []).append(album)
 
     for artist in artists:
         artist_id = artist["id"]
@@ -708,7 +740,7 @@ def index_library(client, store):
             )
         )
 
-        for album in client.get("album", {"artistId": artist_id}):
+        for album in albums_by_artist.get(artist_id, []):
             album_rows.append(
                 (
                     album["id"],
@@ -720,11 +752,24 @@ def index_library(client, store):
                 )
             )
 
-        # Paths and the date a track landed live on the file, not the track.
+        # Tracks and files have no unfiltered endpoint -- Lidarr rejects a call
-        files = {
+        # with no filter at all -- so these stay per artist. One artist it
-            handle["id"]: handle for handle in client.get("trackfile", {"artistId": artist_id})
+        # cannot serve must not cost the whole index, but it cannot pass
-        }
+        # silently either: their tracks end up absent, and every scrobble of
-        for track in client.get("track", {"artistId": artist_id}):
+        # theirs then reads as unmatched.
+        try:
+            # Paths and the date a track landed live on the file, not the track.
+            files = {
+                handle["id"]: handle
+                for handle in client.get("trackfile", {"artistId": artist_id})
+            }
+            tracks = client.get("track", {"artistId": artist_id})
+        except LidarrError as error:
+            logger.warning("could not index %s: %s", name or artist_id, error)
+            skipped.append(name or str(artist_id))
+            continue
+
+        for track in tracks:
             handle = files.get(track.get("trackFileId") or 0) or {}
             title = track.get("title") or ""
             track_rows.append(
@@ -744,6 +789,7 @@ def index_library(client, store):
             )
 
     store.replace_library(artist_rows, album_rows, track_rows)
+    store.set_state("index_skipped", str(len(skipped)))
     logger.info(
         "library indexed: %d artists, %d albums, %d tracks (%d with files)",
         len(artist_rows),
@@ -751,6 +797,12 @@ def index_library(client, store):
         len(track_rows),
         sum(row[7] for row in track_rows),
     )
+    if skipped:
+        logger.warning(
+            "%d artists could not be indexed, so their tracks are missing: %s",
+            len(skipped),
+            ", ".join(sorted(skipped)[:10]),
+        )
     return len(artist_rows), len(track_rows)
 
 
@@ -817,6 +869,13 @@ def coverage_report(store):
         return
 
     logger.info("--- match coverage ---")
+    skipped = int(store.get_state("index_skipped") or 0)
+    if skipped:
+        logger.warning(
+            "the index is incomplete: %d artists are missing their tracks, so every"
+            " figure below is a floor. Do not cull against it.",
+            skipped,
+        )
     for method in ("mbid", "name", "none"):
         row = store.connection.execute(
             "SELECT COUNT(*) AS pairs, COALESCE(SUM(plays), 0) AS plays"

tests/conftest.py

@@ -1,6 +1,8 @@
+import io
 import json
 import os
 import sys
+import urllib.error
 import urllib.parse
 
 import pytest
@@ -128,7 +130,10 @@ class FakeLidarr:
     exercise the same stitching the real thing does.
     """
 
-    def __init__(self, artists=()):
+    def __init__(self, artists=(), fail=()):
+        # (path, artistId) pairs the fake refuses to serve, standing in for the
+        # 500s Lidarr returns on data it cannot hydrate.
+        self.fail = set(fail)
         self.artists, self.albums, self.tracks, self.files = [], [], [], []
         for artist_index, entry in enumerate(artists, start=1):
             artist_id = artist_index
@@ -190,10 +195,26 @@ class FakeLidarr:
         }
         self.calls.append((path, query))
 
+        artist_id = int(query.get("artistId", 0))
+        if (path, artist_id) in self.fail:
+            raise urllib.error.HTTPError(
+                url, 500, "Internal Server Error", {}, io.BytesIO(b'{"message": "boom"}')
+            )
+
         if path == "artist":
             return json.dumps(self.artists)
-        artist_id = int(query.get("artistId", 0))
         source = {"album": self.albums, "track": self.tracks, "trackfile": self.files}[path]
+        if path == "album" and not artist_id:
+            # Lidarr's unfiltered album endpoint returns the lot.
+            return json.dumps(source)
+        if not artist_id:
+            raise urllib.error.HTTPError(
+                url,
+                400,
+                "Bad Request",
+                {},
+                io.BytesIO(b'{"message": "artistId must be provided"}'),
+            )
         return json.dumps([row for row in source if row["artistId"] == artist_id])
 
 

tests/test_music_curator.py

@@ -500,3 +500,61 @@ def test_matching_is_redone_when_new_scrobbles_arrive(tmp_path):
     music_curator.run_once(client_for(api), store, "lyra", NOW, 0)
 
     assert verdict(store, "Yellowcard", "Transmission Home")[0] == "name"
+
+
+def test_albums_come_from_the_unfiltered_endpoint(tmp_path):
+    """`?artistId=` is Lidarr's unguarded path and 500s on data it cannot
+    hydrate; the unfiltered one skips such albums instead."""
+    api = FakeLidarr(LIBRARY)
+    store = store_at(tmp_path)
+
+    music_curator.index_library(music_curator.Lidarr("http://lidarr", "key", transport=api), store)
+
+    album_calls = [query for path, query in api.calls if path == "album"]
+    assert album_calls == [{}]
+    assert store.scalar("SELECT COUNT(*) FROM lidarr_album") == 3
+
+
+def test_an_artist_lidarr_cannot_serve_does_not_kill_the_index(tmp_path):
+    # Artist 2 is AC/DC in LIBRARY; its track lookup fails.
+    api = FakeLidarr(LIBRARY, fail=[("track", 2)])
+    store = store_at(tmp_path)
+
+    music_curator.index_library(music_curator.Lidarr("http://lidarr", "key", transport=api), store)
+
+    assert store.scalar("SELECT COUNT(*) FROM lidarr_artist") == 3
+    assert store.scalar("SELECT COUNT(*) FROM lidarr_track WHERE artist_id = 2") == 0
+    # The other two artists are indexed in full.
+    assert store.scalar("SELECT COUNT(*) FROM lidarr_track") == 3
+    assert store.get_state("index_skipped") == "1"
+
+
+def test_a_skipped_artist_is_recorded_so_the_report_can_disown_the_numbers(tmp_path):
+    """An incomplete index makes played music look cold. It has to be loud."""
+    api = FakeLidarr(LIBRARY, fail=[("trackfile", 1)])
+    store = store_at(tmp_path)
+
+    music_curator.index_library(music_curator.Lidarr("http://lidarr", "key", transport=api), store)
+    music_curator.match_library(store)
+
+    assert store.get_state("index_skipped") == "1"
+
+
+def test_a_clean_index_records_no_skips(tmp_path):
+    store = indexed(tmp_path, [])
+
+    assert store.get_state("index_skipped") == "0"
+
+
+def test_a_lidarr_error_carries_the_url_and_what_the_server_said():
+    """A bare status code sends you looking at the wrong thing entirely."""
+    api = FakeLidarr(LIBRARY, fail=[("album", 0)])
+    client = music_curator.Lidarr("http://lidarr:8686", "key", transport=api)
+
+    with pytest.raises(music_curator.LidarrError) as raised:
+        client.get("album")
+
+    message = str(raised.value)
+    assert "http://lidarr:8686/api/v1/album" in message
+    assert "HTTP 500" in message
+    assert "boom" in message