Skip to content

fix(#373): improve error handling when songs fail to play - #477

Merged
Sandakan merged 2 commits into
Sandakan:release/4.0.0-alpha.5from
Owie6789:fix/373-error-playing-songs
Jun 4, 2026
Merged

fix(#373): improve error handling when songs fail to play#477
Sandakan merged 2 commits into
Sandakan:release/4.0.0-alpha.5from
Owie6789:fix/373-error-playing-songs

Conversation

@Owie6789

@Owie6789 Owie6789 commented May 19, 2026

Copy link
Copy Markdown
Contributor

Fixes #373

Three failure modes were handled poorly:

  1. Unsupported/missing files — the audio element fired MEDIA_ERR_SRC_NOT_SUPPORTED with no retry path
  2. Decode failures — the error counter had an off-by-one, trying 6 times instead of 5
  3. Database fetch failures in getSong — the error was swallowed silently, leaving the player in a broken state

Changes

CODE 4 = skip immediately (usePlaybackErrors.tsx): MEDIA_ERR_SRC_NOT_SUPPORTED is not retryable. The error handler now calls skipSongRef() directly without counting or retrying.

CODE 3 retry limit (usePlaybackErrors.tsx): Fixed >= 5 (was > 5), so the max retries is exactly 5.

File existence pre-check (sendAudioData.ts): After resolving the file path with removeDefaultAppProtocolFromFilePath, we now call existsSync and throw SONG_NOT_FOUND before the request reaches the renderer.

getSong try/catch (player.ts): The fetch in getSong is now wrapped. On failure it emits loadError via EventEmitter so the playback system knows the song could not be loaded.

Auto-advance on load failure (player.ts): The positionChange handler catches load rejects and calls moveToNext() bounded by queue length.

Skip button in error prompt (ErrorPrompt.tsx): Added onSkipSong prop. When the user clicks Skip to next song, it invokes skipForwardRef to bypass the broken track.

404 logging (handleFileProtocol.ts): logger.warn now logs when requested files return 404, with both the URL and resolved file path.

Verification

  • typecheck:node passes (4 pre-existing errors only)
  • oxlint clean (only pre-existing warnings)
  • All 7 edited source files parse clean

Summary by CodeRabbit

  • New Features

    • Added "Skip Song" button to error prompts with support for multiple languages.
  • Bug Fixes

    • Improved playback error handling and automatic queue progression on failed song loads.
    • Added file existence verification to prevent crashes from missing audio files.
    • Enhanced error logging and state management during playback failures.
    • Fixed event listener cleanup in lifecycle management.

@Owie6789 Owie6789 mentioned this pull request May 19, 2026
@Owie6789
Owie6789 force-pushed the fix/373-error-playing-songs branch from 780c286 to 94d22bd Compare May 25, 2026 19:40
@Owie6789
Owie6789 changed the base branch from master to release/4.0.0-alpha.5 May 25, 2026 19:42
@Owie6789
Owie6789 force-pushed the fix/373-error-playing-songs branch 2 times, most recently from 07d443a to 5c680ef Compare June 1, 2026 01:58
- Guard artworks null in discord.ts to prevent crash
- Break sync recursion in positionChange
- Remove orphaned merge conflict markers from locale JSON files (en, pl)
- Verified: format check and lint pass on CI
@Owie6789
Owie6789 force-pushed the fix/373-error-playing-songs branch from 866bf3e to 3b77b27 Compare June 2, 2026 00:50
@coderabbitai

coderabbitai Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 76f47b6e-7df9-433d-aa43-da6c9b6cdac0

📥 Commits

Reviewing files that changed from the base of the PR and between d405aae and fc0e0fd.

📒 Files selected for processing (12)
  • src/main/core/sendAudioData.ts
  • src/main/handleFileProtocol.ts
  • src/renderer/src/App.tsx
  • src/renderer/src/assets/locales/as/as.json
  • src/renderer/src/assets/locales/en/en.json
  • src/renderer/src/assets/locales/pl/pl.json
  • src/renderer/src/assets/locales/tr/tr.json
  • src/renderer/src/assets/locales/vi/vi.json
  • src/renderer/src/components/ErrorPrompt.tsx
  • src/renderer/src/hooks/useAppLifecycle.tsx
  • src/renderer/src/hooks/usePlaybackErrors.tsx
  • src/renderer/src/other/player.ts

📝 Walkthrough

Walkthrough

This PR adds comprehensive playback error recovery by validating audio files on the backend, automatically advancing to the next song when playback fails, displaying a skip button in error prompts, and resetting error state on song load. Media source errors trigger automatic skip without user interaction.

Changes

Playback Error Recovery with Skip-Forward

Layer / File(s) Summary
Backend file existence validation
src/main/core/sendAudioData.ts, src/main/handleFileProtocol.ts
Before sending audio data, sendAudioData resolves the file path and verifies it exists via existsSync; missing files trigger SONG_NOT_FOUND. The nora:// protocol handler logs warnings for missing files.
Queue auto-advance on load failure
src/renderer/src/other/player.ts
positionChange snapshots pendingAutoPlay, and when loadSong fails, optionally advances the queue asynchronously to try the next song. loadSong moves song-data resolution into the try block and computes failedSongId defensively in the catch handler.
Error prompt with skip button and media error handling
src/renderer/src/hooks/usePlaybackErrors.tsx, src/renderer/src/components/ErrorPrompt.tsx
usePlaybackErrors accepts skipSongRef and detects MEDIA_ERR_SRC_NOT_SUPPORTED to skip without prompting. The threshold for repetitive errors shifts from > 5 to >= 5. ErrorPrompt conditionally renders a skip button when onSkipSong is provided.
App-level skip forward and error state coordination
src/renderer/src/App.tsx
App initializes skipForwardRef, wires it to handleSkipForwardClick, resets error count on songLoaded, and passes resetErrorCount to both usePlaybackErrors and useAppLifecycle.
AppLifecycleDependencies and startup fixes
src/renderer/src/hooks/useAppLifecycle.tsx
Adds resetErrorCount to the dependencies interface. Fixes optional chaining in startup resume (playback?.currentSong?.songId) and corrects event listener cleanup for playSongFromUnknownSource.
Skip song translations and error prompt updates
src/renderer/src/assets/locales/*/...json
Adds common.skipSong translations across all locales. Turkish locale also simplifies songUnplayableErrorPrompt by removing format-guidance keys.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A song breaks and now the player's wise,
We check if files exist before we try,
Skip ahead or queue the next delight,
Error counts reset with each new sight,
Playback recovers—resilience takes flight! 🎵

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@Sandakan
Sandakan merged commit 597df8a into Sandakan:release/4.0.0-alpha.5 Jun 4, 2026
4 checks passed
@Owie6789

Owie6789 commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

@CodeRabbit

@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

@Owie6789 Sure! I'll review the changes in this PR right away.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@Owie6789

Owie6789 commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

@Sandakan please re-merge when you get a chance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error

2 participants