Do not send replay frames to spectator server if initial begin play invocation failed

osu.Game/Online/Spectator/OnlineSpectatorClient.cs

@@ -7,6 +7,7 @@
 using Microsoft.AspNetCore.SignalR.Client;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
+using osu.Framework.Logging;
 using osu.Game.Online.API;
 using osu.Game.Online.Multiplayer;
 
@@ -51,16 +52,17 @@ private void load(IAPIProvider api)
             }
         }
 
-        protected override async Task BeginPlayingInternal(long? scoreToken, SpectatorState state)
+        protected override async Task<bool> BeginPlayingInternal(long? scoreToken, SpectatorState state)
         {
             if (!IsConnected.Value)
-                return;
+                return false;
 
             Debug.Assert(connection != null);
 
             try
             {
                 await connection.InvokeAsync(nameof(ISpectatorServer.BeginPlaySession), scoreToken, state).ConfigureAwait(false);
+                return true;
             }
             catch (Exception exception)
             {
@@ -69,11 +71,14 @@ protected override async Task BeginPlayingInternal(long? scoreToken, SpectatorSt
                     Debug.Assert(connector != null);
 
                     await connector.Reconnect().ConfigureAwait(false);
-                    await BeginPlayingInternal(scoreToken, state).ConfigureAwait(false);
+                    return await BeginPlayingInternal(scoreToken, state).ConfigureAwait(false);
                 }
 
                 // Exceptions can occur if, for instance, the locally played beatmap doesn't have a server-side counterpart.
-                // For now, let's ignore these so they don't cause unobserved exceptions to appear to the user (and sentry).
+                // For now, let's ignore these so they don't cause unobserved exceptions to appear to the user (and sentry),
+                // but log to disk for diagnostic purposes.
+                Logger.Log($"{nameof(OnlineSpectatorClient)}.{nameof(BeginPlayingInternal)} failed: {exception.Message}", LoggingTarget.Network);
+                return false;
             }
         }
 

osu.Game/Online/Spectator/SpectatorClient.cs

@@ -10,6 +10,7 @@
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Development;
+using osu.Framework.Extensions;
 using osu.Framework.Graphics;
 using osu.Framework.Logging;
 using osu.Game.Beatmaps;
@@ -216,21 +217,34 @@ public void BeginPlaying(long? scoreToken, GameplayState state, Score score)
                 if (isPlaying)
                     throw new InvalidOperationException($"Cannot invoke {nameof(BeginPlaying)} when already playing");
 
-                isPlaying = true;
-
                 // transfer state at point of beginning play
                 currentState.BeatmapID = score.ScoreInfo.BeatmapInfo!.OnlineID;
                 currentState.RulesetID = score.ScoreInfo.RulesetID;
                 currentState.Mods = score.ScoreInfo.Mods.Select(m => new APIMod(m)).ToArray();
                 currentState.State = SpectatedUserState.Playing;
                 currentState.MaximumStatistics = state.ScoreProcessor.MaximumStatistics;
 
-                currentBeatmap = state.Beatmap;
-                currentScore = score;
-                currentScoreToken = scoreToken;
-                currentScoreProcessor = state.ScoreProcessor;
+                setStateForScore(scoreToken, state, score);
+
+                BeginPlayingInternal(currentScoreToken, currentState).ContinueWith(t =>
+                {
+                    bool success = t.GetResultSafely();
 
-                BeginPlayingInternal(currentScoreToken, currentState);
+                    if (!success)
+                    {
+                        Logger.Log($"Clearing {nameof(SpectatorClient)} state due to failed {nameof(BeginPlayingInternal)} call.");
+                        Schedule(() =>
+                        {
+                            clearScoreState();
+
+                            currentState.BeatmapID = null;
+                            currentState.RulesetID = null;
+                            currentState.Mods = [];
+                            currentState.State = SpectatedUserState.Idle;
+                            currentState.MaximumStatistics = [];
+                        });
+                    }
+                });
             });
         }
 
@@ -278,11 +292,7 @@ public void EndPlaying(GameplayState state)
                 if (pendingFrames.Count > 0)
                     purgePendingFrames();
 
-                isPlaying = false;
-                currentBeatmap = null;
-                currentScore = null;
-                currentScoreProcessor = null;
-                currentScoreToken = null;
+                clearScoreState();
 
                 if (state.HasPassed)
                     currentState.State = SpectatedUserState.Passed;
@@ -295,6 +305,26 @@ public void EndPlaying(GameplayState state)
             });
         }
 
+        private void setStateForScore(long? scoreToken, GameplayState state, Score score)
+        {
+            isPlaying = true;
+
+            currentBeatmap = state.Beatmap;
+            currentScore = score;
+            currentScoreToken = scoreToken;
+            currentScoreProcessor = state.ScoreProcessor;
+        }
+
+        private void clearScoreState()
+        {
+            isPlaying = false;
+
+            currentBeatmap = null;
+            currentScore = null;
+            currentScoreProcessor = null;
+            currentScoreToken = null;
+        }
+
         public virtual void WatchUser(int userId)
         {
             Debug.Assert(ThreadSafety.IsUpdateThread);
@@ -326,7 +356,11 @@ public void StopWatchingUser(int userId)
             });
         }
 
-        protected abstract Task BeginPlayingInternal(long? scoreToken, SpectatorState state);
+        /// <summary>
+        /// Contains the actual implementation of the "begin play" operation.
+        /// </summary>
+        /// <returns>Whether the server-side invocation to start play succeeded.</returns>
+        protected abstract Task<bool> BeginPlayingInternal(long? scoreToken, SpectatorState state);
 
         protected abstract Task SendFramesInternal(FrameDataBundle bundle);
 
@@ -355,6 +389,16 @@ private void purgePendingFrames()
             if (pendingFrames.Count == 0)
                 return;
 
+            if (!isPlaying)
+            {
+                // it is possible for this to happen if the `BeginPlayingInternal()` call takes a long time,
+                // the client accumulates a purgeable bundle of frames in the meantime,
+                // and then `BeginPlayingInternal()` finally fails and `clearScoreState()` is called to abort the streaming session.
+                Logger.Log($"{nameof(SpectatorClient)} dropping pending frames as the user is no longer considered to be playing.");
+                pendingFrames.Clear();
+                return;
+            }
+
             Debug.Assert(currentScore != null);
             Debug.Assert(currentScoreProcessor != null);
 

osu.Game/Tests/Visual/Spectator/TestSpectatorClient.cs

@@ -159,14 +159,15 @@ void flush()
             }
         }
 
-        protected override Task BeginPlayingInternal(long? scoreToken, SpectatorState state)
+        protected override async Task<bool> BeginPlayingInternal(long? scoreToken, SpectatorState state)
         {
             // Track the local user's playing beatmap ID.
             Debug.Assert(state.BeatmapID != null);
             userBeatmapDictionary[api.LocalUser.Value.Id] = state.BeatmapID.Value;
             userModsDictionary[api.LocalUser.Value.Id] = state.Mods.ToArray();
 
-            return ((ISpectatorClient)this).UserBeganPlaying(api.LocalUser.Value.Id, state);
+            await ((ISpectatorClient)this).UserBeganPlaying(api.LocalUser.Value.Id, state).ConfigureAwait(false);
+            return true;
         }
 
         protected override Task SendFramesInternal(FrameDataBundle bundle)