From 2cd967a4562dab6ac659f05a659574d87172b154 Mon Sep 17 00:00:00 2001 From: Deluan Date: Fri, 17 Jul 2026 15:18:28 -0400 Subject: [PATCH] fix(artwork): keep the stored blurhash on transient recompute errors 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. --- core/artwork/blurhash_updater.go | 13 +++++++++---- core/artwork/blurhash_updater_internal_test.go | 8 ++++---- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/core/artwork/blurhash_updater.go b/core/artwork/blurhash_updater.go index 23a8799ee..eb2f9f40a 100644 --- a/core/artwork/blurhash_updater.go +++ b/core/artwork/blurhash_updater.go @@ -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) diff --git a/core/artwork/blurhash_updater_internal_test.go b/core/artwork/blurhash_updater_internal_test.go index 8d020e37b..d2e5960bb 100644 --- a/core/artwork/blurhash_updater_internal_test.go +++ b/core/artwork/blurhash_updater_internal_test.go @@ -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() {