From 8cb41929357c79c4d437a1ee9e0c9b2a08f0964d Mon Sep 17 00:00:00 2001 From: ranokay Date: Sat, 17 Oct 2026 13:34:53 +0300 Subject: [PATCH] fix(ui): stabilize progressive refreshes and recover from transient lyrics errors Identical refresh payloads keep the previous lyric references, so the panel's scroll reset (and downstream memoization) no longer fires on every poll tick. Non-cooldown failures now retry on a bounded backoff (5s/15s/45s/90s/180s), covering the plugin's failed fast pass while its background sweep fills the source caches. Signed-off-by: ranokay --- ui/src/audioplayer/useEnhancedLyrics.js | 71 ++++++++++++++++- ui/src/audioplayer/useEnhancedLyrics.test.js | 84 ++++++++++++++++++++ 2 files changed, 151 insertions(+), 4 deletions(-) diff --git a/ui/src/audioplayer/useEnhancedLyrics.js b/ui/src/audioplayer/useEnhancedLyrics.js index d703e2509..563750330 100644 --- a/ui/src/audioplayer/useEnhancedLyrics.js +++ b/ui/src/audioplayer/useEnhancedLyrics.js @@ -16,6 +16,11 @@ export const emptyLyricLayers = Object.freeze({ const MAX_LYRIC_CACHE_ENTRIES = 75 const NEGATIVE_CACHE_TTL_MS = 30_000 const COOLDOWN_PATTERN = /(?:cooldown active for|retry after)\s+(\d+)s/gi +// Transient plugin failures (provider 5xx) can clear on their own: the plugin's +// background sweep fills its per-source caches after the failed fast pass, so a +// bounded backoff recovers without a manual track switch. Server-reported +// cooldowns use their own schedule instead. +const AUTO_RETRY_DELAYS_MS = [5_000, 15_000, 45_000, 90_000, 180_000] const cache = new Map() const inFlight = new Map() @@ -59,6 +64,20 @@ const readCachedLyrics = (cacheKey) => { return cached.structuredLyrics } +// Servers re-serialize identical payloads per request, so a refreshed response +// only replaces the previous one when its content actually changed. Keeping the +// reference stable stops downstream memoization (and the lyrics panel's scroll +// reset) from churning on every progressive refresh. +const sameStructuredLyrics = (left, right) => { + if (left === right) return true + if (!left || !right) return false + try { + return JSON.stringify(left) === JSON.stringify(right) + } catch { + return false + } +} + const createLyricsRequest = ({ trackId, cacheKey }) => { const controller = new AbortController() const entry = { @@ -135,6 +154,7 @@ const useEnhancedLyrics = ({ const [state, setState] = useState(() => emptyLyricsState(cacheKey)) const [requestGeneration, setRequestGeneration] = useState(0) const [refreshToken, setRefreshToken] = useState(0) + const [autoRetry, setAutoRetry] = useState(null) const [, setCountdownTick] = useState(0) // The refresh path reads the currently displayed state when deciding whether @@ -145,9 +165,11 @@ const useEnhancedLyrics = ({ stateRef.current = state }, [state]) const handledRefreshRef = useRef(refreshToken) + const autoRetryRef = useRef({ cacheKey: null, attempts: 0 }) const retry = useCallback(() => { cache.delete(cacheKey) + autoRetryRef.current = { cacheKey, attempts: 0 } setRequestGeneration((current) => current + 1) }, [cacheKey]) @@ -164,6 +186,7 @@ const useEnhancedLyrics = ({ useEffect(() => { if (!trackId || disabled || !requested) { handledRefreshRef.current = refreshToken + setAutoRetry(null) setState(emptyLyricsState(cacheKey)) return undefined } @@ -211,14 +234,20 @@ const useEnhancedLyrics = ({ request.promise .then((structuredLyrics) => { if (!active) return - setState({ + autoRetryRef.current = { cacheKey, attempts: 0 } + setAutoRetry(null) + setState((current) => ({ cacheKey, - structuredLyrics, + structuredLyrics: + current.cacheKey === cacheKey && + sameStructuredLyrics(current.structuredLyrics, structuredLyrics) + ? current.structuredLyrics + : structuredLyrics, loading: false, refreshing: false, error: null, retryAt: null, - }) + })) }) .catch((error) => { if (!active || error?.name === 'AbortError') return @@ -232,6 +261,22 @@ const useEnhancedLyrics = ({ return } cache.delete(cacheKey) + const cooldownDelay = retryDelayFromError(error) + if (cooldownDelay != null) { + autoRetryRef.current = { cacheKey, attempts: 0 } + setAutoRetry(null) + } else { + const attempts = + autoRetryRef.current.cacheKey === cacheKey + ? autoRetryRef.current.attempts + : 0 + autoRetryRef.current = { cacheKey, attempts: attempts + 1 } + setAutoRetry( + attempts < AUTO_RETRY_DELAYS_MS.length + ? { cacheKey, at: Date.now() + AUTO_RETRY_DELAYS_MS[attempts] } + : null, + ) + } setState({ cacheKey, structuredLyrics: null, @@ -239,7 +284,7 @@ const useEnhancedLyrics = ({ refreshing: false, error, retryAt: (() => { - const delay = retryDelayFromError(error) + const delay = cooldownDelay return delay == null ? null : Date.now() + delay })(), }) @@ -273,6 +318,24 @@ const useEnhancedLyrics = ({ } }, [cacheKey, disabled, requested, retry, state.cacheKey, state.retryAt]) + useEffect(() => { + if ( + !autoRetry || + autoRetry.cacheKey !== cacheKey || + disabled || + !requested + ) { + return undefined + } + + const retryDelay = Math.max(0, autoRetry.at - Date.now()) + const retryTimer = window.setTimeout(() => { + setAutoRetry(null) + setRequestGeneration((current) => current + 1) + }, retryDelay) + return () => window.clearTimeout(retryTimer) + }, [autoRetry, cacheKey, disabled, requested]) + const structuredLyrics = state.cacheKey === cacheKey ? state.structuredLyrics : null const variants = useMemo( diff --git a/ui/src/audioplayer/useEnhancedLyrics.test.js b/ui/src/audioplayer/useEnhancedLyrics.test.js index 0e1e15b97..337060316 100644 --- a/ui/src/audioplayer/useEnhancedLyrics.test.js +++ b/ui/src/audioplayer/useEnhancedLyrics.test.js @@ -367,6 +367,90 @@ describe('useEnhancedLyrics', () => { expect(subsonic.getLyricsBySongId).toHaveBeenCalledTimes(1) }) + it('keeps lyric references stable when a refresh returns the same payload', async () => { + subsonic.getLyricsBySongId.mockImplementation(async () => + responseForEntries([ + sourceEntry('NetEase', 'NetEase line'), + sourceEntry('LRCLib', 'LRCLib line'), + ]), + ) + + const { result } = renderLyrics({ trackId: 'song-stable' }) + + await expectLyric(result, 'NetEase line') + const layers = result.current.layers + const variants = result.current.variants + + act(() => { + result.current.refresh() + }) + await waitFor(() => expect(result.current.refreshing).toBe(false)) + + expect(subsonic.getLyricsBySongId).toHaveBeenCalledTimes(2) + expect(result.current.layers).toBe(layers) + expect(result.current.variants).toBe(variants) + }) + + it('auto-retries a failed request with a bounded backoff', async () => { + vi.useFakeTimers() + subsonic.getLyricsBySongId + .mockRejectedValueOnce( + new Error('plugin call failed: BiniLyrics search returned HTTP 502'), + ) + .mockResolvedValueOnce( + responseForEntries([sourceEntry('NetEase', 'Recovered line')]), + ) + + const { result } = renderLyrics({ trackId: 'song-auto-retry' }) + await act(async () => { + await Promise.resolve() + await Promise.resolve() + }) + + expect(result.current.error).not.toBeNull() + expect(subsonic.getLyricsBySongId).toHaveBeenCalledTimes(1) + + await act(async () => { + vi.advanceTimersByTime(5_000) + }) + await act(async () => { + await Promise.resolve() + await Promise.resolve() + }) + + expect(subsonic.getLyricsBySongId).toHaveBeenCalledTimes(2) + expect(result.current.error).toBeNull() + expect(result.current.layers.main.line[0].value).toBe('Recovered line') + }) + + it('stops auto-retrying after the backoff schedule is exhausted', async () => { + vi.useFakeTimers() + subsonic.getLyricsBySongId.mockRejectedValue(new Error('boom')) + + const { result } = renderLyrics({ trackId: 'song-auto-retry-capped' }) + await act(async () => { + await Promise.resolve() + await Promise.resolve() + }) + expect(result.current.error).not.toBeNull() + + for (const delay of [5_000, 15_000, 45_000, 90_000, 180_000]) { + await act(async () => { + vi.advanceTimersByTime(delay) + }) + await act(async () => { + await Promise.resolve() + await Promise.resolve() + }) + } + expect(subsonic.getLyricsBySongId).toHaveBeenCalledTimes(6) + + await act(async () => { + vi.advanceTimersByTime(600_000) + }) + expect(subsonic.getLyricsBySongId).toHaveBeenCalledTimes(6) + }) + it('keeps current lyrics when a refresh fails', async () => { subsonic.getLyricsBySongId .mockResolvedValueOnce(