fix(artwork): never settle absent for a kind no recheck job revisits

The 12h retry budget hands a bare failure to the periodic stale-absent
sweep, which is what makes the resulting absent row recoverable. Media
files are deliberately excluded from that sweep -- they resolve embedded
only, at scan or on view -- so exhausting the budget on a transient read
error recorded a "this track has no cover" verdict that nothing would
ever revisit.

Settle absent only for kinds a recheck job covers. Without a row the
track stays unresolved, so the next view re-enqueues it.
This commit is contained in:
Deluan 2026-07-26 20:16:18 -04:00
parent c9c363523b
commit 28f3c720da
3 changed files with 39 additions and 5 deletions

View File

@ -5,6 +5,7 @@ import (
"crypto/md5"
"encoding/hex"
"fmt"
"slices"
"time"
"github.com/navidrome/navidrome/conf"
@ -23,6 +24,13 @@ var recheckKinds = []model.Kind{
model.KindArtistArtwork, model.KindAlbumArtwork, model.KindPlaylistArtwork, model.KindRadioArtwork,
}
// hasRecheckPath reports whether a periodic job will revisit this kind, which is what makes
// settling absent on an exhausted retry budget recoverable rather than permanent.
func hasRecheckPath(prefix string) bool {
kind, ok := model.ParseKind(prefix)
return ok && slices.Contains(recheckKinds, kind)
}
// artworkEpoch invalidates all resolution state when bumped; bump it in the same change that
// alters resolution semantics. Deliberately not the server version, which changes every build.
const artworkEpoch = 1

View File

@ -273,11 +273,11 @@ func (w *Worker) process(ctx context.Context, item model.ArtworkQueueItem) (outc
}
break
}
// 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.
// 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) {
// Retry budget exhausted: stop retrying. Absent is only recoverable where a periodic
// recheck will revisit it, so kinds without one keep no row at all; and art already
// being served is kept, since exhaustion means the source stayed unreachable rather
// than that the entity lost its cover.
if out == outcomeFailed && hasRecheckPath(item.ItemKind) && !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 {

View File

@ -364,6 +364,32 @@ var _ = Describe("Worker", func() {
Expect(ia.Hash).To(Equal("cafebabe"), "a persistent outage must not discard served art")
})
// Media files are excluded from recheckKinds, so an absent row written here would never
// be revisited: a transient read error would look like "this track has no cover" forever.
It("does not settle absent on exhaustion for a kind with no recheck path", func() {
conf.Server.EnableMediaFileCoverArt = true
ds.MockedMediaFile = tests.CreateMockMediaFileRepo()
ds.MockedMediaFile.(*tests.MockMediaFileRepo).SetData(model.MediaFiles{
{ID: "mfX", LibraryID: 0, Path: "tests/fixtures/artist/an-album/gone.mp3", HasCoverArt: true},
})
Expect(queueRepo.Enqueue(model.ArtworkQueueItem{ItemKind: "mf", ItemID: "mfX"})).To(Succeed())
for k, v := range queueRepo.Data {
if v.ItemID == "mfX" {
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, "mf", "mfX")).To(BeNil(), "the row must stop retrying")
_, err = artRepo.GetItemArtwork(model.KindMediaFileArtwork, "mfX", model.ImageTypePrimary)
Expect(err).To(MatchError(model.ErrNotFound),
"no row leaves the track unresolved, so a later view can still recover it")
})
It("resolves a private playlist under an admin context instead of failing forever", func() {
ds.MockedUser = adminUserRepo()
vds := &visibilityPlaylistDS{