Skip songs that can't be played instead of stopping

Skipping through songs could leave the player stopped with Next/Previous
only changing the title. Cause: a song whose file is missing on the
server; Navidrome answers /rest/stream with a JSON error marked HTTP 200,
which was cached as the song, so the player failed to read it and stopped.

- Song downloads with a text/JSON/XML Content-Type are refused, so server
  error messages are never cached or played.
- SkipUnplayable: on a playback error, remove the song's cached copy,
  show a short note and skip to the next song (waits for the rest of the
  queue if it is still arriving; at most 3 in a row; not on network errors).
- Next/Previous re-prepare a stopped player.

Verified on the emulator against the owner's server: the broken song is
skipped with a note, the bad cached copy is removed and not re-saved,
10 rounds of rapid next/previous never left the player stopped, and after
an offline failure one tap on Next plays again. 84 unit tests pass.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

CLAUDE.md

@@ -89,8 +89,10 @@ Key rules:
 adb -s emulator-5554 install -r app/build/outputs/apk/debug/app-debug.apk
 ```
 
-- 77 JVM unit tests (MockWebServer for API calls incl. Deezer and scrobble, color math, A–Z
-  index, album facts, formatting, search matching / top result / recent searches, listen rules).
+- 84 JVM unit tests (MockWebServer for API calls incl. Deezer and scrobble, color math, A–Z
+  index, album facts, formatting, search matching / top result / recent searches, listen rules,
+  unplayable-song rules).
+- Songs that fail to play: `adb logcat -s SyncWavePlayback`.
 - Scrobbling: `adb logcat -s SyncWaveScrobble` shows now playing / sent / kept for later. Saved
   listens: `adb shell run-as com.tysmi.syncwave cat shared_prefs/scrobbles.xml`.
 - For testing without the owner's server: Navidrome demo, `demo.navidrome.org`,
@@ -193,6 +195,15 @@ adb -s emulator-5554 install -r app/build/outputs/apk/debug/app-debug.apk
    pull-to-refresh keeps old cards until new ones arrive. Rediscover and Recently Played reload
    each time Home is shown again, except when coming back from a page opened from Home.
 
+15. Unplayable songs (2026-10-14): owner reported "skipping a few songs froze the app". Cause: a
+   song whose file is missing on the server (Navidrome answers `stream` with a JSON error marked
+   HTTP 200); it was cached as audio, the player failed and stopped, and Next/Previous then only
+   changed the title. Now: song downloads with a text/JSON/XML Content-Type are refused
+   (`UnplayableRules.mayBeAudio`, never cached); `SkipUnplayable` (PlaybackService) removes the
+   failed song's cached copy, shows a toast and skips to the next (waits if the rest of the queue
+   is still arriving; at most 3 in a row; not on network errors); `PlayOnSkipPlayer` re-prepares a
+   stopped player on skip. Verified on the emulator against the owner's server.
+
 ## In progress / unverified
 
 The owner has build 1308fb6 (2026-10-13) on their phone: everything in Done above. Collect
@@ -216,6 +227,9 @@ their feedback on what only the phone can show:
   labels and disc headings on album pages (demo library is single-disc FLAC).
 - **Never tested:** lock-screen controls (emulator has no screen lock), themed (monochrome)
   app icon.
+- **Owner's server:** "Ain’t No Fun (Waiting Round to Be a Millionaire)" (AC/DC, Dirty Deeds…)
+  is listed by Navidrome but its file is missing ("no such file or directory"; maybe the curly
+  apostrophe). A full library rescan in Navidrome should fix or drop it. Now skipped with a note.
 - Known limits: album sort resets on restart; artist pages load songs one album at a time;
   Navidrome login sends the password in a POST body (plain over `http://`); Download buttons
   and the Downloaded page are placeholders; Lyrics and Cast buttons say "coming soon".

app/src/main/java/com/tysmi/syncwave/playback/PlaybackService.kt

