diff --git a/android/app/src/main/java/com/fabledsword/minstrel/api/endpoints/PlaybackErrorsApi.kt b/android/app/src/main/java/com/fabledsword/minstrel/api/endpoints/PlaybackErrorsApi.kt new file mode 100644 index 00000000..d2161638 --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/api/endpoints/PlaybackErrorsApi.kt @@ -0,0 +1,33 @@ +package com.fabledsword.minstrel.api.endpoints + +import retrofit2.http.Body +import retrofit2.http.POST + +/** + * Retrofit interface for `POST /api/playback-errors`. Reports a + * client-detected playback failure (zero-duration, decode error, ...) + * so the admin inbox surfaces it. + * + * Failures during dispatch are reported via the MutationQueue path + * for offline replay (see [com.fabledsword.minstrel.cache.mutations.MutationQueue.enqueuePlaybackErrorReport]). + */ +interface PlaybackErrorsApi { + @POST("api/playback-errors") + suspend fun report(@Body body: PlaybackErrorReportRequest) +} + +/** + * POST body for `/api/playback-errors`. `kind` is one of + * "zero_duration" / "load_failed" / "stalled"; server validates against + * the CHECK constraint enum. `detail` is optional free-text (Media3 + * error message, etc.). `clientId` reuses the existing playback + * client identifier the device already sends with /api/plays/* so + * support can correlate reports across surfaces. + */ +@kotlinx.serialization.Serializable +data class PlaybackErrorReportRequest( + @kotlinx.serialization.SerialName("track_id") val trackId: String, + val kind: String, + val detail: String? = null, + @kotlinx.serialization.SerialName("client_id") val clientId: String, +) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/cache/mutations/MutationQueue.kt b/android/app/src/main/java/com/fabledsword/minstrel/cache/mutations/MutationQueue.kt index f08f8354..541178cb 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/cache/mutations/MutationQueue.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/cache/mutations/MutationQueue.kt @@ -21,6 +21,7 @@ object MutationKind { const val QUARANTINE_FLAG: String = "quarantine_flag" const val PLAY_OFFLINE: String = "play_offline" const val REQUEST_CANCEL: String = "request_cancel" + const val PLAYBACK_ERROR_REPORT: String = "playback_error_report" } /** @@ -144,6 +145,13 @@ class MutationQueue @Inject constructor( ), ), ) + + suspend fun enqueuePlaybackErrorReport(payload: PlaybackErrorReportPayload): Long = dao.insert( + CachedMutationEntity( + kind = MutationKind.PLAYBACK_ERROR_REPORT, + payload = json.encodeToString(PlaybackErrorReportPayload.serializer(), payload), + ), + ) } /** @@ -203,3 +211,20 @@ data class PlayOfflinePayload( */ @Serializable data class RequestCancelPayload(val requestId: String) + +/** + * Persisted payload for `MutationKind.PLAYBACK_ERROR_REPORT` — the + * `POST /api/playback-errors` call lost during a connectivity hiccup. + * The replayer re-fires the POST with this body. Server is naturally + * idempotent enough — multiple reports of the same (track, user, + * kind) become multiple rows in the admin inbox, which is acceptable + * (the admin can resolve them all with one action). `clientId` is + * the existing playback client identifier from AuthStore. + */ +@Serializable +data class PlaybackErrorReportPayload( + val trackId: String, + val kind: String, + val detail: String? = null, + val clientId: String, +) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/cache/mutations/MutationReplayer.kt b/android/app/src/main/java/com/fabledsword/minstrel/cache/mutations/MutationReplayer.kt index 06fb336e..4fd4c8fb 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/cache/mutations/MutationReplayer.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/cache/mutations/MutationReplayer.kt @@ -5,6 +5,8 @@ import com.fabledsword.minstrel.api.endpoints.DiscoverApi import com.fabledsword.minstrel.api.endpoints.EventsApi import com.fabledsword.minstrel.api.endpoints.FlagRequest import com.fabledsword.minstrel.api.endpoints.LikesApi +import com.fabledsword.minstrel.api.endpoints.PlaybackErrorReportRequest +import com.fabledsword.minstrel.api.endpoints.PlaybackErrorsApi import com.fabledsword.minstrel.api.endpoints.PlaylistsApi import com.fabledsword.minstrel.api.endpoints.QuarantineApi import com.fabledsword.minstrel.api.endpoints.RequestsApi @@ -66,6 +68,7 @@ class MutationReplayer @Inject constructor( private val playlistsApi: PlaylistsApi = retrofit.create() private val eventsApi: EventsApi = retrofit.create() private val requestsApi: RequestsApi = retrofit.create() + private val playbackErrorsApi: PlaybackErrorsApi = retrofit.create() private val mutex = Mutex() @@ -116,6 +119,7 @@ class MutationReplayer @Inject constructor( MutationKind.QUARANTINE_FLAG -> dispatchQuarantineFlag(row.payload) MutationKind.PLAY_OFFLINE -> dispatchPlayOffline(row.payload) MutationKind.REQUEST_CANCEL -> dispatchRequestCancel(row.payload) + MutationKind.PLAYBACK_ERROR_REPORT -> dispatchPlaybackErrorReport(row.payload) else -> { // Unknown kind — drop the row by claiming success so a // stale schema entry can't wedge the queue forever. @@ -258,4 +262,24 @@ class MutationReplayer @Inject constructor( false } } + + private suspend fun dispatchPlaybackErrorReport(payload: String): Boolean { + val decoded = json.decodeFromString(PlaybackErrorReportPayload.serializer(), payload) + return try { + playbackErrorsApi.report( + PlaybackErrorReportRequest( + trackId = decoded.trackId, + kind = decoded.kind, + detail = decoded.detail, + clientId = decoded.clientId, + ), + ) + true + } catch ( + @Suppress("TooGenericExceptionCaught", "SwallowedException") e: Throwable, + ) { + // Intentional swallow: row stays queued; next drain pass retries. + false + } + } } diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/PlaybackErrorReporter.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/PlaybackErrorReporter.kt index 12a6e775..f423cc5f 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/PlaybackErrorReporter.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/PlaybackErrorReporter.kt @@ -13,19 +13,23 @@ 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. + * Surfaces playback failures (load errors + zero-duration tracks) as + * user-visible snackbar messages AND admin-inbox reports. * - * 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". + * Without the snackbar 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". Without the admin report the + * operator never finds out the track is bad and the next user hits + * the same wall. * - * 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. + * The snackbar text mirrors Flutter's `playback_error_reporter.dart`: + * collect [PlayerController.playbackErrorEvents], debounce in a 2s + * window, emit "Couldn't play 'X' — skipping" for a single error or + * "Skipped N unplayable tracks" when a burst lands inside the window. + * + * The server report fires per event (no debounce) via + * [PlaybackErrorRepository], which handles the offline-first + * MutationQueue fallback per the standing rule for server writes. * * ShellScaffold collects [messages] into its existing snackbar host * so the reporter doesn't need its own UI surface. Constructed at @@ -35,6 +39,7 @@ private const val DEBOUNCE_MS = 2_000L @Singleton class PlaybackErrorReporter @Inject constructor( private val playerController: PlayerController, + private val repository: PlaybackErrorRepository, @ApplicationScope private val scope: CoroutineScope, ) { private val outChannel = Channel(Channel.BUFFERED) @@ -46,8 +51,11 @@ class PlaybackErrorReporter @Inject constructor( scope.launch { val buffer = mutableListOf() var debounceJob: kotlinx.coroutines.Job? = null - playerController.playbackErrorEvents.collect { title -> - buffer.add(title) + playerController.playbackErrorEvents.collect { event -> + // Fire-and-forget the server report — repository handles + // success/queue branching so callers don't see throws. + scope.launch { repository.report(event) } + buffer.add(event.title) debounceJob?.cancel() debounceJob = scope.launch { delay(DEBOUNCE_MS) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/PlaybackErrorRepository.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/PlaybackErrorRepository.kt new file mode 100644 index 00000000..197d7452 --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/PlaybackErrorRepository.kt @@ -0,0 +1,78 @@ +package com.fabledsword.minstrel.player + +import com.fabledsword.minstrel.api.endpoints.PlaybackErrorReportRequest +import com.fabledsword.minstrel.api.endpoints.PlaybackErrorsApi +import com.fabledsword.minstrel.auth.AuthStore +import com.fabledsword.minstrel.cache.mutations.MutationQueue +import com.fabledsword.minstrel.cache.mutations.PlaybackErrorReportPayload +import retrofit2.Retrofit +import retrofit2.create +import java.util.UUID +import javax.inject.Inject +import javax.inject.Singleton + +/** + * Writes a [PlaybackErrorEvent] to the server. Follows the standing + * offline-first rule for server writes: try the POST first, fall back + * to the MutationQueue on transport failure so a tracked report is + * never lost. + * + * Reused client_id comes from [AuthStore.clientId] — the same UUID- + * per-install identifier the play-events reporter already sends, so + * support can correlate playback errors with the surrounding plays. + */ +@Singleton +class PlaybackErrorRepository @Inject constructor( + private val authStore: AuthStore, + private val mutationQueue: MutationQueue, + retrofit: Retrofit, +) { + private val api: PlaybackErrorsApi = retrofit.create() + + /** + * Reports a playback failure. Returns [Outcome.ACCEPTED] on a 2xx, + * [Outcome.QUEUED] when the call was buffered to the MutationQueue + * for later replay. Never throws — callers don't need a try block. + */ + suspend fun report(event: PlaybackErrorEvent): Outcome { + val clientId = resolveClientId() + val payload = PlaybackErrorReportPayload( + trackId = event.trackId, + kind = event.kind, + detail = event.detail, + clientId = clientId, + ) + return try { + api.report( + PlaybackErrorReportRequest( + trackId = payload.trackId, + kind = payload.kind, + detail = payload.detail, + clientId = payload.clientId, + ), + ) + Outcome.ACCEPTED + } catch ( + @Suppress("TooGenericExceptionCaught", "SwallowedException") e: Throwable, + ) { + // Intentional swallow — see LikesRepository.toggleLike for + // the same offline-first rationale. + mutationQueue.enqueuePlaybackErrorReport(payload) + Outcome.QUEUED + } + } + + /** + * Returns the persisted client id, generating + persisting one on + * first read. Mirrors [PlayEventsReporter.resolveClientId] — the + * value is shared across all client-id-carrying surfaces. + */ + private fun resolveClientId(): String { + authStore.clientId.value?.let { return it } + val fresh = UUID.randomUUID().toString() + authStore.setClientId(fresh) + return fresh + } + + enum class Outcome { ACCEPTED, QUEUED } +} diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt index de653403..813f5747 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt @@ -68,13 +68,23 @@ class PlayerController @Inject constructor( /** * 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. + * Fires twice: once when Media3's `onPlayerError` surfaces a load + * failure (decoder, transport, EOS), and once when the player + * reaches STATE_READY with a duration of zero / TIME_UNSET — the + * "track loaded but has no audio" case the user sees as the + * player sitting frozen on a track. Conflated channel so back- + * pressure can't stall the player loop. */ - private val playbackErrorEventsChannel = Channel(Channel.CONFLATED) - val playbackErrorEvents: Flow = playbackErrorEventsChannel.receiveAsFlow() + private val playbackErrorEventsChannel = Channel(Channel.CONFLATED) + val playbackErrorEvents: Flow = playbackErrorEventsChannel.receiveAsFlow() + + /** + * Tracks which queue index has already been evaluated for the + * zero-duration / load-failed checks so STATE_READY firing + * repeatedly (after every seek, pause/resume) doesn't re-emit + * the same error event. Reset on each onMediaItemTransition. + */ + private var lastEvaluatedItemIndex: Int = -1 /** * Stable queue snapshot kept in sync with the player's MediaItems — @@ -234,12 +244,53 @@ class PlayerController @Inject constructor( 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) + val current = queueRefs.getOrNull(controller.currentMediaItemIndex) + val title = current?.title?.takeIf { it.isNotEmpty() } ?: "Track" + val trackId = current?.id ?: return + playbackErrorEventsChannel.trySend( + PlaybackErrorEvent( + trackId = trackId, + kind = "load_failed", + title = title, + detail = error.message, + ), + ) + } + + override fun onMediaItemTransition( + mediaItem: androidx.media3.common.MediaItem?, + reason: Int, + ) { + // Reset the per-item evaluation guard so the new + // item's STATE_READY transition gets a fresh check. + lastEvaluatedItemIndex = -1 + } + + override fun onPlaybackStateChanged(playbackState: Int) { + if (playbackState != Player.STATE_READY) return + val idx = controller.currentMediaItemIndex + if (idx == lastEvaluatedItemIndex) return + lastEvaluatedItemIndex = idx + val current = queueRefs.getOrNull(idx) ?: return + val duration = controller.duration + if (duration > 0L && duration != androidx.media3.common.C.TIME_UNSET) return + // Zero-duration / unset-duration track: file decoded + // to nothing playable. Fail fast — fire an error + // event for the reporter (snackbar + server log) and + // advance so the user doesn't sit on the dead track. + playbackErrorEventsChannel.trySend( + PlaybackErrorEvent( + trackId = current.id, + kind = "zero_duration", + title = current.title.ifEmpty { "Track" }, + detail = "duration=$duration", + ), + ) + if (controller.hasNextMediaItem()) { + controller.seekToNextMediaItem() + } else { + controller.stop() + } } override fun onEvents(player: Player, events: Player.Events) { @@ -368,3 +419,19 @@ class PlayerController @Inject constructor( // surfaces are driven by Media3's MediaSession callbacks separately; // they don't need this poll. private const val POSITION_POLL_INTERVAL_MS = 500L + +/** + * Structured playback failure event for [PlaybackErrorReporter]. Drives + * both the user-facing snackbar ("Couldn't play X — skipping") and the + * /api/playback-errors admin inbox. + * + * `kind` is one of "zero_duration" / "load_failed" / "stalled" matching + * the server's CHECK constraint. `detail` is free-text (the Media3 + * error message, the observed duration value, etc.). + */ +data class PlaybackErrorEvent( + val trackId: String, + val kind: String, + val title: String, + val detail: String? = null, +)