Merge pull request #17576 from peppy/fix-multiplayer-unobserved

Centralise exception handling of `MultiplayerClient` calls

osu.Game/Online/Multiplayer/MultiplayerClientExtensions.cs

@@ -0,0 +1,44 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+#nullable enable
+
+using System;
+using System.Diagnostics;
+using System.Threading.Tasks;
+using Microsoft.AspNetCore.SignalR;
+using osu.Framework.Logging;
+
+namespace osu.Game.Online.Multiplayer
+{
+    public static class MultiplayerClientExtensions
+    {
+        public static void FireAndForget(this Task task, Action? onSuccess = null, Action<Exception>? onError = null) =>
+            task.ContinueWith(t =>
+            {
+                if (t.IsFaulted)
+                {
+                    Exception? exception = t.Exception;
+
+                    if (exception is AggregateException ae)
+                        exception = ae.InnerException;
+
+                    Debug.Assert(exception != null);
+
+                    string message = exception is HubException
+                        // HubExceptions arrive with additional message context added, but we want to display the human readable message:
+                        // "An unexpected error occurred invoking 'AddPlaylistItem' on the server.InvalidStateException: Can't enqueue more than 3 items at once."
+                        // We generally use the message field for a user-parseable error (eventually to be replaced), so drop the first part for now.
+                        ? exception.Message.Substring(exception.Message.IndexOf(':') + 1).Trim()
+                        : exception.Message;
+
+                    Logger.Log(message, level: LogLevel.Important);
+                    onError?.Invoke(exception);
+                }
+                else
+                {
+                    onSuccess?.Invoke();
+                }
+            });
+    }
+}

osu.Game/Screens/OnlinePlay/Multiplayer/Match/MatchStartControl.cs

@@ -114,18 +114,17 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match
 
             bool isReady() => Client.LocalUser?.State == MultiplayerUserState.Ready || Client.LocalUser?.State == MultiplayerUserState.Spectating;
 
-            void toggleReady() => Client.ToggleReady().ContinueWith(_ => endOperation());
+            void toggleReady() => Client.ToggleReady().FireAndForget(
+                onSuccess: endOperation,
+                onError: _ => endOperation());
 
-            void startMatch() => Client.StartMatch().ContinueWith(t =>
+            void startMatch() => Client.StartMatch().FireAndForget(onSuccess: () =>
             {
-                // accessing Exception here silences any potential errors from the antecedent task
+                // gameplay is starting, the button will be unblocked on load requested.
-                if (t.Exception != null)
+            }, onError: _ =>
             {
                 // gameplay was not started due to an exception; unblock button.
                 endOperation();
-                }
-
-                // gameplay is starting, the button will be unblocked on load requested.
             });
         }
 

osu.Game/Screens/OnlinePlay/Multiplayer/Match/Playlist/MultiplayerQueueList.cs