@@ -55,7 +55,9 @@ class PlaybackService : MediaLibraryService() {
 
         // Audio and notification artwork both load through the client that signs requests
         // to the logged-in server, so no credentials are ever put in a URL.
+        // Error messages from the server (sent as "success") are refused, so they're never cached as a song.
         val httpDataSource = OkHttpDataSource.Factory(container.authenticatedHttpClient)
+            .setContentTypePredicate { UnplayableRules.mayBeAudio(it) }
         // Audio goes through the on-phone cache: replays read from disk, new songs are saved
         // while they play. Stream URLs carry no credentials, so cache keys stay stable.
         val audioSource = CacheDataSource.Factory()
@@ -117,6 +119,7 @@ class PlaybackService : MediaLibraryService() {
         // lock screen (or with the app closed) count too.
         player.addListener(container.scrobbler.listenerFor(player))
         container.sleepTimer.attach(player)
+        player.addListener(SkipUnplayable(this, player, container.audioCache.cache, scope))
         player.addListener(Autoplay(player, container.libraryRepository, container.mediaItemFactory, container.playbackSettings, scope))
 
         // Signing out stops the music and forgets the queue.
@@ -236,15 +239,19 @@ private object TraceListener : AnalyticsListener {
         PlaybackTrace.finish("FIRST SOUND")
 }
 
-/** Skipping to the next or previous song while paused also starts playing it. */
+/**
+ * Skipping to the next or previous song while paused also starts playing it, and so does
+ * skipping after a song failed (the player stops until it's prepared again).
+ */
 @OptIn(UnstableApi::class)
 private class PlayOnSkipPlayer(player: Player) : ForwardingPlayer(player) {
-    override fun seekToNext() = super.seekToNext().also { playIfPaused() }
-    override fun seekToNextMediaItem() = super.seekToNextMediaItem().also { playIfPaused() }
-    override fun seekToPrevious() = super.seekToPrevious().also { playIfPaused() }
-    override fun seekToPreviousMediaItem() = super.seekToPreviousMediaItem().also { playIfPaused() }
+    override fun seekToNext() = super.seekToNext().also { playAfterSkip() }
+    override fun seekToNextMediaItem() = super.seekToNextMediaItem().also { playAfterSkip() }
+    override fun seekToPrevious() = super.seekToPrevious().also { playAfterSkip() }
+    override fun seekToPreviousMediaItem() = super.seekToPreviousMediaItem().also { playAfterSkip() }
 
-    private fun playIfPaused() {
+    private fun playAfterSkip() {
+        if (playbackState == Player.STATE_IDLE) prepare()
         if (!playWhenReady) play()
     }
 }

app/src/main/java/com/tysmi/syncwave/playback/UnplayableSongs.kt

@@ -0,0 +1,113 @@
+package com.tysmi.syncwave.playback
+
+import android.content.Context
+import android.util.Log
+import android.widget.Toast
+import androidx.annotation.OptIn
+import androidx.media3.common.MediaItem
+import androidx.media3.common.PlaybackException
+import androidx.media3.common.Player
+import androidx.media3.common.Timeline
+import androidx.media3.common.util.UnstableApi
+import androidx.media3.datasource.cache.Cache
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.launch
+
+/**
+ * When a song can't be played (its file is missing on the server, or the download isn't audio),
+ * the player stops and would ignore Next / Previous. Instead: forget any saved copy of it, say so
+ * briefly, and carry on with the next song. Gives up after a few failures in a row, and doesn't
+ * skip ahead when the server can't be reached (every song would fail; tapping Play retries).
+ */
+@OptIn(UnstableApi::class)
+class SkipUnplayable(
+    private val context: Context,
+    private val player: Player,
+    private val cache: Cache,
+    private val scope: CoroutineScope,
+) : Player.Listener {
+    private var failuresInARow = 0
+
+    /**
+     * Failed while the rest of the queue was still on its way (a tapped song is sent first):
+     * skip as soon as there's a next song.
+     */
+    private var skipWhenQueued = false
+
+    override fun onIsPlayingChanged(isPlaying: Boolean) {
+        if (isPlaying) {
+            failuresInARow = 0
+            skipWhenQueued = false
+        }
+    }
+
+    override fun onTimelineChanged(timeline: Timeline, reason: Int) {
+        if (skipWhenQueued && player.playerError != null && player.hasNextMediaItem()) {
+            skipWhenQueued = false
+            skipToNext()
+        }
+    }
+
+    override fun onMediaItemTransition(mediaItem: MediaItem?, reason: Int) {
+        skipWhenQueued = false
+    }
+
+    override fun onPlayerError(error: PlaybackException) {
+        val item = player.currentMediaItem
+        val title = item?.mediaMetadata?.title?.toString().orEmpty()
+        Log.w(TAG, "Couldn't play \"$title\"", error)
+        // A bad saved copy (e.g. a server error message kept as the song) would fail every time.
+        item?.localConfiguration?.uri?.toString()?.let { key ->
+            scope.launch(Dispatchers.IO) { runCatching { cache.removeResource(key) } }
+        }
+        failuresInARow++
+        val skip = UnplayableRules.shouldSkip(error.errorCode, failuresInARow, hasNext = true)
+        val skipNow = skip && player.hasNextMediaItem()
+        Toast.makeText(context, UnplayableRules.message(title, error.errorCode, skipNow), Toast.LENGTH_SHORT).show()
+        if (skipNow) skipToNext() else skipWhenQueued = skip
+    }
+
+    private fun skipToNext() {
+        player.seekToNextMediaItem()
+        player.prepare()
+    }
+
+    private companion object {
+        const val TAG = "SyncWavePlayback"
+    }
+}
+
+/** The decisions behind [SkipUnplayable] and the stream checks, kept apart for unit tests. */
+object UnplayableRules {
+    const val MAX_SKIPS_IN_A_ROW = 3
+
+    private val networkErrors = setOf(
+        PlaybackException.ERROR_CODE_IO_NETWORK_CONNECTION_FAILED,
+        PlaybackException.ERROR_CODE_IO_NETWORK_CONNECTION_TIMEOUT,
+    )
+
+    fun isNetworkError(errorCode: Int) = errorCode in networkErrors
+
+    fun shouldSkip(errorCode: Int, failuresInARow: Int, hasNext: Boolean): Boolean =
+        hasNext && !isNetworkError(errorCode) && failuresInARow <= MAX_SKIPS_IN_A_ROW
+
+    fun message(title: String, errorCode: Int, skipped: Boolean): String {
+        val song = if (title.isBlank()) "this song" else "“$title”"
+        return when {
+            isNetworkError(errorCode) -> "Couldn't reach the server to play $song"
+            skipped -> "Couldn't play $song, skipped"
+            else -> "Couldn't play $song"
+        }
+    }
+
+    /**
+     * Whether a song download's Content-Type can be audio. The server answers some failures
+     * (e.g. a missing file) with an error message marked as success; that must not be saved
+     * or played as the song. Unknown or missing types are allowed.
+     */
+    fun mayBeAudio(contentType: String): Boolean {
+        val type = contentType.substringBefore(';').trim().lowercase()
+        return !(type.startsWith("text/") || type.endsWith("json") || type.endsWith("xml"))
+    }
+}

app/src/test/java/com/tysmi/syncwave/playback/UnplayableRulesTest.kt

@@ -0,0 +1,61 @@
+package com.tysmi.syncwave.playback
+
+import androidx.media3.common.PlaybackException
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Test
+
+class UnplayableRulesTest {
+
+    private val badFile = PlaybackException.ERROR_CODE_PARSING_CONTAINER_UNSUPPORTED
+    private val offline = PlaybackException.ERROR_CODE_IO_NETWORK_CONNECTION_FAILED
+
+    @Test
+    fun `skips a song that can't be played`() {
+        assertTrue(UnplayableRules.shouldSkip(badFile, failuresInARow = 1, hasNext = true))
+    }
+
+    @Test
+    fun `stops at the end of the queue`() {
+        assertFalse(UnplayableRules.shouldSkip(badFile, failuresInARow = 1, hasNext = false))
+    }
+
+    @Test
+    fun `gives up after a few failures in a row`() {
+        assertTrue(UnplayableRules.shouldSkip(badFile, UnplayableRules.MAX_SKIPS_IN_A_ROW, hasNext = true))
+        assertFalse(UnplayableRules.shouldSkip(badFile, UnplayableRules.MAX_SKIPS_IN_A_ROW + 1, hasNext = true))
+    }
+
+    @Test
+    fun `doesn't skip ahead when the server can't be reached`() {
+        assertFalse(UnplayableRules.shouldSkip(offline, failuresInARow = 1, hasNext = true))
+        assertFalse(UnplayableRules.shouldSkip(PlaybackException.ERROR_CODE_IO_NETWORK_CONNECTION_TIMEOUT, 1, true))
+    }
+
+    @Test
+    fun `messages name the song`() {
+        assertEquals("Couldn't play “Ain't No Fun”, skipped", UnplayableRules.message("Ain't No Fun", badFile, skipped = true))
+        assertEquals("Couldn't play “Ain't No Fun”", UnplayableRules.message("Ain't No Fun", badFile, skipped = false))
+        assertEquals("Couldn't reach the server to play this song", UnplayableRules.message("", offline, skipped = false))
+    }
+
+    @Test
+    fun `server error messages are not audio`() {
+        assertFalse(UnplayableRules.mayBeAudio("application/json"))
+        assertFalse(UnplayableRules.mayBeAudio("application/json; charset=utf-8"))
+        assertFalse(UnplayableRules.mayBeAudio("text/xml"))
+        assertFalse(UnplayableRules.mayBeAudio("application/xml"))
+        assertFalse(UnplayableRules.mayBeAudio("text/html"))
+        assertFalse(UnplayableRules.mayBeAudio("Application/JSON"))
+    }
+
+    @Test
+    fun `audio and unknown types are allowed`() {
+        assertTrue(UnplayableRules.mayBeAudio("audio/flac"))
+        assertTrue(UnplayableRules.mayBeAudio("audio/mp4"))
+        assertTrue(UnplayableRules.mayBeAudio("audio/mpeg"))
+        assertTrue(UnplayableRules.mayBeAudio("application/octet-stream"))
+        assertTrue(UnplayableRules.mayBeAudio(""))
+    }
+}