fix(watch-together): retry guest media switches until they commit
Guest switch dispatch pre-marked its dedup key and fired-and-forgot, so any failure (fetch error, reload busy with an auto-advance, navigation race with the host exiting) silently stranded the guest on the old media. A CurrentPlaybackDispatcher now marks a key handled only after the sink reports success against the committed identity, with a serialized in-flight slot, timeout, and generation reset; the reconciler re-offers unattached media on every host heartbeat, making the heartbeat the retry channel. Fetches that outlive their dispatch are re-validated against the current snapshot so a stale switch can't override the live one. hostExitedPlayer now rides the controller's ordered message queue with host authentication instead of racing state handling in the provider.
This commit is contained in:
@@ -10,11 +10,14 @@ import '../../utils/app_logger.dart';
|
||||
import '../models/playback_state.dart';
|
||||
import '../models/sync_message.dart';
|
||||
import '../models/watch_session.dart';
|
||||
import '../services/current_playback_dispatcher.dart';
|
||||
import '../services/watch_together_controller.dart';
|
||||
import '../services/watch_together_peer_service.dart';
|
||||
|
||||
/// Callback type for when media switches (for guest navigation)
|
||||
typedef MediaSwitchCallback = void Function(String ratingKey, ServerId serverId, String mediaTitle);
|
||||
/// Callback type for when media switches (for guest navigation). Returns
|
||||
/// whether the switch was handled; unhandled keys are re-dispatched on the
|
||||
/// host's next state heartbeat.
|
||||
typedef MediaSwitchCallback = Future<bool> Function(String ratingKey, ServerId serverId, String mediaTitle);
|
||||
|
||||
/// Provider for Watch Together functionality
|
||||
///
|
||||
@@ -34,7 +37,7 @@ class WatchTogetherProvider with ChangeNotifier {
|
||||
List<String> _waitingOnPeerIds = const [];
|
||||
PlaybackPhase? _playbackPhase;
|
||||
String _displayName = 'User';
|
||||
String? _lastHandledCurrentPlaybackKey;
|
||||
final CurrentPlaybackDispatcher _playbackDispatcher = CurrentPlaybackDispatcher();
|
||||
|
||||
// Coalesce rapid-fire notifyListeners() calls into a single rebuild per frame.
|
||||
// During Watch Together join, 4-5 notifications fire within milliseconds;
|
||||
@@ -163,7 +166,7 @@ class WatchTogetherProvider with ChangeNotifier {
|
||||
errorMessage: session.errorMessage,
|
||||
hostPeerId: session.hostPeerId,
|
||||
);
|
||||
_lastHandledCurrentPlaybackKey = null;
|
||||
_playbackDispatcher.reset();
|
||||
}
|
||||
|
||||
void _dispatchCurrentPlayback({
|
||||
@@ -178,13 +181,19 @@ class WatchTogetherProvider with ChangeNotifier {
|
||||
return;
|
||||
}
|
||||
|
||||
_lastHandledCurrentPlaybackKey = _buildPlaybackKey(ratingKey, ServerId(serverId));
|
||||
appLogger.d('WatchTogether: Dispatching current playback from $source: $mediaTitle');
|
||||
callback(ratingKey, ServerId(serverId), mediaTitle);
|
||||
// The key is only marked handled if the callback reports success; a
|
||||
// failed switch is retried on the host's next state heartbeat.
|
||||
unawaited(
|
||||
_playbackDispatcher.dispatch(
|
||||
_buildPlaybackKey(ratingKey, serverId)!,
|
||||
() => callback(ratingKey, serverId, mediaTitle),
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
void markCurrentPlaybackHandled({required String ratingKey, required ServerId serverId}) {
|
||||
_lastHandledCurrentPlaybackKey = _buildPlaybackKey(ratingKey, serverId);
|
||||
_playbackDispatcher.markHandled(_buildPlaybackKey(ratingKey, serverId)!);
|
||||
}
|
||||
|
||||
void requestCurrentPlaybackSnapshot() {
|
||||
@@ -239,6 +248,8 @@ class WatchTogetherProvider with ChangeNotifier {
|
||||
|
||||
controller.onMediaStateReceived = _handleMediaStateReceived;
|
||||
|
||||
controller.onHostExitedPlayer = _handleHostExitedPlayer;
|
||||
|
||||
controller.onRemoteAction = (peerId, hint) {
|
||||
final type = switch (hint) {
|
||||
PlaybackActionHint.play => ParticipantEventType.resumed,
|
||||
@@ -291,7 +302,7 @@ class WatchTogetherProvider with ChangeNotifier {
|
||||
}) async {
|
||||
// Clean up any existing session
|
||||
await leaveSession();
|
||||
_lastHandledCurrentPlaybackKey = null;
|
||||
_playbackDispatcher.reset();
|
||||
|
||||
appLogger.d('WatchTogether: Creating session with control mode: $controlMode');
|
||||
|
||||
@@ -335,7 +346,7 @@ class WatchTogetherProvider with ChangeNotifier {
|
||||
Future<void> joinSession(String sessionId, {String? displayName}) async {
|
||||
// Clean up any existing session
|
||||
await leaveSession();
|
||||
_lastHandledCurrentPlaybackKey = null;
|
||||
_playbackDispatcher.reset();
|
||||
|
||||
appLogger.d('WatchTogether: Joining session: $sessionId');
|
||||
|
||||
@@ -446,7 +457,7 @@ class WatchTogetherProvider with ChangeNotifier {
|
||||
_isWaitingForPeers = false;
|
||||
_waitingOnPeerIds = const [];
|
||||
_playbackPhase = null;
|
||||
_lastHandledCurrentPlaybackKey = null;
|
||||
_playbackDispatcher.reset();
|
||||
_lastActionEventMs.clear();
|
||||
_hostIntentionallyLeft = false;
|
||||
|
||||
@@ -606,7 +617,7 @@ class WatchTogetherProvider with ChangeNotifier {
|
||||
// If the host deliberately left, end the session for everyone.
|
||||
if (!isHost && message.peerId == _session?.hostPeerId) {
|
||||
_hostIntentionallyLeft = true;
|
||||
_handleHostExitedPlayer(message);
|
||||
_handleHostExitedPlayer();
|
||||
leaveSession();
|
||||
}
|
||||
|
||||
@@ -614,9 +625,8 @@ class WatchTogetherProvider with ChangeNotifier {
|
||||
}
|
||||
break;
|
||||
|
||||
case SyncMessageType.hostExitedPlayer:
|
||||
_handleHostExitedPlayer(message);
|
||||
break;
|
||||
// hostExitedPlayer is routed through the controller's ordered message
|
||||
// queue so it can't overtake (or be overtaken by) state messages.
|
||||
|
||||
default:
|
||||
// Playback sync messages (state/status/control/...) are handled by
|
||||
@@ -647,22 +657,38 @@ class WatchTogetherProvider with ChangeNotifier {
|
||||
void _handleMediaStateReceived(String ratingKey, String serverId, String? mediaTitle) {
|
||||
if (isHost) return;
|
||||
|
||||
final playbackKey = _buildPlaybackKey(ratingKey, serverIdOrNull(serverId));
|
||||
final shouldDispatch = playbackKey != _lastHandledCurrentPlaybackKey;
|
||||
final typedServerId = serverIdOrNull(serverId);
|
||||
if (typedServerId == null) {
|
||||
appLogger.w('WatchTogether: Ignoring playback state with blank serverId');
|
||||
return;
|
||||
}
|
||||
final playbackKey = _buildPlaybackKey(ratingKey, typedServerId);
|
||||
|
||||
_updateCurrentPlaybackSnapshot(ratingKey: ratingKey, serverId: ServerId(serverId), mediaTitle: mediaTitle ?? '');
|
||||
notifyListeners();
|
||||
// Detached guests receive this on every heartbeat; only rebuild when the
|
||||
// snapshot actually changes.
|
||||
final session = _session;
|
||||
final snapshotChanged =
|
||||
session == null ||
|
||||
session.mediaRatingKey != ratingKey ||
|
||||
session.mediaServerId != typedServerId ||
|
||||
session.mediaTitle != (mediaTitle ?? '');
|
||||
_updateCurrentPlaybackSnapshot(ratingKey: ratingKey, serverId: typedServerId, mediaTitle: mediaTitle ?? '');
|
||||
if (snapshotChanged) notifyListeners();
|
||||
|
||||
if (shouldDispatch) {
|
||||
if (_playbackDispatcher.shouldDispatch(playbackKey)) {
|
||||
_dispatchCurrentPlayback(
|
||||
ratingKey: ratingKey,
|
||||
serverId: ServerId(serverId),
|
||||
serverId: typedServerId,
|
||||
mediaTitle: mediaTitle ?? '',
|
||||
source: 'playback state',
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@visibleForTesting
|
||||
void debugHandleMediaState(String ratingKey, String serverId, String? mediaTitle) =>
|
||||
_handleMediaStateReceived(ratingKey, serverId, mediaTitle);
|
||||
|
||||
/// Called when user seeks locally (to sync with peers)
|
||||
void onLocalSeek(Duration position) {
|
||||
_controller?.onLocalSeek(position);
|
||||
@@ -710,7 +736,7 @@ class WatchTogetherProvider with ChangeNotifier {
|
||||
}
|
||||
|
||||
/// Handle host exited player message (guest only)
|
||||
void _handleHostExitedPlayer(SyncMessage _) {
|
||||
void _handleHostExitedPlayer() {
|
||||
if (isHost) return; // Host doesn't need to handle their own exit
|
||||
|
||||
appLogger.d('WatchTogether: Host exited player, callback set: ${onHostExitedPlayer != null}');
|
||||
|
||||
@@ -0,0 +1,50 @@
|
||||
import '../../utils/app_logger.dart';
|
||||
|
||||
/// Serializes guest media-switch dispatches and provides heartbeat-driven
|
||||
/// retry: a key is only marked handled after its callback reports success,
|
||||
/// so a failed switch is re-dispatched by the host's next state heartbeat.
|
||||
class CurrentPlaybackDispatcher {
|
||||
static const dispatchTimeout = Duration(seconds: 30);
|
||||
|
||||
String? _lastHandledKey;
|
||||
String? _inFlightKey;
|
||||
int _generation = 0;
|
||||
|
||||
String? get inFlightKey => _inFlightKey;
|
||||
|
||||
/// Whether [key] should be dispatched now. A single in-flight slot
|
||||
/// serializes dispatches (concurrent navigations would stack player
|
||||
/// routes); once it frees, the next heartbeat carries the latest key.
|
||||
bool shouldDispatch(String? key) => key != null && key != _lastHandledKey && _inFlightKey == null;
|
||||
|
||||
/// Suppress future dispatches of [key] (e.g. a user-initiated join already
|
||||
/// navigating to it).
|
||||
void markHandled(String key) => _lastHandledKey = key;
|
||||
|
||||
/// Session left / host exited player: clears state and invalidates any
|
||||
/// in-flight completion so a stale success can't suppress a later re-join
|
||||
/// of the same media.
|
||||
void reset() {
|
||||
_generation++;
|
||||
_inFlightKey = null;
|
||||
_lastHandledKey = null;
|
||||
}
|
||||
|
||||
Future<void> dispatch(String key, Future<bool> Function() invoke, {Duration timeout = dispatchTimeout}) async {
|
||||
// Synchronous — claims the slot before the first await so a
|
||||
// same-microtask second state can't double-dispatch.
|
||||
_inFlightKey = key;
|
||||
final generation = _generation;
|
||||
var handled = false;
|
||||
try {
|
||||
// then<bool> re-types the future: a throwing async callback is
|
||||
// reified as Future<Never>, whose timeout() rejects a bool onTimeout.
|
||||
handled = await invoke().then<bool>((value) => value).timeout(timeout, onTimeout: () => false);
|
||||
} catch (e, stackTrace) {
|
||||
appLogger.w('WatchTogether: media switch dispatch failed for $key', error: e, stackTrace: stackTrace);
|
||||
}
|
||||
if (generation != _generation) return; // reset() happened mid-flight
|
||||
_inFlightKey = null;
|
||||
if (handled) _lastHandledKey = key; // else: next heartbeat retries
|
||||
}
|
||||
}
|
||||
@@ -10,6 +10,8 @@ import 'clock_sync.dart';
|
||||
/// Callbacks the reconciler surfaces to the provider/UI layer.
|
||||
class GuestReconcilerCallbacks {
|
||||
/// The host's state names media we don't have loaded — navigate/reload.
|
||||
/// Fires on EVERY such state (the heartbeat is the retry channel for
|
||||
/// failed switches); the provider's dispatcher dedups.
|
||||
final void Function(String ratingKey, String serverId, String? mediaTitle)? onMediaSwitchNeeded;
|
||||
|
||||
final void Function(ControlMode mode)? onControlModeChanged;
|
||||
@@ -226,7 +228,6 @@ class GuestPlaybackReconciler {
|
||||
void onState(PlaybackState state) {
|
||||
if (state.seq <= _lastSeq) return; // Stale or reordered.
|
||||
_lastSeq = state.seq;
|
||||
final previous = _latestState;
|
||||
_latestState = state;
|
||||
|
||||
if (state.controlMode != _reportedControlMode) {
|
||||
@@ -259,13 +260,10 @@ class GuestPlaybackReconciler {
|
||||
_sendStatus(force: true);
|
||||
}
|
||||
|
||||
// The host moved to media we don't have — hand off to the switch flow.
|
||||
if (_attachedMediaKey != null && state.mediaKey != _attachedMediaKey) {
|
||||
_callbacks.onMediaSwitchNeeded?.call(state.ratingKey, state.serverId, state.mediaTitle);
|
||||
return;
|
||||
}
|
||||
if (previous?.mediaKey != state.mediaKey && _attachedMediaKey == null) {
|
||||
// Not in the player yet — let the provider navigate.
|
||||
// Not attached to the host's media (detached, or attached to something
|
||||
// else) — hand off to the switch flow on every state so a failed switch
|
||||
// retries on the next heartbeat. The provider's dispatcher dedups.
|
||||
if (_attachedMediaKey == null || state.mediaKey != _attachedMediaKey) {
|
||||
_callbacks.onMediaSwitchNeeded?.call(state.ratingKey, state.serverId, state.mediaTitle);
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -90,6 +90,7 @@ class WatchTogetherController {
|
||||
void Function(bool correcting)? onCorrectingChanged;
|
||||
void Function(ControlMode mode)? onControlModeReceived;
|
||||
void Function(String ratingKey, String serverId, String? mediaTitle)? onMediaStateReceived;
|
||||
void Function()? onHostExitedPlayer;
|
||||
void Function(String peerId, PlaybackActionHint hint)? onRemoteAction;
|
||||
void Function(String peerId)? onPeerNeedsUpdate;
|
||||
void Function(List<String> peerIds)? onResumedWithout;
|
||||
@@ -339,7 +340,12 @@ class WatchTogetherController {
|
||||
break;
|
||||
|
||||
case SyncMessageType.hostExitedPlayer:
|
||||
// Handled at the provider level.
|
||||
// Rides the ordered queue so it can't locally overtake state
|
||||
// messages that preceded it on the wire. Only the host may end the
|
||||
// media epoch.
|
||||
if (!_session.isHost && senderId == _session.hostPeerId) {
|
||||
onHostExitedPlayer?.call();
|
||||
}
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user