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(