fix(artwork): propagate playlist tile failures and dedupe external step

core/artwork/resolve.go

@@ -81,11 +81,9 @@ func resolveAlbum(ctx context.Context, ds model.DataStore, prov external.Provide
 				return res, nil
 			}
 		case pattern == "external":
-			r, path, err := extGate(fromAlbumExternalSource(ctx, *al, prov))
-			if r != nil {
-				return resolution{reader: r, source: "external", sourcePath: path}, nil
-			}
-			if err != nil && !errors.Is(err, model.ErrNotFound) {
+			if res, ok, isErr := resolveExternalStep(extGate, fromAlbumExternalSource(ctx, *al, prov)); ok {
+				return res, nil
+			} else if isErr {
 				extErr = true
 			}
 		case len(imgFiles) > 0:
@@ -139,11 +137,9 @@ func resolveArtist(ctx context.Context, ds model.DataStore, prov external.Provid
 		pattern = strings.TrimSpace(pattern)
 		switch {
 		case pattern == "external":
-			r, path, err := extGate(fromArtistExternalSource(ctx, *ar, prov))
-			if r != nil {
-				return resolution{reader: r, source: "external", sourcePath: path}, nil
-			}
-			if err != nil && !errors.Is(err, model.ErrNotFound) {
+			if res, ok, isErr := resolveExternalStep(extGate, fromArtistExternalSource(ctx, *ar, prov)); ok {
+				return res, nil
+			} else if isErr {
 				extErr = true
 			}
 		case pattern == "image-folder":
@@ -183,9 +179,13 @@ func resolvePlaylist(ctx context.Context, ds model.DataStore, prov external.Prov
 
 	var tiles []image.Image
 	var extErr bool
+	var tileErr error // first internal (non-external) tile failure, e.g. album deleted mid-flight
 	for _, albumID := range albumIDs {
 		res, err := resolveAlbum(ctx, ds, prov, ffm, albumID, extGate)
 		if err != nil {
+			if tileErr == nil {
+				tileErr = err
+			}
 			continue
 		}
 		if res.extError {
@@ -203,10 +203,16 @@ func resolvePlaylist(ctx context.Context, ds model.DataStore, prov external.Prov
 			break
 		}
 	}
+	if len(tiles) == 0 {
+		// A tile-level failure must never resolve as a clean absent: propagate
+		// internal errors, and force extError for external ones.
+		if tileErr != nil {
+			return resolution{}, fmt.Errorf("resolvePlaylist: sampled album art failed: %w", tileErr)
+		}
+		return resolution{extError: extErr}, nil
+	}
 	// Grow to 4 tiles by repeating what we have, mirroring reader_playlist.go's loadTiles.
 	switch len(tiles) {
-	case 0:
-		return resolution{extError: extErr}, nil
 	case 2:
 		tiles = append(tiles, tiles[1], tiles[0])
 	case 3:
@@ -229,6 +235,17 @@ func resolveRadio(ctx context.Context, ds model.DataStore, radioID string) (reso
 	return res, nil
 }
 
+// resolveExternalStep runs an external sourceFunc through extGate, shared by
+// resolveAlbum and resolveArtist. ok reports a hit; extErr reports a
+// non-not-found error (a not-found is a definitive "no", not a failure).
+func resolveExternalStep(extGate extGateFunc, sf func() (io.ReadCloser, string, error)) (res resolution, ok bool, extErr bool) {
+	r, path, err := extGate(sf)
+	if r != nil {
+		return resolution{reader: r, source: "external", sourcePath: path}, true, false
+	}
+	return resolution{}, false, err != nil && !errors.Is(err, model.ErrNotFound)
+}
+
 func resolveEmbedded(ctx context.Context, lib libraryView, ffm ffmpeg.FFmpeg, embedRel string) (resolution, bool) {
 	if embedRel == "" {
 		return resolution{}, false

core/artwork/resolve_test.go

@@ -207,6 +207,54 @@ var _ = Describe("resolveItem", func() {
 			Expect(res.source).To(Equal("folder"))
 			Expect(filepath.ToSlash(res.sourcePath)).To(HaveSuffix("tests/fixtures/artist/an-album/artist.png"))
 		})
+
+		It("sets extError when the external source errors without being not-found", func() {
+			conf.Server.ArtistArtPriority = "external"
+			artistRepo := tests.CreateMockArtistRepo()
+			artistRepo.SetData(model.Artists{{ID: "ar3", Name: "Artist"}})
+			ds.MockedArtist = artistRepo
+			prov.artistImage = func(context.Context, string) (*url.URL, error) {
+				return nil, errors.New("agent timed out")
+			}
+
+			res, err := resolveItem(ctx, ds, prov, ffm, model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar3"}, nil)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(res.reader).To(BeNil())
+			Expect(res.extError).To(BeTrue())
+		})
+
+		It("does not set extError when the external source reports not-found", func() {
+			conf.Server.ArtistArtPriority = "external"
+			artistRepo := tests.CreateMockArtistRepo()
+			artistRepo.SetData(model.Artists{{ID: "ar4", Name: "Artist"}})
+			ds.MockedArtist = artistRepo
+			// prov.artistImage left nil -> fakeExternalProvider returns model.ErrNotFound
+
+			res, err := resolveItem(ctx, ds, prov, ffm, model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar4"}, nil)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(res.reader).To(BeNil())
+			Expect(res.extError).To(BeFalse())
+		})
+
+		It("routes the external step through a custom extGate", func() {
+			conf.Server.ArtistArtPriority = "external"
+			artistRepo := tests.CreateMockArtistRepo()
+			artistRepo.SetData(model.Artists{{ID: "ar5", Name: "Artist"}})
+			ds.MockedArtist = artistRepo
+			prov.artistImage = func(context.Context, string) (*url.URL, error) {
+				return nil, errors.New("boom")
+			}
+			var extGateCalls int
+			extGate := func(f func() (io.ReadCloser, string, error)) (io.ReadCloser, string, error) {
+				extGateCalls++
+				return f()
+			}
+
+			res, err := resolveItem(ctx, ds, prov, ffm, model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar5"}, extGate)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(res.extError).To(BeTrue())
+			Expect(extGateCalls).To(Equal(1))
+		})
 	})
 
 	Describe("radio", func() {
@@ -301,5 +349,18 @@ var _ = Describe("resolveItem", func() {
 			Expect(res.reader).To(BeNil())
 			Expect(res.source).To(BeEmpty())
 		})
+
+		It("does not resolve as absent when every sampled album fails to resolve", func() {
+			// "missing1"/"missing2" are not in MockAlbumRepo's data, so resolveAlbum
+			// returns a genuine (non-external) error for every sampled tile.
+			plRepo := tests.CreateMockPlaylistRepo()
+			plRepo.SetData(model.Playlists{{ID: "pl3", Name: "Playlist"}})
+			plRepo.TracksRepo = &tests.MockPlaylistTrackRepo{AlbumIDs: []string{"missing1", "missing2"}}
+			ds.MockedPlaylist = plRepo
+
+			res, err := resolveItem(ctx, ds, prov, ffm, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "pl3"}, nil)
+			Expect(err).To(HaveOccurred())
+			Expect(res).To(Equal(resolution{}))
+		})
 	})
 })