diff --git a/python-backend/musicmouse/services/web/api.py b/python-backend/musicmouse/services/web/api.py index d9c77d6..df4646c 100644 --- a/python-backend/musicmouse/services/web/api.py +++ b/python-backend/musicmouse/services/web/api.py @@ -104,9 +104,20 @@ def build_router( raise HTTPException(status_code=404, detail="no cover") return FileResponse( album.cover, - # Cover files are content-addressed by album id and rewritten only by a - # rescan, so a long cache is safe and saves 20 requests per page load. - headers={"Cache-Control": "public, max-age=604800"}, + # This used to send `public, max-age=604800`, on the reasoning that a cover + # is "content-addressed by album id". It is not: the id addresses *which + # album* the art belongs to, and the bytes behind that URL change whenever + # the art is reprocessed. When `MAX_COVER_PX` arrived and every cover was + # rewritten smaller, every browser that had loaded the page in the previous + # week went on decoding the old 3000px file out of its own disk cache, and + # a trace was the only way to see it. + # + # `no-cache` does not mean "do not store", it means "revalidate before + # reusing" - so the browser keeps the file and usually gets a 304. The cost + # is one conditional request per cover, and it is not on the paint path at + # all: the service worker answers covers from its own cache first and does + # the revalidation behind the page (see web/public/sw.js). + headers={"Cache-Control": "no-cache"}, ) @router.get("/tracks/{album_id}/{index}/analysis") diff --git a/web/public/sw.js b/web/public/sw.js index d9ac3e3..09a69d8 100644 --- a/web/public/sw.js +++ b/web/public/sw.js @@ -52,17 +52,27 @@ self.addEventListener("fetch", (event) => { const url = new URL(request.url); if (url.origin !== self.location.origin) return; - // Cover art is cached aggressively - it is the only heavy thing here, and an album's - // art does not change from one load to the next. See the note on COVERS above for - // what to do when the way it is *produced* changes. + // Cover art: stale-while-revalidate. The cached copy is served straight away, which + // is what keeps a grid of album art off the network entirely, and a fresh copy is + // fetched behind the page and put back for next time. + // + // It was plain cache-first, which is subtly wrong in a way that cost a long afternoon: + // the bytes behind a cover URL do change (the backend reprocesses art), and + // cache-first on a URL that never changes means a client can serve a stale cover for + // ever. Revalidating in the background is the cheap way to be both fast and eventually + // right, and it needs no version bump when art is reprocessed. if (url.pathname.startsWith("/api/albums/")) { event.respondWith( caches.open(COVERS).then(async (cache) => { const hit = await cache.match(request); - if (hit) return hit; - const response = await fetch(request); - if (response.ok) cache.put(request, response.clone()); - return response; + const fetching = fetch(request) + .then((response) => { + if (response.ok) cache.put(request, response.clone()); + return response; + }) + // Offline, or the mouse is off: a cached cover is still better than none. + .catch(() => hit); + return hit ?? fetching; }), ); return;