Repository navigation
fix fetch reliability, memory leaks, and click-seek bug - #13
Conversation
Cyberfilo
commented
Apr 9, 2026
- Add fetch timeouts (8s) to all lyrics requests to prevent indefinite hangs from CORS or unresponsive servers (fixes [BUG] Lyrics load for very long, and are of not word by word #12)
- Add AbortController to cancel stale fetches on rapid song changes
- Increase server retry count from 2 to 3 for KPOE and Tidal
- Fix handleLineClick selector using wrong timestamp unit (ms * 1000)
- Fix memory leaks: clear all timeouts in disconnectedCallback
- Fix event listener leak: use stored bound references for proper removal
- Use stored bound ref for animateProgress to avoid allocation per frame
- Remove duplicate eslint-disable comments
- Replace any types with generics in GoogleService romanize methods
- Add fetch timeouts to GoogleService translate/romanize requests
- Add fetch timeouts (8s) to all lyrics requests to prevent indefinite hangs from CORS or unresponsive servers (fixes binimum#12) - Add AbortController to cancel stale fetches on rapid song changes - Increase server retry count from 2 to 3 for KPOE and Tidal - Fix handleLineClick selector using wrong timestamp unit (ms * 1000) - Fix memory leaks: clear all timeouts in disconnectedCallback - Fix event listener leak: use stored bound references for proper removal - Use stored bound ref for animateProgress to avoid allocation per frame - Remove duplicate eslint-disable comments - Replace any types with generics in GoogleService romanize methods - Add fetch timeouts to GoogleService translate/romanize requests
|
Thanks for all the patches! If you'll let me test it and get a quick second review I'll get it merged asap. |
There was a problem hiding this comment.
Pull request overview
This PR improves lyrics loading reliability and UI behavior in AmLyrics by adding request timeouts, reducing leaks, and fixing a click-to-seek/scroll targeting issue, plus tightening typing and adding timeouts in GoogleService’s Google Translate requests.
Changes:
- Add fetch timeouts across lyrics-provider requests and Google Translate/romanize calls.
- Introduce cancellation plumbing and cleanup to reduce leaks (timeouts, event listeners, animation frame binding).
- Adjust provider fallback breadth (try 3 servers) and fix click-to-seek line lookup to match millisecond timestamps.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/AmLyrics.ts |
Adds fetch timeouts, attempts stale-fetch cancellation, fixes click-to-seek selector, and cleans up timeouts/listeners/RAF usage. |
src/GoogleService.ts |
Adds fetch timeouts for translate/romanize calls and replaces any with a generic, typed romanization shape. |
Comments suppressed due to low confidence (1)
src/AmLyrics.ts:1663
fetchLyricsaborts the previous controller, but most awaited work below (provider fetches) doesn't usecontroller.signal, and state updates (e.g., settingavailableSources/lyrics) aren't guarded after each await. As a result, a superseded request can still complete and overwrite UI state even though its controller was aborted.
Recommend propagating controller.signal into all fetch/provider helpers and checking controller.signal.aborted before committing results to component state.
private async fetchLyrics() {
// Cancel any in-flight fetch to prevent stale results from racing
this.fetchAbortController?.abort();
const controller = new AbortController();
this.fetchAbortController = controller;
this.isLoading = true;
this.lyrics = undefined;
this.lyricsSource = null;
this.availableSources = [];
this.currentSourceIndex = 0;
this.isFetchingAlternatives = false;
this.hasFetchedAllProviders = false;
try {
const resolvedMetadata = await this.resolveSongMetadata();
// If a newer fetch was triggered while we awaited, bail out
if (controller.signal.aborted) return;
const isMusicIdOnlyRequest =
Boolean(this.musicId) &&
!this.songTitle &&
!this.songArtist &&
!this.query;
const collectedSources: { lines: LyricsLine[]; source: string }[] = [];
if (resolvedMetadata?.metadata && !isMusicIdOnlyRequest) {
const title = resolvedMetadata.metadata.title?.trim() || '';
const artist = resolvedMetadata.metadata.artist?.trim() || '';
const youLyResults = await AmLyrics.fetchLyricsFromYouLyPlus(
title,
artist,
resolvedMetadata.metadata,
);
if (youLyResults && youLyResults.length > 0) {
collectedSources.push(...youLyResults);
}
}
if (collectedSources.length === 0 && resolvedMetadata?.metadata) {
const tidalResult = await AmLyrics.fetchLyricsFromTidal(
resolvedMetadata.metadata,
resolvedMetadata.catalogIsrc,
);
if (tidalResult && tidalResult.lines.length > 0) {
collectedSources.push({
lines: tidalResult.lines,
source: 'Tidal',
});
}
}
// Fallback: LRCLIB
if (collectedSources.length === 0 && resolvedMetadata?.metadata) {
const lrclibResult = await AmLyrics.fetchLyricsFromLrclib(
resolvedMetadata.metadata,
);
if (lrclibResult && lrclibResult.lines.length > 0) {
collectedSources.push({
lines: lrclibResult.lines,
source: 'LRCLIB',
});
}
}
if (collectedSources.length === 0 && resolvedMetadata?.metadata) {
const geniusResult = await AmLyrics.fetchLyricsFromGenius(
resolvedMetadata.metadata,
);
if (geniusResult && geniusResult.lines.length > 0) {
collectedSources.push({
lines: geniusResult.lines,
source: 'Genius',
});
}
}
this.hasFetchedAllProviders =
collectedSources.length === 0 ||
collectedSources.some(
s =>
s.source === 'LRCLIB' ||
s.source === 'Tidal' ||
s.source === 'Genius',
);
if (collectedSources.length > 0) {
this.availableSources = AmLyrics.mergeAndSortSources(collectedSources);
this.currentSourceIndex = 0;
this.lyrics = this.availableSources[0].lines;
this.lyricsSource = this.availableSources[0].source;
await this.onLyricsLoaded();
return;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.