mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-18 07:30:29 +00:00
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 <github@ranokay.com>
This commit is contained in:
parent
3998ce224d
commit
8cb4192935
@ -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(
|
||||
|
||||
@ -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(
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user