@@ -62,7 +62,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match.Playlist
             {
                 base.LoadComplete();
 
-                RequestDeletion = item => multiplayerClient.RemovePlaylistItem(item.ID);
+                RequestDeletion = item => multiplayerClient.RemovePlaylistItem(item.ID).FireAndForget();
 
                 multiplayerClient.RoomUpdated += onRoomUpdated;
                 onRoomUpdated();

osu.Game/Screens/OnlinePlay/Multiplayer/Multiplayer.cs

@@ -48,7 +48,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer
             // If gameplay wasn't finished, then we have a simple path back to the idle state by aborting gameplay.
             if (!playerLoader.GameplayPassed)
             {
-                client.AbortGameplay();
+                client.AbortGameplay().FireAndForget();
                 return;
             }
 

osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSongSelect.cs

@@ -1,13 +1,9 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
-using System;
-using System.Diagnostics;
 using System.Linq;
 using System.Threading.Tasks;
-using Microsoft.AspNetCore.SignalR;
 using osu.Framework.Allocation;
-using osu.Framework.Logging;
 using osu.Framework.Screens;
 using osu.Game.Beatmaps;
 using osu.Game.Graphics.UserInterface;
@@ -76,40 +72,18 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer
 
                 Task task = itemToEdit != null ? client.EditPlaylistItem(multiplayerItem) : client.AddPlaylistItem(multiplayerItem);
 
-                task.ContinueWith(t =>
+                task.FireAndForget(onSuccess: () => Schedule(() =>
                 {
-                    Schedule(() =>
-                    {
-                        // If an error or server side trigger occurred this screen may have already exited by external means.
-                        if (!this.IsCurrentScreen())
-                            return;
-
                     loadingLayer.Hide();
 
-                        if (t.IsFaulted)
+                    // If an error or server side trigger occurred this screen may have already exited by external means.
-                        {
+                    if (this.IsCurrentScreen())
-                            Exception exception = t.Exception;
-
-                            if (exception is AggregateException ae)
-                                exception = ae.InnerException;
-
-                            Debug.Assert(exception != null);
-
-                            string message = exception is HubException
-                                // HubExceptions arrive with additional message context added, but we want to display the human readable message:
-                                // "An unexpected error occurred invoking 'AddPlaylistItem' on the server.InvalidStateException: Can't enqueue more than 3 items at once."
-                                // We generally use the message field for a user-parseable error (eventually to be replaced), so drop the first part for now.
-                                ? exception.Message.Substring(exception.Message.IndexOf(':') + 1).Trim()
-                                : exception.Message;
-
-                            Logger.Log(message, level: LogLevel.Important);
-                            Carousel.AllowSelection = true;
-                            return;
-                        }
-
                         this.Exit();
-                    });
+                }), onError: _ => Schedule(() =>
-                });
+                {
+                    loadingLayer.Hide();
+                    Carousel.AllowSelection = true;
+                }));
             }
             else
             {

osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs

@@ -281,7 +281,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer
             if (client.Room == null)
                 return;
 
-            client.ChangeUserMods(mods.NewValue);
+            client.ChangeUserMods(mods.NewValue).FireAndForget();
 
             modSettingChangeTracker = new ModSettingChangeTracker(mods.NewValue);
             modSettingChangeTracker.SettingChanged += onModSettingsChanged;
@@ -296,7 +296,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer
                 if (client.Room == null)
                     return;
 
-                client.ChangeUserMods(UserMods.Value);
+                client.ChangeUserMods(UserMods.Value).FireAndForget();
             }, 500);
         }
 
@@ -305,7 +305,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer
             if (client.Room == null)
                 return;
 
-            client.ChangeBeatmapAvailability(availability.NewValue);
+            client.ChangeBeatmapAvailability(availability.NewValue).FireAndForget();
 
             if (availability.NewValue.State != DownloadState.LocallyAvailable)
             {

osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantPanel.cs

@@ -169,7 +169,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Participants
                             Origin = Anchor.Centre,
                             Alpha = 0,
                             Margin = new MarginPadding(4),
-                            Action = () => Client.KickUser(User.UserID),
+                            Action = () => Client.KickUser(User.UserID).FireAndForget(),
                         },
                     },
                 }
@@ -231,7 +231,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Participants
                         if (!Client.IsHost)
                             return;
 
-                        Client.TransferHost(targetUser);
+                        Client.TransferHost(targetUser).FireAndForget();
                     }),
                     new OsuMenuItem("Kick", MenuItemType.Destructive, () =>
                     {
@@ -239,7 +239,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Participants
                         if (!Client.IsHost)
                             return;
 
-                        Client.KickUser(targetUser);
+                        Client.KickUser(targetUser).FireAndForget();
                     })
                 };
             }

osu.Game/Screens/OnlinePlay/Multiplayer/Participants/TeamDisplay.cs

@@ -83,7 +83,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Participants
             Client.SendMatchRequest(new ChangeTeamRequest
             {
                 TeamID = ((Client.LocalUser?.MatchState as TeamVersusUserState)?.TeamID + 1) % 2 ?? 0,
-            });
+            }).FireAndForget();
         }
 
         public int? DisplayedTeam { get; private set; }