One failed search batch no longer strands the albums behind it

CHANGELOG.md

@@ -11,6 +11,12 @@ alone.
 
 ### Fixed
 
+- **One failed search request no longer strands the albums behind it.**
+  Searches go out in batches of a hundred, and a batch that failed raised
+  immediately, so on a 250-album chart a single timed-out request left the
+  last fifty albums unsearched with nothing said about it. Every batch is
+  now attempted, and the summary reports how many albums missed out and
+  why.
 - **`--search` did not start downloads for the first album of each new
   artist.** Adding an album was assumed to search it, via Lidarr's
   `addOptions.searchForNewAlbum`. That flag is read by

chartarr/cli.py

@@ -461,13 +461,14 @@ def events():
         line += f" · failed {accent(counts['failed'])}"
     print(line)
     if args.search and touched:
-        # the only thing that actually starts a download: one AlbumSearch
+        # the only thing that actually starts a download: an AlbumSearch
         # for everything this run added or turned on
-        try:
-            api.search_albums(touched)
-            print(dim(f"asked lidarr to search for {_n(len(touched), 'album')}"))
-        except lidarr.LidarrError as e:
-            print(dim(f"search request failed: {e}"))
+        queued, errors = api.search_albums(touched)
+        if queued:
+            print(dim(f"asked lidarr to search for {_n(queued, 'album')}"))
+        missed = len(touched) - queued
+        if missed:
+            print(dim(f"{_n(missed, 'album')} went unsearched: {errors[0]}"))
     elif touched and not args.dry_run:
         # without --search the albums sit there monitored and idle, which
         # looks like a finished job that downloaded nothing

chartarr/lidarr.py

@@ -235,7 +235,21 @@ def add_album(self, rgid: str, quality_profile_id: int,
         time.sleep(0.2)  # be gentle
         return "added", (created or {}).get("id")
 
-    def search_albums(self, album_ids: list[int]) -> None:
-        for i in range(0, len(album_ids), 100):
-            self._call("command", method="POST",
-                       json={"name": "AlbumSearch", "albumIds": album_ids[i:i + 100]})
+    def search_albums(self, album_ids: list[int]) -> tuple[int, list[str]]:
+        """queue an AlbumSearch for these rows, in batches of 100.
+
+        returns (queued, errors). a batch that fails does not stop the
+        rest: on a long chart the last fifty albums shouldn't go unsearched
+        because one request in the middle timed out.
+        """
+        ids = [i for i in album_ids if i is not None]
+        queued, errors = 0, []
+        for i in range(0, len(ids), 100):
+            chunk = ids[i:i + 100]
+            try:
+                self._call("command", method="POST",
+                           json={"name": "AlbumSearch", "albumIds": chunk})
+                queued += len(chunk)
+            except LidarrError as e:
+                errors.append(str(e))
+        return queued, errors

tests/test_cli.py

@@ -380,6 +380,7 @@ def add_album(self, rgid, qp, mp, rf, also_monitor=None):
 
     def search_albums(self, album_ids):
         self.searched.extend(album_ids)
+        return len(album_ids), []
 
 
 def _push(monkeypatch, tmp_path, outcomes, **flags):
@@ -446,6 +447,24 @@ def test_every_pushed_album_is_searched_exactly_once(
     assert len(api.searched) == len(set(api.searched))
 
 
+def test_a_partly_failed_search_says_how_many_were_missed(
+        monkeypatch, tmp_path, capsys):
+    api = _FakeApi({"rg-1": ("added", 1), "rg-2": ("added", 2)})
+    api.search_albums = lambda ids: (1, ["HTTP 500: boom"])
+    monkeypatch.setattr(cli.lidarr, "Lidarr", lambda url, key: api)
+    monkeypatch.setattr(cli, "_screen_ok", lambda: False)
+    items = [{"key": "k1", "row": {"artist": "a", "title": "t"}, "rgid": "rg-1",
+              "artist_mbid": "am"},
+             {"key": "k2", "row": {"artist": "b", "title": "u"}, "rgid": "rg-2",
+              "artist_mbid": "am"}]
+    args = cli.build_parser().parse_args(["chart.csv", "--search"])
+    cli.stage_push(items, "artist", "title", args,
+                   {"lidarr_url": "http://l:8686", "api_key": "k"})
+    out = capsys.readouterr().out
+    assert "search for 1 album" in out
+    assert "1 album went unsearched" in out and "boom" in out
+
+
 def test_without_search_the_summary_says_nothing_was_searched(
         monkeypatch, tmp_path, capsys):
     # monitored-but-idle looks like a finished job that downloaded nothing

tests/test_lidarr.py

@@ -243,13 +243,40 @@ def test_find_album_raises_on_5xx():
 def test_search_albums_chunks_by_100_and_loses_nothing():
     responses.add(responses.POST, API + "/command", json={"id": 1}, status=201)
     ids = list(range(250))
-    api().search_albums(ids)
+    assert api().search_albums(ids) == (250, [])
     bodies = [json.loads(c.request.body) for c in responses.calls]
     assert [b["name"] for b in bodies] == ["AlbumSearch"] * 3
     assert [len(b["albumIds"]) for b in bodies] == [100, 100, 50]
     assert [i for b in bodies for i in b["albumIds"]] == ids
 
 
+@responses.activate
+def test_one_failed_batch_does_not_strand_the_rest():
+    # a 250-album chart is three requests; the middle one failing used to
+    # raise and leave the last fifty albums unsearched and unmentioned
+    responses.add(responses.POST, API + "/command", json={"id": 1}, status=201)
+    responses.add(responses.POST, API + "/command", body="boom", status=500)
+    responses.add(responses.POST, API + "/command", json={"id": 3}, status=201)
+    queued, errors = api().search_albums(list(range(250)))
+    assert queued == 150          # the first and third batches
+    assert len(errors) == 1
+    assert len(responses.calls) == 3  # the third was still attempted
+
+
+@responses.activate
+def test_every_batch_failing_is_reported_not_raised():
+    responses.add(responses.POST, API + "/command", body="boom", status=500)
+    queued, errors = api().search_albums([1, 2, 3])
+    assert queued == 0
+    assert len(errors) == 1
+
+
+def test_search_albums_with_nothing_to_do_makes_no_requests():
+    # add_album returns None for the id when lidarr doesn't send one back
+    assert api().search_albums([]) == (0, [])
+    assert api().search_albums([None, None]) == (0, [])
+
+
 @responses.activate
 def test_wrong_api_key_says_where_to_find_the_right_one():
     responses.add(responses.GET, API + "/system/status",