nik/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-08-01 07:21:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(artwork): keep the stored blurhash on transient recompute errors

Browse Source 
process() cleared the stored hash whenever computeFromArtwork returned an
error OR an empty hash. A transient failure (the 30s context timeout, a flaky
cache/DB/reader read) is not evidence the artwork changed, so clearing on it
made the Jellyfin DTO fall back to a fake blurhash and churn clients' cover
caches until a later successful fill restored it.

Split the two outcomes: a compute error now leaves the stored hash intact and
lets a later fill retry, while an empty hash (a placeholder, i.e. the cover is
confirmed gone) still clears it. Deletion witnessed by a failed serve is
already handled separately by the gone path.
This commit is contained in:
Deluan 2026-07-17 15:18:28 -04:00
parent 66d6fdc1a6
commit 2cd967a456
2 changed files with 13 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
core/artwork/blurhash_updater.go
View File

@ -181,10 +181,15 @@ func (u *blurHashUpdater) process(ctx context.Context, artID model.ArtworkID, re
return
}
hash, err := u.computeFromArtwork(ctx, artID)
if err != nil || hash == "" {
log.Trace(ctx, "BlurHash: nothing to persist", "artID", artID, err)
// Reaching compute with a stored hash means the cover became a placeholder or vanished;
// clear it so the DTO stops describing artwork no longer served.
if err != nil {
// A transient failure (timeout, flaky cache/DB read) is not evidence the artwork changed;
// leave the stored hash intact and let a later fill retry, so clients don't churn on a fake.
log.Trace(ctx, "BlurHash: recompute failed, keeping stored hash", "artID", artID, err)
return
}
if hash == "" {
// An empty hash means the served bytes are a placeholder: the cover is gone. Clear a stored
// hash so the DTO stops describing artwork no longer served.
if stored != "" {
if err := u.persist(ctx, artID, "", req.snapshot); err != nil {
log.Warn(ctx, "BlurHash: error clearing stale hash", "artID", artID, err)

8
core/artwork/blurhash_updater_internal_test.go
View File

@ -80,20 +80,20 @@ var _ = Describe("blurHashUpdater", func() {
Expect(stored.BlurHash).To(Equal("LEHV6nWB2yk8"))
})
It("clears a stored hash when a recompute yields no result", func() {
It("keeps the stored hash when a recompute fails transiently", func() {
al := model.Album{ID: "al-1", UpdatedAt: version, BlurHash: "LEHV6nWB2yk8", BlurHashUpdatedAt: nil}
repo := tests.CreateMockAlbumRepo()
repo.SetData(model.Albums{al})
ds.MockedAlbum = repo
ds.MockedFolder = failingFolderRepo{}
// A stored hash with a newer snapshot is change evidence; the compute fails, so the
// stale hash must go.
// A newer snapshot forces a recompute, but the reader chain errors (transient): the stored
// hash must survive, so clients don't churn on a fake until a later fill succeeds.
newer := version.Add(time.Hour)
u.process(GinkgoT().Context(), al.CoverArtID(), enqueueRequest{snapshot: newer})
stored, err := ds.Album(GinkgoT().Context()).Get("al-1")
Expect(err).ToNot(HaveOccurred())
Expect(stored.BlurHash).To(BeEmpty())
Expect(stored.BlurHash).To(Equal("LEHV6nWB2yk8"))
})
It("clears a stored hash when the source is gone", func() {