mirror of
https://github.com/ppy/osu.git
synced 2026-05-13 19:54:15 +08:00
Fix several issues with multiplayer & playlists room join error logging
This is in response to https://osu.ppy.sh/community/forums/topics/2058708?n=5, wherein the user is having a problem with joining multiplayer, but I have basically no diagnosing capabilities, because the logs are all 2025-03-26 18:57:57 [error]: Failed to join multiplayer room: 2025-03-26 18:58:40 [error]: Failed to join multiplayer room: 2025-03-26 18:58:41 [error]: Failed to join multiplayer room: 2025-03-26 18:58:41 [error]: Failed to join multiplayer room: which appears to originate from https://github.com/ppy/osu/blob/c82eaafe98d96b9f49a4a7f168ef5c484e67d76f/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerLoungeSubScreen.cs#L91 Now, as far as I can tell, there are two possibilities here: 1. The exception's `Message` is null or empty. That's not exactly great or typical, but sure, this could possibly happen - in which case the error logging is silently eating whatever little relevant detail there is left to use. 2. The exception is *actually* `null` itself, and we're in the X Files. This PR is intending to defend against (1). In examining the logging further, I also spotted the following issues: - In the single path that specifies a custom failure handler (which is `DrawableLoungeRoom` which handles joining a passworded room), the custom failure handler being present means that the error would be presented to the user, but it would not be logged. At all. - In playlists, if the exception for whatever reason had an empty message, an empty notification would get posted. And in general to me it feels a bit dodgy to be directly presenting exception notifications to users without any preamble, hence the added "Failed to open playlist" prefix.
This commit is contained in:
@@ -40,7 +40,7 @@ namespace osu.Game.Tests.Visual.Multiplayer
|
||||
{
|
||||
var mockLounge = new Mock<IOnlinePlayLounge>();
|
||||
mockLounge
|
||||
.Setup(l => l.Join(It.IsAny<Room>(), It.IsAny<string>(), It.IsAny<Action<Room>>(), It.IsAny<Action<string>>()))
|
||||
.Setup(l => l.Join(It.IsAny<Room>(), It.IsAny<string>(), It.IsAny<Action<Room>>(), It.IsAny<Action<string, Exception?>>()))
|
||||
.Callback<Room, string, Action<Room>, Action<string>>((_, _, _, d) =>
|
||||
{
|
||||
Task.Run(() =>
|
||||
|
||||
@@ -17,6 +17,7 @@ using osu.Framework.Graphics.UserInterface;
|
||||
using osu.Framework.Input.Bindings;
|
||||
using osu.Framework.Input.Events;
|
||||
using osu.Framework.Localisation;
|
||||
using osu.Framework.Logging;
|
||||
using osu.Game.Extensions;
|
||||
using osu.Game.Graphics;
|
||||
using osu.Game.Graphics.Sprites;
|
||||
@@ -306,13 +307,14 @@ namespace osu.Game.Screens.OnlinePlay.Lounge
|
||||
GetContainingFocusManager()?.TriggerFocusContention(passwordTextBox);
|
||||
}
|
||||
|
||||
private void joinFailed(string error) => Schedule(() =>
|
||||
private void joinFailed(string message, Exception? exception) => Schedule(() =>
|
||||
{
|
||||
passwordTextBox.Text = string.Empty;
|
||||
|
||||
GetContainingFocusManager()!.ChangeFocus(passwordTextBox);
|
||||
|
||||
errorText.Text = error;
|
||||
Logger.Log($"Failed to join room with password. {exception}");
|
||||
errorText.Text = message;
|
||||
errorText
|
||||
.FadeIn()
|
||||
.FlashColour(Color4.White, 200)
|
||||
|
||||
@@ -14,8 +14,8 @@ namespace osu.Game.Screens.OnlinePlay.Lounge
|
||||
/// <param name="room">The room to join.</param>
|
||||
/// <param name="password">The password.</param>
|
||||
/// <param name="onSuccess">A delegate to invoke if the user joined the room.</param>
|
||||
/// <param name="onFailure">A delegate to invoke if the user is not able join the room.</param>
|
||||
void Join(Room room, string? password, Action<Room>? onSuccess = null, Action<string>? onFailure = null);
|
||||
/// <param name="onFailure">A delegate to invoke if the user is not able to join the room.</param>
|
||||
void Join(Room room, string? password, Action<Room>? onSuccess = null, Action<string, Exception?>? onFailure = null);
|
||||
|
||||
/// <summary>
|
||||
/// Copies the given room and opens it as a fresh (not-yet-created) one.
|
||||
|
||||
@@ -334,7 +334,7 @@ namespace osu.Game.Screens.OnlinePlay.Lounge
|
||||
popoverContainer.HidePopover();
|
||||
}
|
||||
|
||||
public void Join(Room room, string? password, Action<Room>? onSuccess = null, Action<string>? onFailure = null) => Schedule(() =>
|
||||
public void Join(Room room, string? password, Action<Room>? onSuccess = null, Action<string, Exception?>? onFailure = null) => Schedule(() =>
|
||||
{
|
||||
if (joiningRoomOperation != null)
|
||||
return;
|
||||
@@ -347,19 +347,19 @@ namespace osu.Game.Screens.OnlinePlay.Lounge
|
||||
joiningRoomOperation?.Dispose();
|
||||
joiningRoomOperation = null;
|
||||
onSuccess?.Invoke(room);
|
||||
}, error =>
|
||||
}, (message, exception) =>
|
||||
{
|
||||
joiningRoomOperation?.Dispose();
|
||||
joiningRoomOperation = null;
|
||||
|
||||
if (onFailure != null)
|
||||
onFailure(error);
|
||||
onFailure(message, exception);
|
||||
else
|
||||
Logger.Log(error, level: LogLevel.Error);
|
||||
Logger.Error(exception, message);
|
||||
});
|
||||
});
|
||||
|
||||
protected abstract void JoinInternal(Room room, string? password, Action<Room> onSuccess, Action<string> onFailure);
|
||||
protected abstract void JoinInternal(Room room, string? password, Action<Room> onSuccess, Action<string, Exception?> onFailure);
|
||||
|
||||
public void OpenCopy(Room room)
|
||||
{
|
||||
|
||||
@@ -75,7 +75,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer
|
||||
|
||||
protected override OnlinePlaySubScreen CreateRoomSubScreen(Room room) => new MultiplayerMatchSubScreen(room);
|
||||
|
||||
protected override void JoinInternal(Room room, string? password, Action<Room> onSuccess, Action<string> onFailure)
|
||||
protected override void JoinInternal(Room room, string? password, Action<Room> onSuccess, Action<string, Exception?> onFailure)
|
||||
{
|
||||
client.JoinRoom(room, password).ContinueWith(result =>
|
||||
{
|
||||
@@ -86,9 +86,9 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer
|
||||
Exception? exception = result.Exception?.AsSingular();
|
||||
|
||||
if (exception?.GetHubExceptionMessage() is string message)
|
||||
onFailure(message);
|
||||
onFailure(message, exception);
|
||||
else
|
||||
onFailure($"Failed to join multiplayer room: {exception?.Message}");
|
||||
onFailure($"Failed to join multiplayer room. {exception?.Message}", exception);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
@@ -60,7 +60,7 @@ namespace osu.Game.Screens.OnlinePlay.Playlists
|
||||
return criteria;
|
||||
}
|
||||
|
||||
protected override void JoinInternal(Room room, string? password, Action<Room> onSuccess, Action<string> onFailure)
|
||||
protected override void JoinInternal(Room room, string? password, Action<Room> onSuccess, Action<string, Exception?> onFailure)
|
||||
{
|
||||
var joinRoomRequest = new JoinRoomRequest(room, password);
|
||||
|
||||
@@ -68,7 +68,7 @@ namespace osu.Game.Screens.OnlinePlay.Playlists
|
||||
joinRoomRequest.Failure += exception =>
|
||||
{
|
||||
if (exception is not OperationCanceledException)
|
||||
onFailure(exception.Message);
|
||||
onFailure($"Failed to open playlist. {exception.Message}", exception);
|
||||
};
|
||||
|
||||
api.Queue(joinRoomRequest);
|
||||
|
||||
Reference in New Issue
Block a user