diff --git a/Assets/Mirror/Runtime/MessagePacker.cs b/Assets/Mirror/Runtime/MessagePacker.cs index 2f03814cc..7b2d2d28d 100644 --- a/Assets/Mirror/Runtime/MessagePacker.cs +++ b/Assets/Mirror/Runtime/MessagePacker.cs @@ -108,7 +108,17 @@ internal static NetworkMessageDelegate WrapHandler(Action handler, b NetworkDiagnostics.OnReceive(message, channelId, reader.Length); } - handler((C)conn, message); + // user handler exception should not stop the whole server + try + { + // user implemented handler + handler((C)conn, message); + } + catch (Exception e) + { + logger.LogError($"Exception in MessageHandler: {e.GetType().Name} {e.Message} {e.StackTrace}"); + conn.Disconnect(); + } }; } } diff --git a/Assets/Mirror/Tests/Editor/CommandTest.cs b/Assets/Mirror/Tests/Editor/CommandTest.cs index 87131d683..5bfbb717e 100644 --- a/Assets/Mirror/Tests/Editor/CommandTest.cs +++ b/Assets/Mirror/Tests/Editor/CommandTest.cs @@ -1,4 +1,5 @@ using System; +using System.Text.RegularExpressions; using NUnit.Framework; using UnityEngine; using UnityEngine.TestTools; @@ -49,6 +50,17 @@ public void CmdSendInt(int someInt, NetworkConnectionToClient conn = null) } } + class ThrowBehaviour : NetworkBehaviour + { + public const string ErrorMessage = "Bad things happened"; + + [Command] + public void SendThrow(int someInt) + { + throw new Exception(ErrorMessage); + } + } + public class CommandTest : RemoteTestBase { [Test] @@ -166,5 +178,22 @@ public void SenderConnectionIsSetWhenCommandIsRecievedWithIgnoreAuthority() ProcessMessages(); Assert.That(callCount, Is.EqualTo(1)); } + + [Test] + public void CommandThatThrowsShouldBeCaught() + { + ThrowBehaviour hostBehaviour = CreateHostObject(true); + + const int someInt = 20; + NetworkConnectionToClient connectionToClient = NetworkServer.connections[0]; + Debug.Assert(connectionToClient != null, $"connectionToClient was null, This means that the test is broken and will give the wrong results"); + + LogAssert.Expect(LogType.Error, new Regex($".*{ThrowBehaviour.ErrorMessage}.*")); + Assert.DoesNotThrow(() => + { + hostBehaviour.SendThrow(someInt); + ProcessMessages(); + }, "Processing new message should not throw, the execption from SendThrow should be caught"); + } } }