Don't suppress errors when fetching artist Last.fm data

Closes #149

lib/music_library/artists.ex

@@ -260,16 +260,12 @@ defmodule MusicLibrary.Artists do
     artist_info = get_artist_info!(artist_id)
     name = get_in(artist_info.musicbrainz_data, ["name"]) || ""
 
-    case LastFm.get_artist_tags(artist_id, name) do
+    with {:ok, tags} <- LastFm.get_artist_tags(artist_id, name) do
-      {:ok, tags} ->
+      tag_names = Enum.map(tags, fn {tag_name, _count} -> tag_name end)
-        tag_names = Enum.map(tags, fn {tag_name, _count} -> tag_name end)
 
-        artist_info
+      artist_info
-        |> ArtistInfo.changeset(%{lastfm_data: %{"tags" => tag_names}})
+      |> ArtistInfo.changeset(%{lastfm_data: %{"tags" => tag_names}})
-        |> Repo.update()
+      |> Repo.update()
-
-      {:error, _reason} ->
-        {:ok, artist_info}
     end
   end
 

lib/music_library/worker/fetch_artist_info.ex

@@ -5,9 +5,8 @@ defmodule MusicLibrary.Worker.FetchArtistInfo do
   def perform(%Oban.Job{args: %{"id" => artist_id}}) do
     with {:ok, _artist_info} <- MusicLibrary.Artists.refresh_artist_info(artist_id),
          {:ok, _artist_info} <- MusicLibrary.Artists.refresh_wikipedia_data(artist_id),
-         {:ok, _artist_info} <- MusicLibrary.Artists.refresh_image(artist_id) do
+         {:ok, _artist_info} <- MusicLibrary.Artists.refresh_image(artist_id),
-      # refresh_lastfm_data returns {:ok, _} even on API errors, so it won't block embeddings
+         {:ok, _artist_info} <- MusicLibrary.Artists.refresh_lastfm_data(artist_id) do
-      MusicLibrary.Artists.refresh_lastfm_data(artist_id)
       MusicLibrary.Records.regenerate_artist_embeddings(artist_id)
     else
       {:error, :no_english_wikipedia} -> {:cancel, :no_english_wikipedia}

test/music_library/worker/fetch_artist_last_fm_data_test.exs

@@ -39,15 +39,13 @@ defmodule MusicLibrary.Worker.FetchArtistLastFmDataTest do
     end
 
     @tag :capture_log
-    test "returns ok when Last.fm returns an error", %{artist_id: artist_id} do
+    test "returns error when Last.fm returns an error", %{artist_id: artist_id} do
       Req.Test.stub(LastFm.API, fn conn ->
         Req.Test.json(conn, %{"error" => 6, "message" => "Artist not found"})
       end)
 
-      assert :ok = perform_job(FetchArtistLastFmData, %{"id" => artist_id})
+      assert {:error, :invalid_parameters} ==
-
+               perform_job(FetchArtistLastFmData, %{"id" => artist_id})
-      artist_info = Artists.get_artist_info!(artist_id)
-      assert ArtistInfo.lastfm_tags(artist_info) == []
     end
 
     test "filters out tags with count below 2", %{artist_id: artist_id} do