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{