diff --git a/core/lyrics/lyrics.go b/core/lyrics/lyrics.go index b9fb8cb74..8dd570b78 100644 --- a/core/lyrics/lyrics.go +++ b/core/lyrics/lyrics.go @@ -87,6 +87,7 @@ func songsByArtistTitleWithLyricsFirst(artist, title string) model.QueryOptions } func (l *lyricsService) getLyricsForCandidates(ctx context.Context, mediaFiles []*model.MediaFile) (model.LyricList, error) { + var pluginErr error for pattern := range strings.SplitSeq(conf.Server.LyricsPriority, ",") { pattern = strings.TrimSpace(pattern) if pattern == "" { @@ -101,6 +102,9 @@ func (l *lyricsService) getLyricsForCandidates(ctx context.Context, mediaFiles [ lyricsList, err := l.getLyricsFromSource(ctx, mf, pattern) if err != nil { log.Error(ctx, "error getting lyrics", "source", pattern, err) + if pattern != "embedded" && !strings.HasPrefix(pattern, ".") { + pluginErr = err + } continue } @@ -110,7 +114,7 @@ func (l *lyricsService) getLyricsForCandidates(ctx context.Context, mediaFiles [ } } - return nil, nil + return nil, pluginErr } func (l *lyricsService) getLyricsFromSource(ctx context.Context, mf *model.MediaFile, pattern string) (model.LyricList, error) { diff --git a/core/lyrics/lyrics_test.go b/core/lyrics/lyrics_test.go index b00bcd576..fbe85914d 100644 --- a/core/lyrics/lyrics_test.go +++ b/core/lyrics/lyrics_test.go @@ -365,6 +365,18 @@ var _ = Describe("Lyrics", func() { Expect(err).To(BeNil()) Expect(list).To(Equal(embeddedLyrics)) // falls through to embedded }) + + It("should return a plugin error when no later source has lyrics", func() { + conf.Server.LyricsPriority = "test-lyrics-plugin,embedded" + mf.Lyrics = "" + mockLoader.err = fmt.Errorf("remote lyrics cooldown active for 30s") + svc := lyrics.NewLyrics(nil, mockLoader) + + list, err := svc.GetLyrics(ctx, &mf) + + Expect(list).To(BeEmpty()) + Expect(err).To(MatchError("remote lyrics cooldown active for 30s")) + }) }) var _ = Describe("GetLyricsByArtistTitle", func() { diff --git a/ui/src/audioplayer/LyricsPanel.jsx b/ui/src/audioplayer/LyricsPanel.jsx index 9b2a45f57..838645ac2 100644 --- a/ui/src/audioplayer/LyricsPanel.jsx +++ b/ui/src/audioplayer/LyricsPanel.jsx @@ -417,6 +417,7 @@ const LyricsPanel = ({ inline = false, loading = false, error = null, + errorMessage = null, labels = {}, }) => { const classes = useStyles() @@ -675,7 +676,7 @@ const LyricsPanel = ({ const message = loading ? labels.loading || 'Loading lyrics' : error - ? labels.unavailable || 'Lyrics unavailable' + ? errorMessage || labels.unavailable || 'Lyrics unavailable' : labels.empty || 'No lyrics available' return (
', () => { ) }) + it('explains a temporary lyrics cooldown', () => { + renderPanel({ + mainLyric: null, + error: new Error('rate limited'), + errorMessage: 'Remote lyrics are rate-limited. Retrying in 30 seconds.', + }) + + expect(screen.getByTestId('lyrics-empty-state')).toHaveTextContent( + 'Remote lyrics are rate-limited. Retrying in 30 seconds.', + ) + }) + it.each(['light', 'dark'])( 'keeps future tokens readable and translations subordinate in the %s theme', (type) => { diff --git a/ui/src/audioplayer/LyricsSidebar.jsx b/ui/src/audioplayer/LyricsSidebar.jsx index cb4aa5867..ae4995f32 100644 --- a/ui/src/audioplayer/LyricsSidebar.jsx +++ b/ui/src/audioplayer/LyricsSidebar.jsx @@ -102,6 +102,7 @@ const LyricsSidebar = ({ audioInstance, loading = false, error = null, + errorMessage = null, labels = {}, returnFocusRef, }) => { @@ -291,6 +292,7 @@ const LyricsSidebar = ({ audioInstance={audioInstance} loading={loading} error={error} + errorMessage={errorMessage} labels={labels} />
diff --git a/ui/src/audioplayer/useEnhancedLyrics.js b/ui/src/audioplayer/useEnhancedLyrics.js index 735922583..8577e9280 100644 --- a/ui/src/audioplayer/useEnhancedLyrics.js +++ b/ui/src/audioplayer/useEnhancedLyrics.js @@ -10,6 +10,7 @@ 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 const cache = new Map() const inFlight = new Map() @@ -23,6 +24,15 @@ const normalizeLyricLayers = (layers) => ({ const readStructuredLyrics = (response) => response?.json?.['subsonic-response']?.lyricsList?.structuredLyrics || [] +const retryDelayFromError = (error) => { + const serverMessage = + error?.body?.['subsonic-response']?.error?.message || error?.message || '' + const matches = [...String(serverMessage).matchAll(COOLDOWN_PATTERN)] + if (matches.length === 0) return null + const seconds = Math.max(...matches.map((match) => Number(match[1]))) + return Number.isFinite(seconds) && seconds > 0 ? seconds * 1000 : null +} + const buildCacheKey = ({ trackId, preferredLanguage, updatedAt }) => [trackId || '', updatedAt || '', preferredLanguage || ''].join('\u0000') @@ -117,8 +127,10 @@ const useEnhancedLyrics = ({ layers: emptyLyricLayers, loading: false, error: null, + retryAt: null, })) const [requestGeneration, setRequestGeneration] = useState(0) + const [, setCountdownTick] = useState(0) const retry = useCallback(() => { cache.delete(cacheKey) @@ -132,13 +144,20 @@ const useEnhancedLyrics = ({ layers: emptyLyricLayers, loading: false, error: null, + retryAt: null, }) return undefined } const cached = readCachedLyrics(cacheKey) if (cached) { - setState({ cacheKey, layers: cached, loading: false, error: null }) + setState({ + cacheKey, + layers: cached, + loading: false, + error: null, + retryAt: null, + }) return undefined } @@ -153,12 +172,19 @@ const useEnhancedLyrics = ({ layers: emptyLyricLayers, loading: true, error: null, + retryAt: null, }) request.promise .then((layers) => { if (!active) return - setState({ cacheKey, layers, loading: false, error: null }) + setState({ + cacheKey, + layers, + loading: false, + error: null, + retryAt: null, + }) }) .catch((error) => { if (!active || error?.name === 'AbortError') return @@ -168,6 +194,10 @@ const useEnhancedLyrics = ({ layers: emptyLyricLayers, loading: false, error, + retryAt: (() => { + const delay = retryDelayFromError(error) + return delay == null ? null : Date.now() + delay + })(), }) }) @@ -184,11 +214,34 @@ const useEnhancedLyrics = ({ trackId, ]) + useEffect(() => { + if ( + state.cacheKey !== cacheKey || + state.retryAt == null || + disabled || + !requested + ) { + return undefined + } + + const retryDelay = Math.max(0, state.retryAt - Date.now()) + const retryTimer = window.setTimeout(retry, retryDelay) + const countdownTimer = window.setInterval( + () => setCountdownTick((current) => current + 1), + 1_000, + ) + return () => { + window.clearTimeout(retryTimer) + window.clearInterval(countdownTimer) + } + }, [cacheKey, disabled, requested, retry, state.cacheKey, state.retryAt]) + if (state.cacheKey !== cacheKey) { return { layers: emptyLyricLayers, loading: Boolean(trackId && !disabled && requested), error: null, + retryAfterSeconds: null, retry, } } @@ -196,6 +249,10 @@ const useEnhancedLyrics = ({ layers: state.layers, loading: state.loading, error: state.error, + retryAfterSeconds: + state.retryAt == null + ? null + : Math.max(1, Math.ceil((state.retryAt - Date.now()) / 1_000)), retry, } } diff --git a/ui/src/audioplayer/useEnhancedLyrics.test.js b/ui/src/audioplayer/useEnhancedLyrics.test.js index a1e26c43d..9ad835cc2 100644 --- a/ui/src/audioplayer/useEnhancedLyrics.test.js +++ b/ui/src/audioplayer/useEnhancedLyrics.test.js @@ -54,6 +54,7 @@ describe('useEnhancedLyrics', () => { }) afterEach(() => { + vi.useRealTimers() localStorage.clear() clearEnhancedLyricsCache() }) @@ -130,6 +131,33 @@ describe('useEnhancedLyrics', () => { expect(subsonic.getLyricsBySongId).toHaveBeenCalledTimes(2) }) + it('reports a cooldown countdown and retries the current song when it expires', async () => { + vi.useFakeTimers() + vi.setSystemTime(new Date('2026-08-23T06:00:00Z')) + const cooldownError = new Error('better lyrics API cooldown active for 2s') + subsonic.getLyricsBySongId + .mockRejectedValueOnce(cooldownError) + .mockResolvedValueOnce(responseFor('Recovered after cooldown')) + + const { result } = renderLyrics({ trackId: 'rate-limited-song' }) + await act(async () => { + await Promise.resolve() + }) + + expect(result.current.error).toBe(cooldownError) + expect(result.current.retryAfterSeconds).toBe(2) + + await act(async () => { + vi.advanceTimersByTime(2_000) + await Promise.resolve() + }) + + expect(subsonic.getLyricsBySongId).toHaveBeenCalledTimes(2) + expect(result.current.layers.main?.line[0].value).toBe( + 'Recovered after cooldown', + ) + }) + it('keeps cached lyrics separate by preferred language', async () => { subsonic.getLyricsBySongId .mockResolvedValueOnce(responseFor('English lyrics', 'en')) diff --git a/ui/src/audioplayer/usePlayerLyrics.jsx b/ui/src/audioplayer/usePlayerLyrics.jsx index edbb6ae3d..9c09e8cb2 100644 --- a/ui/src/audioplayer/usePlayerLyrics.jsx +++ b/ui/src/audioplayer/usePlayerLyrics.jsx @@ -27,6 +27,7 @@ const usePlayerLyrics = ({ layers: lyricLayers, loading: lyricsLoading, error: lyricsError, + retryAfterSeconds: lyricsRetryAfterSeconds, retry: retryLyrics, } = useEnhancedLyrics({ trackId, @@ -56,6 +57,12 @@ const usePlayerLyrics = ({ toggle: translate('player.toggleLyricText'), loading: translate('player.lyricsLoadingText'), unavailable: translate('player.lyricsUnavailableText'), + rateLimited: + lyricsRetryAfterSeconds == null + ? null + : translate('player.lyricsRateLimitedText', { + smart_count: lyricsRetryAfterSeconds, + }), empty: translate('player.emptyLyricText'), resize: translate('player.resizeLyricsSidebarText'), showTranslation: translate('player.showLyricsTranslationText'), @@ -63,7 +70,7 @@ const usePlayerLyrics = ({ showPronunciation: translate('player.showLyricsPronunciationText'), hidePronunciation: translate('player.hideLyricsPronunciationText'), }), - [translate], + [lyricsRetryAfterSeconds, translate], ) const toggleLyrics = useCallback(() => { @@ -118,6 +125,7 @@ const usePlayerLyrics = ({ audioInstance, loading: lyricsLoading, error: lyricsError, + errorMessage: labels.rateLimited, labels, obscuredByQueue, returnFocusRef: lyricsToggleRef, @@ -160,6 +168,7 @@ const usePlayerLyrics = ({ audioInstance={audioInstance} loading={lyricsLoading} error={lyricsError} + errorMessage={labels.rateLimited} labels={labels} inline /> diff --git a/ui/src/i18n/en.json b/ui/src/i18n/en.json index c82add952..6697d4053 100644 --- a/ui/src/i18n/en.json +++ b/ui/src/i18n/en.json @@ -660,6 +660,7 @@ "lyricsTitleText": "Lyrics", "lyricsLoadingText": "Loading lyrics", "lyricsUnavailableText": "Lyrics unavailable", + "lyricsRateLimitedText": "Remote lyrics are rate-limited. Retrying in %{smart_count} second. |||| Remote lyrics are rate-limited. Retrying in %{smart_count} seconds.", "resizeLyricsSidebarText": "Resize lyrics sidebar", "showLyricsTranslationText": "Show translation", "hideLyricsTranslationText": "Hide translation", diff --git a/ui/src/subsonic/index.js b/ui/src/subsonic/index.js index 63a56f647..8fb0be528 100644 --- a/ui/src/subsonic/index.js +++ b/ui/src/subsonic/index.js @@ -131,11 +131,24 @@ const getTopSongs = (artist, count = 50) => { return httpClient(url('getTopSongs', null, { artist, count })) } +const requireSuccessfulLyricsResponse = (response) => { + const subsonicResponse = response?.json?.['subsonic-response'] + if (subsonicResponse?.status !== 'failed') return response + + const error = new Error( + subsonicResponse.error?.message || 'Lyrics request failed', + ) + error.body = response.json + error.code = subsonicResponse.error?.code + throw error +} + const getLyricsBySongId = (id, requestOptions) => { const requestUrl = url('getLyricsBySongId', id, { enhanced: true }) - return requestOptions + const request = requestOptions ? httpClient(requestUrl, requestOptions) : httpClient(requestUrl) + return request.then(requireSuccessfulLyricsResponse) } const streamUrl = (id, options) => { diff --git a/ui/src/subsonic/index.test.js b/ui/src/subsonic/index.test.js index 4e163b5e2..d50e686b8 100644 --- a/ui/src/subsonic/index.test.js +++ b/ui/src/subsonic/index.test.js @@ -207,6 +207,25 @@ describe('getLyricsBySongId', () => { expect(requestedUrl).toContain('id=song-123') expect(requestedUrl).toContain('enhanced=true') }) + + it('rejects a failed Subsonic lyrics response with its server message', async () => { + httpClient.mockResolvedValueOnce({ + json: { + 'subsonic-response': { + status: 'failed', + error: { + code: 0, + message: + 'Internal Server Error: better lyrics API cooldown active for 30s', + }, + }, + }, + }) + + await expect(subsonic.getLyricsBySongId('song-123')).rejects.toThrow( + 'better lyrics API cooldown active for 30s', + ) + }) }) describe('getDiscCoverArtUrl', () => {