feat(android): PlaybackErrorReporter — surface ExoPlayer skip-on-error (audit v3 §4.1)
The audit called this the worst kind of bug — when a track fails
to load (404, decoder failure, premature EOS, network drop)
ExoPlayer just silently skips to the next, hiding exactly the
signal that distinguishes "this file is broken" from "the app is
flaky".
* PlayerController.onPlayerError now emits the failing track's
title onto a CONFLATED Channel exposed as playbackErrorEvents:
Flow<String>. Conflated rather than buffered so a stuck loop
can't pile up; the reporter's debounce coalesces anyway.
* PlaybackErrorReporter @Singleton collects the per-error stream,
buffers in a 2-second debounce window, and emits coalesced
user-facing strings:
1 error → "Couldn't play \"X\" — skipping"
N errors → "Skipped N unplayable tracks"
Matches the Flutter reporter shape so users see one toast on a
network blip that kills N tracks instead of a stack of N toasts.
* PlaybackErrorViewModel bridges the reporter's Flow into Hilt's
ViewModel layer so ShellScaffold can collectAsState via
hiltViewModel().
* ShellScaffold adds a second LaunchedEffect that pipes the
reporter messages into the existing shared snackbar — no new
UI surface needed.
* MinstrelApplication uses the construct-the-singleton trick to
start the reporter at app launch so it's collecting from the
first Media3 connect.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -14,6 +14,7 @@ import com.fabledsword.minstrel.events.LiveEventsDispatcher
|
||||
import com.fabledsword.minstrel.metadata.FreshnessSweeper
|
||||
import com.fabledsword.minstrel.player.CoverPrefetcher
|
||||
import com.fabledsword.minstrel.player.PlayEventsReporter
|
||||
import com.fabledsword.minstrel.player.PlaybackErrorReporter
|
||||
import com.fabledsword.minstrel.player.ResumeController
|
||||
import com.fabledsword.minstrel.update.data.VersionCheckController
|
||||
import dagger.hilt.android.HiltAndroidApp
|
||||
@@ -73,6 +74,16 @@ class MinstrelApplication :
|
||||
*/
|
||||
@Suppress("unused") @Inject lateinit var playEventsReporter: PlayEventsReporter
|
||||
|
||||
/**
|
||||
* Same construct-the-singleton trick — PlaybackErrorReporter
|
||||
* subscribes to PlayerController.playbackErrorEvents and emits
|
||||
* coalesced "Couldn't play X" / "Skipped N tracks" messages on
|
||||
* its own Flow, which ShellScaffold surfaces in the shared
|
||||
* snackbar. Without this construct ExoPlayer skip-on-error is
|
||||
* invisible to the user.
|
||||
*/
|
||||
@Suppress("unused") @Inject lateinit var playbackErrorReporter: PlaybackErrorReporter
|
||||
|
||||
/**
|
||||
* Same construct-the-singleton trick — CoverPrefetcher's init
|
||||
* block subscribes to PlayerController.uiState and warms Coil's
|
||||
|
||||
@@ -0,0 +1,68 @@
|
||||
package com.fabledsword.minstrel.player
|
||||
|
||||
import com.fabledsword.minstrel.di.ApplicationScope
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.channels.Channel
|
||||
import kotlinx.coroutines.delay
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.receiveAsFlow
|
||||
import kotlinx.coroutines.launch
|
||||
import javax.inject.Inject
|
||||
import javax.inject.Singleton
|
||||
|
||||
private const val DEBOUNCE_MS = 2_000L
|
||||
|
||||
/**
|
||||
* Surfaces ExoPlayer track-load failures (404 / decoder failure /
|
||||
* premature EOS / network drop) as user-visible snackbar messages.
|
||||
*
|
||||
* Without this the player just silently skips the dead track — the
|
||||
* worst kind of bug, because the user can't tell "this file is
|
||||
* broken" from "the app is flaky".
|
||||
*
|
||||
* Pattern mirrors Flutter's `playback_error_reporter.dart`: collect
|
||||
* per-error events from [PlayerController.playbackErrorEvents],
|
||||
* debounce in a 2s window, and emit "Couldn't play 'X' — skipping"
|
||||
* for a single error or "Skipped N unplayable tracks" when a burst
|
||||
* lands inside the window. Stops the user from seeing a stack of N
|
||||
* toasts on a network blip that fails N tracks in a row.
|
||||
*
|
||||
* ShellScaffold collects [messages] into its existing snackbar host
|
||||
* so the reporter doesn't need its own UI surface. Constructed at
|
||||
* app launch via the construct-the-singleton trick in
|
||||
* [com.fabledsword.minstrel.MinstrelApplication].
|
||||
*/
|
||||
@Singleton
|
||||
class PlaybackErrorReporter @Inject constructor(
|
||||
private val playerController: PlayerController,
|
||||
@ApplicationScope private val scope: CoroutineScope,
|
||||
) {
|
||||
private val outChannel = Channel<String>(Channel.BUFFERED)
|
||||
|
||||
/** Coalesced user-facing snackbar text. Subscribers should show in a SnackbarHost. */
|
||||
val messages: Flow<String> = outChannel.receiveAsFlow()
|
||||
|
||||
init {
|
||||
scope.launch {
|
||||
val buffer = mutableListOf<String>()
|
||||
var debounceJob: kotlinx.coroutines.Job? = null
|
||||
playerController.playbackErrorEvents.collect { title ->
|
||||
buffer.add(title)
|
||||
debounceJob?.cancel()
|
||||
debounceJob = scope.launch {
|
||||
delay(DEBOUNCE_MS)
|
||||
if (buffer.isEmpty()) return@launch
|
||||
val n = buffer.size
|
||||
val first = buffer.first()
|
||||
buffer.clear()
|
||||
val msg = if (n == 1) {
|
||||
"Couldn't play \"$first\" — skipping"
|
||||
} else {
|
||||
"Skipped $n unplayable tracks"
|
||||
}
|
||||
outChannel.trySend(msg)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -12,9 +12,12 @@ import com.fabledsword.minstrel.di.ApplicationScope
|
||||
import com.fabledsword.minstrel.models.TrackRef
|
||||
import dagger.hilt.android.qualifiers.ApplicationContext
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.channels.Channel
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.StateFlow
|
||||
import kotlinx.coroutines.flow.asStateFlow
|
||||
import kotlinx.coroutines.flow.receiveAsFlow
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.suspendCancellableCoroutine
|
||||
import javax.inject.Inject
|
||||
@@ -59,6 +62,16 @@ class PlayerController @Inject constructor(
|
||||
private val uiStateInternal = MutableStateFlow(PlayerUiState())
|
||||
val uiState: StateFlow<PlayerUiState> = uiStateInternal.asStateFlow()
|
||||
|
||||
/**
|
||||
* Per-error events for [com.fabledsword.minstrel.player.PlaybackErrorReporter].
|
||||
* Emits the failing track's title (or `"Track"` when unknown) each
|
||||
* time Media3's `onPlayerError` fires — not bound to the lifetime of
|
||||
* any UI subscriber so a torn-down screen doesn't drop events.
|
||||
* Conflated channel so backpressure can't stall the player loop.
|
||||
*/
|
||||
private val playbackErrorEventsChannel = Channel<String>(Channel.CONFLATED)
|
||||
val playbackErrorEvents: Flow<String> = playbackErrorEventsChannel.receiveAsFlow()
|
||||
|
||||
/**
|
||||
* Stable queue snapshot kept in sync with the player's MediaItems —
|
||||
* the player's own getMediaItem(index) returns Media3 types; we keep
|
||||
@@ -181,6 +194,15 @@ class PlayerController @Inject constructor(
|
||||
mediaController = controller
|
||||
controller.addListener(
|
||||
object : Player.Listener {
|
||||
override fun onPlayerError(error: androidx.media3.common.PlaybackException) {
|
||||
val title = queueRefs
|
||||
.getOrNull(controller.currentMediaItemIndex)
|
||||
?.title
|
||||
?.takeIf { it.isNotEmpty() }
|
||||
?: "Track"
|
||||
playbackErrorEventsChannel.trySend(title)
|
||||
}
|
||||
|
||||
override fun onEvents(player: Player, events: Player.Events) {
|
||||
val idx = player.currentMediaItemIndex
|
||||
val current = queueRefs.getOrNull(idx)
|
||||
|
||||
+21
@@ -0,0 +1,21 @@
|
||||
package com.fabledsword.minstrel.player.ui
|
||||
|
||||
import androidx.lifecycle.ViewModel
|
||||
import com.fabledsword.minstrel.player.PlaybackErrorReporter
|
||||
import dagger.hilt.android.lifecycle.HiltViewModel
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import javax.inject.Inject
|
||||
|
||||
/**
|
||||
* Hilt-injectable wrapper exposing [PlaybackErrorReporter.messages]
|
||||
* to ShellScaffold. The reporter itself is an app-scoped singleton;
|
||||
* this VM just bridges its Flow into a `hiltViewModel()`-resolvable
|
||||
* surface so ShellScaffold can collect it without an EntryPoint
|
||||
* accessor.
|
||||
*/
|
||||
@HiltViewModel
|
||||
class PlaybackErrorViewModel @Inject constructor(
|
||||
reporter: PlaybackErrorReporter,
|
||||
) : ViewModel() {
|
||||
val messages: Flow<String> = reporter.messages
|
||||
}
|
||||
@@ -16,6 +16,7 @@ import com.fabledsword.minstrel.connectivity.ui.ConnectionErrorBanner
|
||||
import com.fabledsword.minstrel.nav.AlbumDetail
|
||||
import com.fabledsword.minstrel.nav.ArtistDetail
|
||||
import com.fabledsword.minstrel.player.ui.MiniPlayer
|
||||
import com.fabledsword.minstrel.player.ui.PlaybackErrorViewModel
|
||||
import com.fabledsword.minstrel.update.ui.VersionTooOldBanner
|
||||
import com.fabledsword.minstrel.shared.widgets.trackactions.TrackActionsViewModel
|
||||
|
||||
@@ -45,6 +46,7 @@ fun ShellScaffold(
|
||||
onExpandPlayer: () -> Unit,
|
||||
modifier: Modifier = Modifier,
|
||||
trackActionsViewModel: TrackActionsViewModel = hiltViewModel(),
|
||||
playbackErrorViewModel: PlaybackErrorViewModel = hiltViewModel(),
|
||||
content: @Composable () -> Unit,
|
||||
) {
|
||||
val snackbarHostState = remember { SnackbarHostState() }
|
||||
@@ -53,6 +55,11 @@ fun ShellScaffold(
|
||||
snackbarHostState.showSnackbar(msg)
|
||||
}
|
||||
}
|
||||
LaunchedEffect(Unit) {
|
||||
playbackErrorViewModel.messages.collect { msg ->
|
||||
snackbarHostState.showSnackbar(msg)
|
||||
}
|
||||
}
|
||||
Column(modifier = modifier.fillMaxSize()) {
|
||||
// Banner slot. VersionTooOldBanner appears when the server's
|
||||
// min_client_version exceeds our build; ConnectionErrorBanner
|
||||
|
||||
Reference in New Issue
Block a user