feat(player): stall watchdog + bounded retry + skip-to-cached (#66)

Reported: on poor coverage a track ends and the next (uncached) track
never starts — streams, hangs, no retry. Root cause: a buffering stall
emits NO error event so the onError path never fires and there was no
stall watchdog; even on a real error _handlePlaybackError immediately
skipped the literal-next (likely also-unreachable) source with no retry.

- _reconcileStallWatchdog: while playing+buffering, a 15s window; if
  buffered position hasn't advanced it's a dead stream → recover; if
  progressing, re-arm (slow-but-downloading is fine). Driven from
  _broadcastState like the idle/position reconcilers.
- _recoverPlayback unifies stall + onError: retry the SAME track once
  (skipToQueueItem rebuilds a fresh source/HTTP — a transient blip no
  longer loses it); on exhaustion, surface via the #58 SnackBar and
  skip to the next cached track, else pause (no thrashing through
  unreachable streams).
- per-track retry budget resets when a track reaches ready+playing.
- _handlePlaybackError now delegates into the unified path.

Core playback change — device-verify on a throttled connection.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-05-19 14:14:25 -04:00
parent 1646b02ca2
commit 154626ae94
+116 -26
View File
@@ -76,6 +76,24 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl
/// need this. Active only while playing; cancelled otherwise.
Timer? _positionBroadcastTimer;
/// Buffering-stall watchdog. On poor coverage a stream hangs in
/// ProcessingState.buffering with NO error event, so the onError
/// recovery never fires. Armed while playing+buffering; on fire, if
/// buffered position hasn't advanced it's a dead stream → recover.
/// Re-arms a fresh window while buffering-but-progressing (slow-but-
/// downloading is not a stall).
static const _stallTimeout = Duration(seconds: 15);
Timer? _stallTimer;
Duration _stallMarkBuffered = Duration.zero;
/// Per-track recovery budget. Reset when a track reaches
/// ready+playing. One retry of the same source (rebuilt fresh), then
/// skip to the next cached track (or pause) instead of thrashing
/// through unreachable streams.
static const _maxRecoverRetries = 1;
String? _recoveringTrackId;
int _recoverAttempts = 0;
/// Player volume captured when an OS duck interruption begins,
/// restored when it ends. Null when not currently ducked.
double? _volumeBeforeDuck;
@@ -492,33 +510,12 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl
///
/// If we're at the last track, seekToNext is a no-op; the state
/// drops to idle and _broadcastState reflects that.
/// onError entrypoint (404 / decoder / premature EOS / network drop).
/// Routes into the unified recovery path (retry the track once, then
/// skip to the next cached track or pause) instead of immediately
/// skipping the literal next — possibly also-unreachable — source.
Future<void> _handlePlaybackError() async {
// mediaItem is the correctly-mapped current track (post #49 logical
// index), so it's the one that just failed — no player-index math.
final failed = mediaItem.value?.title;
if (failed != null && failed.isNotEmpty) {
_playbackErrors.add(failed);
}
final currentIdx = _player.currentIndex;
final queueLen = queue.value.length;
if (currentIdx == null || currentIdx + 1 >= queueLen) {
// Last track or no queue context — just pause; _broadcastState
// already reflects the idle/error processingState.
try {
await _player.pause();
} catch (_) {}
return;
}
try {
await _player.seekToNext();
} catch (_) {
// If seekToNext fails too (next source not built yet, etc.),
// there's nothing safe to do — pause and let the user recover
// manually.
try {
await _player.pause();
} catch (_) {}
}
await _recoverPlayback();
}
void _onCurrentIndexChanged(int? idx) {
@@ -573,6 +570,8 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl
_idleStopTimer = null;
_positionBroadcastTimer?.cancel();
_positionBroadcastTimer = null;
_stallTimer?.cancel();
_stallTimer = null;
try {
await _player.stop();
} catch (_) {}
@@ -786,6 +785,7 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl
));
_reconcileIdleTimer();
_reconcilePositionBroadcast();
_reconcileStallWatchdog();
}
/// Arms (or cancels) the idle-cleanup timer based on the current
@@ -842,6 +842,96 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl
);
}
/// Buffering-stall watchdog. Armed while playing+buffering/loading.
/// On fire: still stuck with no >=1s buffered progress → dead stream,
/// recover; progressing → re-arm a fresh window (slow-but-downloading
/// is fine). When the track is happily ready+playing, clears the
/// per-track recovery budget. Driven from _broadcastState; mirrors
/// the other reconcile helpers.
void _reconcileStallWatchdog() {
final ps = _player.processingState;
if (ps == ProcessingState.ready && _player.playing) {
_recoveringTrackId = null;
_recoverAttempts = 0;
}
final buffering = _player.playing &&
(ps == ProcessingState.buffering || ps == ProcessingState.loading);
if (!buffering) {
_stallTimer?.cancel();
_stallTimer = null;
return;
}
if (_stallTimer != null) return; // window already running
_stallMarkBuffered = _player.bufferedPosition;
_stallTimer = Timer(_stallTimeout, () {
_stallTimer = null;
final ps2 = _player.processingState;
final stillBuffering = _player.playing &&
(ps2 == ProcessingState.buffering ||
ps2 == ProcessingState.loading);
if (!stillBuffering) return;
final progressed = (_player.bufferedPosition - _stallMarkBuffered) >=
const Duration(seconds: 1);
if (progressed) {
_reconcileStallWatchdog(); // slow but downloading — re-arm
} else {
unawaited(_recoverPlayback());
}
});
}
/// Unified recovery for a failed OR stalled stream. Retries the
/// current track once (rebuilt from scratch via skipToQueueItem so a
/// transient blip doesn't lose it); on exhaustion, surfaces the
/// failure (#58 SnackBar) and skips to the next cached track — or
/// pauses — instead of blindly walking into more unreachable streams.
Future<void> _recoverPlayback() async {
_stallTimer?.cancel();
_stallTimer = null;
final media = mediaItem.value;
if (media == null) return;
final trackId = media.id;
final curIdx = _lastTracks.indexWhere((t) => t.id == trackId);
if (curIdx < 0) {
try {
await _player.pause();
} catch (_) {}
return;
}
if (_recoveringTrackId != trackId) {
_recoveringTrackId = trackId;
_recoverAttempts = 0;
}
if (_recoverAttempts < _maxRecoverRetries) {
_recoverAttempts++;
// Rebuild + restart the SAME track (fresh source / HTTP).
await skipToQueueItem(curIdx);
return;
}
if (media.title.isNotEmpty) _playbackErrors.add(media.title);
_recoveringTrackId = null;
_recoverAttempts = 0;
await _skipToNextCachedOrPause(curIdx);
}
/// Skips to the first track after [fromIdx] that is fully cached on
/// disk (plays with zero network). If none, pauses rather than
/// thrashing through unreachable streams on a dead connection.
Future<void> _skipToNextCachedOrPause(int fromIdx) async {
final mgr = _audioCacheManager;
if (mgr != null) {
for (var i = fromIdx + 1; i < _lastTracks.length; i++) {
if (await mgr.pathFor(_lastTracks[i].id) != null) {
await skipToQueueItem(i);
return;
}
}
}
try {
await _player.pause();
} catch (_) {}
}
/// Configures the OS audio session for music playback and wires
/// interruption + becoming-noisy handling. just_audio auto-activates /
/// deactivates the session around playback once it's configured, but