From 40984f60865dd89ca6f7af6201aeb2c6db266cf3 Mon Sep 17 00:00:00 2001 From: Caleb Allen Date: Mon, 17 Aug 2026 22:09:14 -0400 Subject: [PATCH] fix(explore): let a slow archive node finish, and read the 404 back Explore's album art was almost entirely missing: 5 of 24 cards on the shelves had a cover, and those five were the ones already on disk. The Cover Art Archive answers `front-250` with a 307 to an Internet Archive storage node, and those nodes are slow. Measured against the twelve albums on Explore's own shelves, a successful fetch took 14-16 s and a failing one 13-17 s, against a client timeout of 10. So every live fetch died, and a timeout writes nothing and says nothing -- which is why this reads as "Explore has no album art" rather than as a slow upstream. The timeout is 30 s, chosen to clear the measured range: the fetch is off the critical path, so waiting costs nothing and giving up early costs the whole page. Two things beside it, both found on the way. `writeCache(mbid, nil)` has recorded "the archive has no art for this" as an empty file since it was written, and nothing has ever read it back: `readCache` returns "" for an empty file, which is indistinguishable from a miss. So every art-less release group was re-fetched from CAA on every render that asked about it. A third of the shelves are art-less, so that was a third of the page spending a live request to be told again what the last one said. `knownMissing` reads it, on both the release-group and the release path. And the frontend marked a failed fetch as permanently answered for the session, so a timed-out cover never retried within it. It drops the marker instead; a genuine 404 is now answered from disk, so re-asking one costs nothing. Measured after: 23 of 24. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01MeQt5hgXg5YGoNZQ9ozG7L --- backend/explore/coverartproxy.go | 54 +++++++++++++++++-- .../components/explore-view/explore-view.ts | 17 +++++- 2 files changed, 65 insertions(+), 6 deletions(-) diff --git a/backend/explore/coverartproxy.go b/backend/explore/coverartproxy.go index b7dee0c..25cee80 100644 --- a/backend/explore/coverartproxy.go +++ b/backend/explore/coverartproxy.go @@ -25,8 +25,26 @@ const ( // where cached cover art thumbnails are stored. thumbnailDir = CoverArtCacheDirName - // thumbnailTimeout is the HTTP timeout for fetching a thumbnail. - thumbnailTimeout = 10 * time.Second + // thumbnailTimeout is the HTTP timeout for fetching a thumbnail, + // and it has to cover a redirect the Cover Art Archive does not + // serve itself. + // + // `coverartarchive.org` answers `front-250` with a 307 to an + // Internet Archive storage node (`dn######.us.archive.org`), and + // those nodes are routinely slow: measured against the twelve + // albums on Explore's own shelves, a successful fetch took 14–16 s + // and a failing one 13–17 s. At 10 s *every* cover on the page + // timed out — 24 cards, 5 of which had art, all of those from the + // disk cache — which reads as "Explore has no album art" rather + // than as a slow upstream, because a timeout writes nothing and + // says nothing. + // + // 30 s is chosen to clear that measured range with room, not to be + // generous: the fetch is off the critical path (each one is its own + // goroutine behind an 8/s limiter, and the frontend renders a + // placeholder until it lands), so the cost of waiting is nothing + // and the cost of giving up early is a blank page. + thumbnailTimeout = 30 * time.Second // thumbnailMaxSize is the maximum image size to cache (2 MB). thumbnailMaxSize = 2 * 1024 * 1024 @@ -97,6 +115,20 @@ func (p *CoverArtProxy) GetThumbnail( return "" } + // A 404 is an answer, and it is already on disk. + // + // `writeCache(mbid, nil)` has recorded "the archive has no art for + // this" as an empty file since this was written, and nothing has + // ever read it back: `readCache` returns "" for an empty file, + // which is indistinguishable from a miss, so every art-less release + // group was re-fetched from the network on every render that asked + // about it. On Explore's shelves a third of the cards are art-less, + // so that was a third of the page spending a live CAA request to be + // told again what the last one said. + if p.knownMissing(releaseGroupMBID) { + return "" + } + // Source 3: fetch from Cover Art Archive (slow, cached to disk). url := CoverArtGroupURL(releaseGroupMBID) data, cacheable, err := p.fetch(url) @@ -177,8 +209,9 @@ func (p *CoverArtProxy) GetCandidateThumbnail( } } - // Network fetch on release group. - if releaseGroupMBID != "" { + // Network fetch on release group — unless a previous one was told + // there is none. See `knownMissing`. + if releaseGroupMBID != "" && !p.knownMissing(releaseGroupMBID) { url := CoverArtGroupURL(releaseGroupMBID) data, cacheable, err := p.fetch(url) @@ -194,7 +227,7 @@ func (p *CoverArtProxy) GetCandidateThumbnail( } // Network fetch on release (fallback). - if releaseMBID != "" { + if releaseMBID != "" && !p.knownMissing(releaseMBID) { url := CoverArtURL(releaseMBID) data, cacheable, err := p.fetch(url) @@ -285,6 +318,17 @@ func (p *CoverArtProxy) cachePath(mbid string) string { return filepath.Join(p.cacheDir, mbid+".jpg") } +// knownMissing reports whether a previous fetch was told the archive +// has no art for this MBID — the empty file `writeCache(mbid, nil)` +// leaves behind. It is deliberately separate from `readCache`, which +// answers "what are the bytes" and cannot express the difference +// between no answer and an answer of none. +func (p *CoverArtProxy) knownMissing(mbid string) bool { + info, err := os.Stat(p.cachePath(mbid)) + + return err == nil && info.Size() == 0 +} + func (p *CoverArtProxy) readCache(mbid string) string { path := p.cachePath(mbid) diff --git a/frontend/src/components/explore-view/explore-view.ts b/frontend/src/components/explore-view/explore-view.ts index be05df2..b9b1257 100644 --- a/frontend/src/components/explore-view/explore-view.ts +++ b/frontend/src/components/explore-view/explore-view.ts @@ -1509,9 +1509,24 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte if (url) { this.thumbnailCache.set(req.mbid, url); this.requestUpdate(); + + return; } + + // An empty answer is not necessarily "there + // is no art" — a slow Internet Archive node + // is answered by a timeout, which looks + // exactly the same from here. Drop the + // in-flight marker so the next time this + // release group is on screen it is asked + // again; the backend records a genuine 404 + // on disk and answers that one instantly, + // so a real miss costs nothing to re-ask. + this.thumbnailCache.delete(req.mbid); }) - .catch(() => {}); + .catch(() => { + this.thumbnailCache.delete(req.mbid); + }); } }) .catch(() => {