From 79b7adce8fdd9aee084e16c5b389d49792e68f36 Mon Sep 17 00:00:00 2001 From: khannurien Date: Sun, 30 Aug 2026 17:29:14 +0000 Subject: [PATCH] v3: fix the queue and re-resolve edge cases in native bandcamp playback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-resolving a stream is a network round-trip, and nothing checked that the player still belonged to that item once it landed. Picking another dump — or closing the player — while a resolve was in flight let the stale result swap the queue back; from onError, which resolves with autoplay, it would also start playing over whatever was chosen instead. A generation counter, bumped by playQueue/advanceTo/stop, now drops any result that no longer owns the player, with a separate counter owning the resolving flag so a superseded resolve can't clear a newer one's spinner. A track that stays dead after a re-resolve is one track, not one album: the cooldown path now steps to the next entry and only falls back to the iframe on the last one. A failed resolve still goes straight to the embed, since that's an album-wide failure rather than a single bad URL. And a track that has vanished from the release, or lost its streamable flag to Bandcamp's free-play cap, no longer resumes track 1 at the dead track's offset — the findIndex miss resets the offset instead of seeking past the end of another track. Clicking the playing row rewound it but never resumed: seekTo only moves currentTime, so re-selecting a finished or paused track looked like a no-op. It now resumes as well. Same idea in the compact rich-content card, whose button advertises "Pause" while active but re-played on click — in native mode that meant a fresh /api/bandcamp/tracks and a restart from track 1. It pauses for streams now; embeds have no transport of their own and keep restarting, as before. playRichContent reports whether playback actually started, so a native-mode Bandcamp page with no streams and no stored embedUrl — an artist root, a /music index, a preorder — is no longer a dead click: the journal card navigates to the dump as it used to, and the rich-content card opens the source page. Also: the tralbum fetch releases the response body on its two error paths. The endpoint is unauthenticated, so random /track/ URLs would leak a connection per request while also defeating the cache. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01E56F55N7m5GGKKEwoYFJtH --- api/services/bandcamp-stream-service.ts | 5 ++ src/components/JournalCard.tsx | 9 +++- src/components/RichContentCard.tsx | 19 +++++-- src/contexts/PlayerProvider.tsx | 70 ++++++++++++++++++++----- src/hooks/usePlayRichContent.ts | 22 ++++---- 5 files changed, 97 insertions(+), 28 deletions(-) diff --git a/api/services/bandcamp-stream-service.ts b/api/services/bandcamp-stream-service.ts index bdfc6e9..45e4cee 100644 --- a/api/services/bandcamp-stream-service.ts +++ b/api/services/bandcamp-stream-service.ts @@ -101,7 +101,11 @@ export async function resolveBandcamp( "Could not reach Bandcamp", ); } + // Every path out of here that doesn't read the body has to release it, or the + // connection stays open until GC. This endpoint is unauthenticated: a caller + // hammering random /track/ URLs would otherwise leak one body a request. if (!res.ok) { + await res.body?.cancel(); throw new APIException( APIErrorCode.SERVER_ERROR, 502, @@ -109,6 +113,7 @@ export async function resolveBandcamp( ); } if (!(res.headers.get("content-type") ?? "").startsWith("text/html")) { + await res.body?.cancel(); throw new APIException( APIErrorCode.SERVER_ERROR, 502, diff --git a/src/components/JournalCard.tsx b/src/components/JournalCard.tsx index f5ebdf3..aa23778 100644 --- a/src/components/JournalCard.tsx +++ b/src/components/JournalCard.tsx @@ -43,6 +43,13 @@ export function JournalCard( navigate(dumpUrl(dump)); } + // A playable card plays. If playback turns out to be impossible — a Bandcamp + // page with no streams and no embed to fall back to — the card must still do + // what an unplayable one does rather than swallowing the click. + async function handlePlayOrNavigate(rc: NonNullable) { + if (!await playRichContent(rc, dumpUrl(dump))) handleNavigate(); + } + // Mirrors FilePreview (the hot/new feeds) so a video shows its generated // still here too, rather than degrading to a text card with a 🎬. const thumbnailUrl = dump.thumbnailMime @@ -151,7 +158,7 @@ export function JournalCard(
  • void playRichContent(playable, dumpUrl(dump)) + ? () => void handlePlayOrNavigate(playable) : handleNavigate} >
    diff --git a/src/components/RichContentCard.tsx b/src/components/RichContentCard.tsx index 4caaa83..bd3229f 100644 --- a/src/components/RichContentCard.tsx +++ b/src/components/RichContentCard.tsx @@ -20,12 +20,20 @@ export default function RichContentCard( { richContent, compact = false, thumbnailOverrideUrl, dumpHref }: RichContentCardProps, ) { - const { current, playing } = useContext(PlayerContext); + const { current, playing, togglePlay } = useContext(PlayerContext); const { playRichContent, pending } = usePlayRichContent(); + const canPlay = canPlayRichContent(richContent); const thumbnailSrc = thumbnailOverrideUrl ?? richContent.thumbnailUrl; // The dump's own thumbnail overrides the provider's, in the player header too. const playable = { ...richContent, thumbnailUrl: thumbnailSrc }; + // Nothing to play and no embed to fall back to: the card body is a link to + // the source, so the thumbnail behaves like one rather than going dead. + const playOrOpen = async () => { + if (!await playRichContent(playable, dumpHref)) { + globalThis.open(richContent.url, "_blank", "noopener,noreferrer"); + } + }; const placeholder = { url: richContent.url, @@ -64,7 +72,12 @@ export default function RichContentCard( onClick={(e) => { e.preventDefault(); e.stopPropagation(); - void playRichContent(playable, dumpHref); + // The button says "Pause" once this card is the one playing, so it + // has to pause. Re-playing would re-resolve the album and lose the + // queue position. Embeds have no transport of their own, so they + // keep restarting, as before. + if (isActive && current?.kind === "stream") togglePlay(); + else void playOrOpen(); }} aria-label={isPlaying ? "Pause" : "Play"} > @@ -105,7 +118,7 @@ export default function RichContentCard( type="button" className={`rich-content-thumbnail-btn${pending ? " is-pending" : ""}`} disabled={pending} - onClick={() => void playRichContent(playable, dumpHref)} + onClick={() => void playOrOpen()} aria-label="Play" > {thumbnailImg} diff --git a/src/contexts/PlayerProvider.tsx b/src/contexts/PlayerProvider.tsx index bf93ad2..167caf5 100644 --- a/src/contexts/PlayerProvider.tsx +++ b/src/contexts/PlayerProvider.tsx @@ -87,16 +87,22 @@ export function PlayerProvider({ children }: { children: React.ReactNode }) { // MediaPlayer's unmount cleanup. Cleared when the new media fires onPlayStateChange(true). const suppressUpdates = useRef(false); + // Bumped whenever the player is moved somewhere new — a fresh queue, a queue + // step, a stop. An async re-resolve compares it against the value it captured + // to tell whether it still owns the player by the time it lands. + const playGen = useRef(0); + // Latest state, for callbacks that must not close over a stale render. // Written in an effect (after render) rather than during it, the same // convention MediaPlayer uses for its callback refs. - const stateRef = useRef({ current, currentTime, playing }); + const stateRef = useRef({ current, currentTime, playing, queue, queueIndex }); useEffect(() => { - stateRef.current = { current, currentTime, playing }; + stateRef.current = { current, currentTime, playing, queue, queueIndex }; }); const playQueue = useCallback((items: PlayerItem[], startIndex = 0) => { if (items.length === 0) return; + playGen.current++; suppressUpdates.current = true; setQueue(items); setQueueIndex(Math.min(Math.max(startIndex, 0), items.length - 1)); @@ -113,6 +119,7 @@ export function PlayerProvider({ children }: { children: React.ReactNode }) { /** Move within the existing queue, as a fresh (autoplaying) item. */ const advanceTo = useCallback((index: number) => { + playGen.current++; suppressUpdates.current = true; setQueueIndex(index); setCurrentTime(0); @@ -123,6 +130,7 @@ export function PlayerProvider({ children }: { children: React.ReactNode }) { }, []); const stop = useCallback(() => { + playGen.current++; setQueue([]); setQueueIndex(0); setPlaying(false); @@ -147,10 +155,13 @@ export function PlayerProvider({ children }: { children: React.ReactNode }) { // seek — while still flipping `playing` false against audio that's running. if (index === queueIndex) { seekTo(0); + // Rewinding alone would make the click look like a no-op on a row that + // has ended, or that the user paused. Start it again. + if (!stateRef.current.playing) togglePlay(); return; } advanceTo(index); - }, [queue.length, queueIndex, advanceTo, seekTo]); + }, [queue.length, queueIndex, advanceTo, seekTo, togglePlay]); const next = useCallback(() => { if (queueIndex + 1 < queue.length) advanceTo(queueIndex + 1); @@ -206,6 +217,23 @@ export function PlayerProvider({ children }: { children: React.ReactNode }) { }], 0); }, [playQueue, stop]); + /** + * A track that stays dead after a re-resolve is one track, not one album: + * step over it and keep the queue. Only the last one falls back to the embed. + */ + const skipOrFallBack = useCallback(( + item: Extract, + ) => { + const { queue: q, queueIndex: i } = stateRef.current; + if (i + 1 < q.length) { + advanceTo(i + 1); + return; + } + fallBackToEmbed(item); + }, [advanceTo, fallBackToEmbed]); + + const resolveSeq = useRef(0); + const reresolve = useCallback(async ( item: Extract, { resumeAt, shouldAutoplay }: { resumeAt: number; shouldAutoplay: boolean }, @@ -215,34 +243,48 @@ export function PlayerProvider({ children }: { children: React.ReactNode }) { const last = lastReresolve.current; if (last && last.key === key && now - last.at < RERESOLVE_COOLDOWN_MS) { // Already tried recently — the track is genuinely dead, not just stale. - fallBackToEmbed(item); + skipOrFallBack(item); return; } lastReresolve.current = { key, at: now }; + // Resolving is a network round-trip against Bandcamp. The user can pick + // another dump, skip on, or close the player while it's in flight — and a + // result that lands after that must not drag the player back here. + const gen = playGen.current; + const stillOurs = () => playGen.current === gen; + // Separate from `gen`: the spinner belongs to the newest resolve, whichever + // item that one is for, so a superseded resolve must not clear it. + const seq = ++resolveSeq.current; + setResolving(true); try { const items = await resolveStreamQueue(item); - const index = Math.max( - items.findIndex((i) => - i.kind === "stream" && i.resolveIndex === item.resolveIndex - ), - 0, + if (!stillOurs()) return; + const found = items.findIndex((i) => + i.kind === "stream" && i.resolveIndex === item.resolveIndex ); + // The track can be gone from the release, or no longer streamable once + // Bandcamp's free-play cap kicks in. Falling back to the first track is + // fine; carrying the old offset onto it would seek past its end. + const index = found < 0 ? 0 : found; + const resume = found < 0 ? 0 : resumeAt; suppressUpdates.current = true; setQueue(items); setQueueIndex(index); - setCurrentTime(resumeAt); - setStartTime(resumeAt); + setCurrentTime(resume); + setStartTime(resume); setDuration(0); setAutoplay(shouldAutoplay); setPlaying(false); } catch { - fallBackToEmbed(item); + // The resolve itself failed (offline, endpoint down) — that's the whole + // album, not one track, so the embed is the right fallback. + if (stillOurs()) fallBackToEmbed(item); } finally { - setResolving(false); + if (resolveSeq.current === seq) setResolving(false); } - }, [fallBackToEmbed]); + }, [fallBackToEmbed, skipOrFallBack]); const onError = useCallback(() => { const item = stateRef.current.current; diff --git a/src/hooks/usePlayRichContent.ts b/src/hooks/usePlayRichContent.ts index fff28ef..2b771f0 100644 --- a/src/hooks/usePlayRichContent.ts +++ b/src/hooks/usePlayRichContent.ts @@ -21,16 +21,19 @@ export interface PlayableRichContent { * before. Bandcamp additionally honours GERBEUR_BANDCAMP_PLAYER: in "native" * mode the page is resolved to its mp3 streams and played as a queue, which is * what makes it autoplay and lets albums play through. Any failure — offline, - * preorder-only release, Bandcamp changing its markup — silently falls back to - * the embed, so this can never leave a card unplayable. + * preorder-only release, Bandcamp changing its markup — falls back to the embed. + * + * Returns whether playback actually started. A native-mode page with no stored + * embedUrl (an artist root, a /music index, a preorder) has nothing to fall + * back to, and the caller has to be told so the click isn't swallowed. */ export function usePlayRichContent() { const { play, playQueue } = useContext(PlayerContext); const [pending, setPending] = useState(false); const playEmbed = useCallback( - (rc: PlayableRichContent, dumpHref?: string) => { - if (!rc.embedUrl) return; + (rc: PlayableRichContent, dumpHref?: string): boolean => { + if (!rc.embedUrl) return false; play({ kind: "embed", embedUrl: rc.embedUrl, @@ -40,17 +43,15 @@ export function usePlayRichContent() { artworkUrl: rc.thumbnailUrl, subtitle: rc.siteName, }); + return true; }, [play], ); const playRichContent = useCallback( - async (rc: PlayableRichContent, dumpHref?: string) => { + async (rc: PlayableRichContent, dumpHref?: string): Promise => { const native = rc.type === "bandcamp" && BANDCAMP_PLAYER === "native"; - if (!native) { - playEmbed(rc, dumpHref); - return; - } + if (!native) return playEmbed(rc, dumpHref); setPending(true); try { @@ -62,9 +63,10 @@ export function usePlayRichContent() { fallbackArtworkUrl: rc.thumbnailUrl, }); playQueue(items, 0); + return true; } catch (err) { console.warn("bandcamp: native playback unavailable, using embed", err); - playEmbed(rc, dumpHref); + return playEmbed(rc, dumpHref); } finally { setPending(false); }