From 2207211997fb71bf87477dc7425f6ceb37f098b5 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sat, 25 Jul 2026 10:08:29 -0400 Subject: [PATCH] fix(artwork): keep served art when the retry budget runs out Exhausting the 12h budget called writeAbsent unconditionally, so an entity whose art was already resolved and serving lost it to a long upstream outage: the hash went empty, clients fell back to the placeholder, and the now-unreferenced bytes were freed by the next prune even though nothing about the image had changed. Exhaustion means the source stayed unreachable, not that the cover disappeared, so absent is now recorded only when there is nothing to keep. --- core/artwork/worker.go | 14 +++++++++++++- core/artwork/worker_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/core/artwork/worker.go b/core/artwork/worker.go index 237cab8c3..989cc604b 100644 --- a/core/artwork/worker.go +++ b/core/artwork/worker.go @@ -228,7 +228,9 @@ func (w *Worker) process(ctx context.Context, item model.ArtworkQueueItem) outco } // Retry budget exhausted: stop retrying. A bare failure settles absent so the stale-absent // sweep (and a page view) can still recover it; a stale-found keeps its already-served art. - if out == outcomeFailed { + // Art we are already serving is kept too: exhaustion means the source stayed unreachable, + // not that the entity lost its cover. + if out == outcomeFailed && !w.hasResolvedArtwork(ctx, item) { writeAbsent(ctx, w.deps.ds.Artwork(ctx), item) } if err := queue.DeleteIfUnchanged(item.ItemKind, item.ItemID, item.ImageType, item.RetryAt); err != nil { @@ -238,6 +240,16 @@ func (w *Worker) process(ctx context.Context, item model.ArtworkQueueItem) outco return out } +// hasResolvedArtwork reports whether the item already has a hash-bearing state row. +func (w *Worker) hasResolvedArtwork(ctx context.Context, item model.ArtworkQueueItem) bool { + kind, ok := model.ParseKind(item.ItemKind) + if !ok { + return false + } + ia, err := w.deps.ds.Artwork(ctx).GetItemArtwork(kind, item.ItemID, item.ImageType) + return err == nil && ia.Hash != "" +} + // precache warms the resize cache for a newly-acquired image at the UI cover size, so the // first UI request is a cache hit. Skipped when disabled; failures are debug-only. func (w *Worker) precache(ctx context.Context, item model.ArtworkQueueItem) { diff --git a/core/artwork/worker_test.go b/core/artwork/worker_test.go index cf1969c73..5d280edcc 100644 --- a/core/artwork/worker_test.go +++ b/core/artwork/worker_test.go @@ -336,6 +336,33 @@ var _ = Describe("Worker", func() { Expect(ia.Hash).To(BeEmpty()) }) + It("keeps already-served art when the retry budget is exhausted", func() { + conf.Server.CoverArtPriority = "external" + ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "al10", Name: "Album"}}) + Expect(artRepo.PutItemArtwork(&model.ItemArtwork{ + ItemKind: "al", ItemID: "al10", ImageType: model.ImageTypePrimary, + Hash: "cafebabe", Source: "external:lastfm", + })).To(Succeed()) + imageAgents(&fakeImageAgent{name: "failAgent", err: errors.New("agent timed out")}) + w = NewWorker(ds, store, ag, ffm, broker, imgCache) + Expect(queueRepo.Enqueue(model.ArtworkQueueItem{ItemKind: "al", ItemID: "al10"})).To(Succeed()) + for k, v := range queueRepo.Data { + if v.ItemID == "al10" { + v.EnqueuedAt = time.Now().Add(-(giveUpAfter + time.Hour)) + queueRepo.Data[k] = v + } + } + + n, err := w.drain(ctx, 1) + Expect(err).ToNot(HaveOccurred()) + Expect(n).To(Equal(1)) + + Expect(findQueued(queueRepo, "al", "al10")).To(BeNil()) + ia, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "al10", model.ImageTypePrimary) + Expect(err).ToNot(HaveOccurred()) + Expect(ia.Hash).To(Equal("cafebabe"), "a persistent outage must not discard served art") + }) + It("resolves a private playlist under an admin context instead of failing forever", func() { ds.MockedUser = adminUserRepo() vds := &visibilityPlaylistDS{