From 6553a75b0e6aedfc5605e7620ae275a675ef03f9 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sat, 25 Jul 2026 10:14:22 -0400 Subject: [PATCH] fix(artwork): keep not-found distinct from artwork-absent GetOrPlaceholder folded model.ErrNotFound in with ErrUnavailable, so an id matching no entity returned 200 and the placeholder PNG. That made the ErrorDataNotFound branch in getCoverArt unreachable: Subsonic went from error 70 to a successful placeholder, and Jellyfin's Primary image endpoint from 404 to 200. Neither is ours to change. An entity with no art and an id with no entity are different answers; only the first is a placeholder. --- core/artwork/serving.go | 4 +++- core/artwork/serving_test.go | 10 ++++++++++ server/subsonic/e2e/subsonic_artwork_test.go | 9 +++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/core/artwork/serving.go b/core/artwork/serving.go index a65565861..5021b1c1c 100644 --- a/core/artwork/serving.go +++ b/core/artwork/serving.go @@ -66,7 +66,9 @@ func (s *service) GetOrPlaceholder(ctx context.Context, id string, size int, squ if err == nil { img, err = s.Get(ctx, artID, size, square) } - if errors.Is(err, ErrUnavailable) || errors.Is(err, model.ErrNotFound) { + // Only a resolvable entity with no art gets the placeholder. An id that matches no entity + // stays ErrNotFound, so getCoverArt keeps answering error 70 and Jellyfin keeps 404ing. + if errors.Is(err, ErrUnavailable) { return s.placeholder(artID.Kind), nil } return img, err diff --git a/core/artwork/serving_test.go b/core/artwork/serving_test.go index acba61875..37a2131a4 100644 --- a/core/artwork/serving_test.go +++ b/core/artwork/serving_test.go @@ -378,6 +378,16 @@ var _ = Describe("Service", func() { phBytes, _ := io.ReadAll(ph) Expect(readAll(img)).To(Equal(phBytes)) }) + + // An entity that has no art and an id that names no entity are different answers: + // Subsonic reports error 70 for the latter, and Jellyfin 404s. + It("reports not-found rather than a placeholder for an id with no entity", func() { + _, err := svc.GetOrPlaceholder(ctx, "al-nosuchalbum", 0, false) + Expect(err).To(MatchError(model.ErrNotFound)) + + _, err = svc.GetOrPlaceholder(ctx, "nosuchrawid", 0, false) + Expect(err).To(MatchError(model.ErrNotFound)) + }) }) }) diff --git a/server/subsonic/e2e/subsonic_artwork_test.go b/server/subsonic/e2e/subsonic_artwork_test.go index 1e7825214..a1d874557 100644 --- a/server/subsonic/e2e/subsonic_artwork_test.go +++ b/server/subsonic/e2e/subsonic_artwork_test.go @@ -201,6 +201,15 @@ var _ = Describe("Artwork Serving", Ordered, func() { Expect(w.Header().Get("Cache-Control")).To(Equal("no-store")) }) + // An album with no art and an id naming no album are different answers; only the former + // is a placeholder. + It("answers error 70 for an id that matches no entity", func() { + w := getCover("id", "al-nosuchalbum") + Expect(w.Code).To(Equal(http.StatusOK)) + Expect(w.Body.String()).To(ContainSubstring(`"code":70`)) + Expect(w.Body.Bytes()).ToNot(Equal(placeholder)) + }) + It("serves /share/img immutably for a JWT whose payload carries the hash", func() { token, err := auth.CreateExpiringPublicToken(time.Now().Add(time.Hour), auth.Claims{ID: "al-" + artfulID + "_" + artfulHash})