From b0018db89d0ca6ac55e028b1f3376e17329da03b Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Mon, 30 Mar 2026 14:29:04 +0100 Subject: [PATCH] Improve error handling for color extraction Includes a around embedding generation, which was happening twice (the first call was redundant as record embeddings are always regenerated after their parent artist embeddings are regenerated). Closes #144. --- lib/music_library/records.ex | 25 +++++++++++++++++++------ lib/music_library_web/error_messages.ex | 1 + priv/gettext/default.pot | 5 +++++ priv/gettext/en/LC_MESSAGES/default.po | 5 +++++ test/music_library/records_test.exs | 7 +++++++ 5 files changed, 37 insertions(+), 6 deletions(-) diff --git a/lib/music_library/records.ex b/lib/music_library/records.ex index fa616726..0cfc0bda 100644 --- a/lib/music_library/records.ex +++ b/lib/music_library/records.ex @@ -3,6 +3,8 @@ defmodule MusicLibrary.Records do Provides function to work with records _irrespectively_ of their status as port of the collection or of the wishlist. """ + require Logger + import Ecto.Query, warn: false alias MusicLibrary.Artists @@ -303,15 +305,28 @@ defmodule MusicLibrary.Records do enqueue_worker(Worker.RefreshCover, %{"id" => record.id}, record_meta(record)) end + defp best_effort_extract_colors(record) do + case maybe_extract_colors(record) do + {:ok, record} -> + record + + {:error, reason} -> + Logger.warning("Color extraction failed for record #{record.id}: #{inspect(reason)}") + record + end + end + defp maybe_extract_colors(%{dominant_colors: [_ | _]} = record), do: {:ok, record} defp maybe_extract_colors(record), do: extract_colors(record) @spec extract_colors(Record.t()) :: {:ok, Record.t()} | {:error, term()} def extract_colors(record) do - asset = Assets.get!(record.cover_hash) - - with {:ok, colors} <- @color_extractor.extract_dominant_colors(asset.content) do + with asset when not is_nil(asset) <- Assets.get(record.cover_hash), + {:ok, colors} <- @color_extractor.extract_dominant_colors(asset.content) do update_record(record, %{dominant_colors: colors}) + else + nil -> {:error, :asset_not_found} + error -> error end end @@ -392,10 +407,8 @@ defmodule MusicLibrary.Records do @spec create_record(map()) :: {:ok, Record.t()} | {:error, Ecto.Changeset.t()} def create_record(attrs \\ %{}) do with {:ok, record} <- do_create_record(attrs) do - {:ok, record} = maybe_extract_colors(record) - generate_embedding_async(record) - record + |> best_effort_extract_colors() |> Record.artist_ids() |> Enum.each(fn artist_id -> Artists.refresh_artist_info_async(artist_id) diff --git a/lib/music_library_web/error_messages.ex b/lib/music_library_web/error_messages.ex index 5490b4cb..9f37a0c6 100644 --- a/lib/music_library_web/error_messages.ex +++ b/lib/music_library_web/error_messages.ex @@ -21,6 +21,7 @@ defmodule MusicLibraryWeb.ErrorMessages do do: gettext("this record is already in your collection") def friendly_message(:not_found), do: gettext("the resource was not found") + def friendly_message(:asset_not_found), do: gettext("the cover image asset was not found") def friendly_message(:no_discogs_data), do: gettext("no Discogs profile available") def friendly_message(:image_not_found), do: gettext("no image could be found") def friendly_message(:invalid_parameters), do: gettext("invalid parameters were provided") diff --git a/priv/gettext/default.pot b/priv/gettext/default.pot index 10b1b3d3..d62516cc 100644 --- a/priv/gettext/default.pot +++ b/priv/gettext/default.pot @@ -2375,3 +2375,8 @@ msgstr "" #, elixir-autogen, elixir-format msgid "Scan a record" msgstr "" + +#: lib/music_library_web/error_messages.ex +#, elixir-autogen, elixir-format +msgid "the cover image asset was not found" +msgstr "" diff --git a/priv/gettext/en/LC_MESSAGES/default.po b/priv/gettext/en/LC_MESSAGES/default.po index 3abbb9f1..7429091f 100644 --- a/priv/gettext/en/LC_MESSAGES/default.po +++ b/priv/gettext/en/LC_MESSAGES/default.po @@ -2375,3 +2375,8 @@ msgstr "" #, elixir-autogen, elixir-format msgid "Scan a record" msgstr "" + +#: lib/music_library_web/error_messages.ex +#, elixir-autogen, elixir-format +msgid "the cover image asset was not found" +msgstr "" diff --git a/test/music_library/records_test.exs b/test/music_library/records_test.exs index 4ee88537..e1260409 100644 --- a/test/music_library/records_test.exs +++ b/test/music_library/records_test.exs @@ -51,6 +51,13 @@ defmodule MusicLibrary.RecordsTest do assert Enum.all?(record.dominant_colors, &color_hex?/1) end + @tag :capture_log + test "succeeds when color extraction fails" do + record = record(dominant_colors: [], cover_hash: "nonexistent_hash") + + assert record.dominant_colors == [] + end + test "queues a task to retrieve artist info data" do record = record(musicbrainz_data: release_group(:lockdown_trilogy))