feat(android): detect zero-duration tracks, fail fast, report to server
android / Build + lint + test (push) Failing after 2m7s
android / Build + lint + test (push) Failing after 2m7s
Operator hit a track that loaded with zero duration; player just sat on it. Two things needed: skip the dead track immediately, and tell the server so the admin inbox can surface the bad file. PlayerController: - Player.Listener.onPlaybackStateChanged(STATE_READY) now checks duration. If it's <= 0 or C.TIME_UNSET, fires a PlaybackErrorEvent with kind="zero_duration" and calls seekToNextMediaItem (or stop if it was the last item). Per-item evaluation guard keeps repeat STATE_READY events (post-seek, post-resume) from re-firing. - onPlayerError now also surfaces a PlaybackErrorEvent with kind="load_failed" + the Media3 exception message as detail. - playbackErrorEvents flow changes from Flow<String> (title only) to Flow<PlaybackErrorEvent> (track_id + kind + title + detail) so downstream consumers can both surface a snackbar AND POST to the admin inbox without duplicating event emission. PlaybackErrorRepository (new): - Wraps POST /api/playback-errors with the offline-first MutationQueue fallback per the standing rule for server writes. - Reuses AuthStore.clientId for the client_id field — same UUID-per- install identifier the play-events reporter sends, so support can correlate playback errors with surrounding plays. PlaybackErrorReporter: - Consumes the new richer event shape. Fires the server report per event (no debounce — the admin inbox should capture every report, not a coalesced summary). Continues to debounce the user-facing snackbar in the 2s window so a burst doesn't spam toasts. MutationQueue / MutationReplayer: - Adds PLAYBACK_ERROR_REPORT kind + PlaybackErrorReportPayload + enqueuePlaybackErrorReport entry point + replayer dispatch case hitting the new PlaybackErrorsApi. Web admin inbox + UI is the next commit.
This commit is contained in:
@@ -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,
|
||||
)
|
||||
+25
@@ -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,
|
||||
)
|
||||
|
||||
+24
@@ -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
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+21
-13
@@ -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<String>(Channel.BUFFERED)
|
||||
@@ -46,8 +51,11 @@ class PlaybackErrorReporter @Inject constructor(
|
||||
scope.launch {
|
||||
val buffer = mutableListOf<String>()
|
||||
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)
|
||||
|
||||
@@ -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 }
|
||||
}
|
||||
@@ -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<String>(Channel.CONFLATED)
|
||||
val playbackErrorEvents: Flow<String> = playbackErrorEventsChannel.receiveAsFlow()
|
||||
private val playbackErrorEventsChannel = Channel<PlaybackErrorEvent>(Channel.CONFLATED)
|
||||
val playbackErrorEvents: Flow<PlaybackErrorEvent> = 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,
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user