fix: Pass the transport type to the error (#2788)

miniwolf · ShadauxCat · web-flow · commit 94a2ffcef3f0 · 2023-12-11T21:52:06.000Z
In cases where the specific transport can be used to guide the user to better support, we pass this information to the error log.

If the version is Unity Transport Protocol (UTP) we also pass the specific transport protocol to determine if relay is being used

Co-authored-by: Kitty Draper &lt;284434+ShadauxCat@users.noreply.github.com&gt;
diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkManagerHooks.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkManagerHooks.cs
@@ -1,4 +1,5 @@
 using System;
+using Unity.Netcode.Transports.UTP;
 
 namespace Unity.Netcode
 {
@@ -56,7 +57,8 @@ public bool OnVerifyCanReceive(ulong senderId, Type messageType, FastBufferReade
                 {
                     if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
                     {
-                        NetworkLog.LogError($"A {nameof(ConnectionApprovedMessage)} was received from a client on the server side. This should not happen. Please report this to the Netcode for GameObjects team at https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/issues and include the following data: Message Size: {messageContent.Length}. Message Content: {NetworkMessageManager.ByteArrayToString(messageContent.ToArray(), 0, messageContent.Length)}");
+                        var transportErrorMsg = GetTransportErrorMessage(messageContent, m_NetworkManager);
+                        NetworkLog.LogError($"A {nameof(ConnectionApprovedMessage)} was received from a client on the server side. {transportErrorMsg}");
                     }
 
                     return false;
@@ -66,7 +68,7 @@ public bool OnVerifyCanReceive(ulong senderId, Type messageType, FastBufferReade
                 {
                     if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
                     {
-                        NetworkLog.LogWarning($"Message received from {nameof(senderId)}={senderId} before it has been accepted");
+                        NetworkLog.LogWarning($"Message received from {nameof(senderId)}={senderId} before it has been accepted.");
                     }
 
                     return false;
@@ -76,7 +78,8 @@ public bool OnVerifyCanReceive(ulong senderId, Type messageType, FastBufferReade
                 {
                     if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
                     {
-                        NetworkLog.LogError($"A {nameof(ConnectionRequestMessage)} was received from a client when the connection has already been established. This should not happen. Please report this to the Netcode for GameObjects team at https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/issues and include the following data: Message Size: {messageContent.Length}. Message Content: {NetworkMessageManager.ByteArrayToString(messageContent.ToArray(), 0, messageContent.Length)}");
+                        var transportErrorMsg = GetTransportErrorMessage(messageContent, m_NetworkManager);
+                        NetworkLog.LogError($"A {nameof(ConnectionRequestMessage)} was received from a client when the connection has already been established. {transportErrorMsg}");
                     }
 
                     return false;
@@ -88,7 +91,8 @@ public bool OnVerifyCanReceive(ulong senderId, Type messageType, FastBufferReade
                 {
                     if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
                     {
-                        NetworkLog.LogError($"A {nameof(ConnectionRequestMessage)} was received from the server on the client side. This should not happen. Please report this to the Netcode for GameObjects team at https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/issues and include the following data: Message Size: {messageContent.Length}. Message Content: {NetworkMessageManager.ByteArrayToString(messageContent.ToArray(), 0, messageContent.Length)}");
+                        var transportErrorMsg = GetTransportErrorMessage(messageContent, m_NetworkManager);
+                        NetworkLog.LogError($"A {nameof(ConnectionRequestMessage)} was received from the server on the client side. {transportErrorMsg}");
                     }
 
                     return false;
@@ -98,7 +102,8 @@ public bool OnVerifyCanReceive(ulong senderId, Type messageType, FastBufferReade
                 {
                     if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
                     {
-                        NetworkLog.LogError($"A {nameof(ConnectionApprovedMessage)} was received from the server when the connection has already been established. This should not happen. Please report this to the Netcode for GameObjects team at https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/issues and include the following data: Message Size: {messageContent.Length}. Message Content: {NetworkMessageManager.ByteArrayToString(messageContent.ToArray(), 0, messageContent.Length)}");
+                        var transportErrorMsg = GetTransportErrorMessage(messageContent, m_NetworkManager);
+                        NetworkLog.LogError($"A {nameof(ConnectionApprovedMessage)} was received from the server when the connection has already been established. {transportErrorMsg}");
                     }
 
                     return false;
@@ -108,6 +113,28 @@ public bool OnVerifyCanReceive(ulong senderId, Type messageType, FastBufferReade
             return !m_NetworkManager.MessageManager.StopProcessing;
         }
 
+        private static string GetTransportErrorMessage(FastBufferReader messageContent, NetworkManager networkManager)
+        {
+            if (networkManager.NetworkConfig.NetworkTransport is not UnityTransport)
+            {
+                return $"NetworkTransport: {networkManager.NetworkConfig.NetworkTransport.GetType()}. Please report this to the maintainer of transport layer.";
+            }
+
+            var transportVersion = GetTransportVersion(networkManager);
+            return $"{transportVersion}. This should not happen. Please report this to the Netcode for GameObjects team at https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/issues and include the following data: Message Size: {messageContent.Length}. Message Content: {NetworkMessageManager.ByteArrayToString(messageContent.ToArray(), 0, messageContent.Length)}";
+        }
+
+        private static string GetTransportVersion(NetworkManager networkManager)
+        {
+            var transportVersion = "NetworkTransport: " + networkManager.NetworkConfig.NetworkTransport.GetType();
+            if (networkManager.NetworkConfig.NetworkTransport is UnityTransport unityTransport)
+            {
+                transportVersion += " UnityTransportProtocol: " + unityTransport.Protocol;
+            }
+
+            return transportVersion;
+        }
+
         public void OnBeforeHandleMessage<T>(ref T message, ref NetworkContext context) where T : INetworkMessage
         {
         }
diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/InvalidConnectionEventsTest.cs b/com.unity.netcode.gameobjects/Tests/Runtime/InvalidConnectionEventsTest.cs
@@ -96,7 +96,7 @@ public IEnumerator WhenSendingConnectionApprovedToAlreadyConnectedClient_Connect
 
             m_ClientNetworkManagers[0].ConnectionManager.MessageManager.Hook(new Hooks<ConnectionApprovedMessage>());
 
-            LogAssert.Expect(LogType.Error, new Regex($"A {nameof(ConnectionApprovedMessage)} was received from the server when the connection has already been established\\. This should not happen\\."));
+            LogAssert.Expect(LogType.Error, new Regex($"A {nameof(ConnectionApprovedMessage)} was received from the server when the connection has already been established\\. NetworkTransport: Unity.Netcode.Transports.UTP.UnityTransport UnityTransportProtocol: UnityTransport. This should not happen\\."));
 
             yield return WaitForMessageReceived<UnnamedMessage>(m_ClientNetworkManagers.ToList());
         }
@@ -118,7 +118,7 @@ public IEnumerator WhenSendingConnectionRequestToAnyClient_ConnectionRequestMess
 
             m_ClientNetworkManagers[0].ConnectionManager.MessageManager.Hook(new Hooks<ConnectionRequestMessage>());
 
-            LogAssert.Expect(LogType.Error, new Regex($"A {nameof(ConnectionRequestMessage)} was received from the server on the client side\\. This should not happen\\."));
+            LogAssert.Expect(LogType.Error, new Regex($"A {nameof(ConnectionRequestMessage)} was received from the server on the client side\\. NetworkTransport: Unity.Netcode.Transports.UTP.UnityTransport UnityTransportProtocol: UnityTransport. This should not happen\\."));
 
             yield return WaitForMessageReceived<UnnamedMessage>(m_ClientNetworkManagers.ToList());
         }
@@ -140,7 +140,7 @@ public IEnumerator WhenSendingConnectionRequestFromAlreadyConnectedClient_Connec
 
             m_ServerNetworkManager.ConnectionManager.MessageManager.Hook(new Hooks<ConnectionRequestMessage>());
 
-            LogAssert.Expect(LogType.Error, new Regex($"A {nameof(ConnectionRequestMessage)} was received from a client when the connection has already been established\\. This should not happen\\."));
+            LogAssert.Expect(LogType.Error, new Regex($"A {nameof(ConnectionRequestMessage)} was received from a client when the connection has already been established\\. NetworkTransport: Unity.Netcode.Transports.UTP.UnityTransport UnityTransportProtocol: UnityTransport. This should not happen\\."));
 
             yield return WaitForMessageReceived<UnnamedMessage>(new List<NetworkManager> { m_ServerNetworkManager });
         }
@@ -165,7 +165,7 @@ public IEnumerator WhenSendingConnectionApprovedFromAnyClient_ConnectionApproved
 
             m_ServerNetworkManager.ConnectionManager.MessageManager.Hook(new Hooks<ConnectionApprovedMessage>());
 
-            LogAssert.Expect(LogType.Error, new Regex($"A {nameof(ConnectionApprovedMessage)} was received from a client on the server side\\. This should not happen\\."));
+            LogAssert.Expect(LogType.Error, new Regex($"A {nameof(ConnectionApprovedMessage)} was received from a client on the server side\\. NetworkTransport: Unity.Netcode.Transports.UTP.UnityTransport UnityTransportProtocol: UnityTransport. This should not happen\\."));
 
             yield return WaitForMessageReceived<UnnamedMessage>(new List<NetworkManager> { m_ServerNetworkManager });
         }

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,5 @@`
`1`	`1`	`using System;`
	`2`	`+using Unity.Netcode.Transports.UTP;`
`2`	`3`
`3`	`4`	`namespace Unity.Netcode`
`4`	`5`	`{`
`@@ -56,7 +57,8 @@ public bool OnVerifyCanReceive(ulong senderId, Type messageType, FastBufferReade`
`56`	`57`	`{`
`57`	`58`	`if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)`
`58`	`59`	`{`
`59`		`- NetworkLog.LogError($"A {nameof(ConnectionApprovedMessage)} was received from a client on the server side. This should not happen. Please report this to the Netcode for GameObjects team at https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/issues and include the following data: Message Size: {messageContent.Length}. Message Content: {NetworkMessageManager.ByteArrayToString(messageContent.ToArray(), 0, messageContent.Length)}");`
	`60`	`+ var transportErrorMsg = GetTransportErrorMessage(messageContent, m_NetworkManager);`
	`61`	`+ NetworkLog.LogError($"A {nameof(ConnectionApprovedMessage)} was received from a client on the server side. {transportErrorMsg}");`
`60`	`62`	`}`
`61`	`63`
`62`	`64`	`return false;`
`@@ -66,7 +68,7 @@ public bool OnVerifyCanReceive(ulong senderId, Type messageType, FastBufferReade`
`66`	`68`	`{`
`67`	`69`	`if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)`
`68`	`70`	`{`
`69`		`- NetworkLog.LogWarning($"Message received from {nameof(senderId)}={senderId} before it has been accepted");`
	`71`	`+ NetworkLog.LogWarning($"Message received from {nameof(senderId)}={senderId} before it has been accepted.");`
`70`	`72`	`}`
`71`	`73`
`72`	`74`	`return false;`
`@@ -76,7 +78,8 @@ public bool OnVerifyCanReceive(ulong senderId, Type messageType, FastBufferReade`
`76`	`78`	`{`
`77`	`79`	`if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)`
`78`	`80`	`{`
`79`		- NetworkLog.LogError($"A {nameof(ConnectionRequestMessage)} was received from a client when the connection has already been established. This should not happen. Please report this to the Netcode for GameObjects team at https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/issues and include the following data: Message Size: {messageContent.Length}. Message Content: {NetworkMessageManager.ByteArrayToString(messageContent.ToArray(), 0, messageContent.Length)}");
	`81`	`+ var transportErrorMsg = GetTransportErrorMessage(messageContent, m_NetworkManager);`
	`82`	`+ NetworkLog.LogError($"A {nameof(ConnectionRequestMessage)} was received from a client when the connection has already been established. {transportErrorMsg}");`
`80`	`83`	`}`
`81`	`84`
`82`	`85`	`return false;`
`@@ -88,7 +91,8 @@ public bool OnVerifyCanReceive(ulong senderId, Type messageType, FastBufferReade`
`88`	`91`	`{`
`89`	`92`	`if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)`
`90`	`93`	`{`
`91`		`- NetworkLog.LogError($"A {nameof(ConnectionRequestMessage)} was received from the server on the client side. This should not happen. Please report this to the Netcode for GameObjects team at https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/issues and include the following data: Message Size: {messageContent.Length}. Message Content: {NetworkMessageManager.ByteArrayToString(messageContent.ToArray(), 0, messageContent.Length)}");`
	`94`	`+ var transportErrorMsg = GetTransportErrorMessage(messageContent, m_NetworkManager);`
	`95`	`+ NetworkLog.LogError($"A {nameof(ConnectionRequestMessage)} was received from the server on the client side. {transportErrorMsg}");`
`92`	`96`	`}`
`93`	`97`
`94`	`98`	`return false;`
`@@ -98,7 +102,8 @@ public bool OnVerifyCanReceive(ulong senderId, Type messageType, FastBufferReade`
`98`	`102`	`{`
`99`	`103`	`if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)`
`100`	`104`	`{`
`101`		- NetworkLog.LogError($"A {nameof(ConnectionApprovedMessage)} was received from the server when the connection has already been established. This should not happen. Please report this to the Netcode for GameObjects team at https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/issues and include the following data: Message Size: {messageContent.Length}. Message Content: {NetworkMessageManager.ByteArrayToString(messageContent.ToArray(), 0, messageContent.Length)}");
	`105`	`+ var transportErrorMsg = GetTransportErrorMessage(messageContent, m_NetworkManager);`
	`106`	`+ NetworkLog.LogError($"A {nameof(ConnectionApprovedMessage)} was received from the server when the connection has already been established. {transportErrorMsg}");`
`102`	`107`	`}`
`103`	`108`
`104`	`109`	`return false;`
`@@ -108,6 +113,28 @@ public bool OnVerifyCanReceive(ulong senderId, Type messageType, FastBufferReade`
`108`	`113`	`return !m_NetworkManager.MessageManager.StopProcessing;`
`109`	`114`	`}`
`110`	`115`
	`116`	`+ private static string GetTransportErrorMessage(FastBufferReader messageContent, NetworkManager networkManager)`
	`117`	`+ {`
	`118`	`+ if (networkManager.NetworkConfig.NetworkTransport is not UnityTransport)`
	`119`	`+ {`
	`120`	`+ return $"NetworkTransport: {networkManager.NetworkConfig.NetworkTransport.GetType()}. Please report this to the maintainer of transport layer.";`
	`121`	`+ }`
	`122`	`+`
	`123`	`+ var transportVersion = GetTransportVersion(networkManager);`
	`124`	`+ return $"{transportVersion}. This should not happen. Please report this to the Netcode for GameObjects team at https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/issues and include the following data: Message Size: {messageContent.Length}. Message Content: {NetworkMessageManager.ByteArrayToString(messageContent.ToArray(), 0, messageContent.Length)}";`
	`125`	`+ }`
	`126`	`+`
	`127`	`+ private static string GetTransportVersion(NetworkManager networkManager)`
	`128`	`+ {`
	`129`	`+ var transportVersion = "NetworkTransport: " + networkManager.NetworkConfig.NetworkTransport.GetType();`
	`130`	`+ if (networkManager.NetworkConfig.NetworkTransport is UnityTransport unityTransport)`
	`131`	`+ {`
	`132`	`+ transportVersion += " UnityTransportProtocol: " + unityTransport.Protocol;`
	`133`	`+ }`
	`134`	`+`
	`135`	`+ return transportVersion;`
	`136`	`+ }`
	`137`	`+`
`111`	`138`	`public void OnBeforeHandleMessage<T>(ref T message, ref NetworkContext context) where T : INetworkMessage`
`112`	`139`	`{`
`113`	`140`	`}`
Original file line number	Diff line number	Diff line change
`@@ -96,7 +96,7 @@ public IEnumerator WhenSendingConnectionApprovedToAlreadyConnectedClient_Connect`
`96`	`96`
`97`	`97`	`m_ClientNetworkManagers[0].ConnectionManager.MessageManager.Hook(new Hooks<ConnectionApprovedMessage>());`
`98`	`98`
`99`		`- LogAssert.Expect(LogType.Error, new Regex($"A {nameof(ConnectionApprovedMessage)} was received from the server when the connection has already been established\\. This should not happen\\."));`
	`99`	`+ LogAssert.Expect(LogType.Error, new Regex($"A {nameof(ConnectionApprovedMessage)} was received from the server when the connection has already been established\\. NetworkTransport: Unity.Netcode.Transports.UTP.UnityTransport UnityTransportProtocol: UnityTransport. This should not happen\\."));`
`100`	`100`
`101`	`101`	`yield return WaitForMessageReceived<UnnamedMessage>(m_ClientNetworkManagers.ToList());`
`102`	`102`	`}`
`@@ -118,7 +118,7 @@ public IEnumerator WhenSendingConnectionRequestToAnyClient_ConnectionRequestMess`
`118`	`118`
`119`	`119`	`m_ClientNetworkManagers[0].ConnectionManager.MessageManager.Hook(new Hooks<ConnectionRequestMessage>());`
`120`	`120`
`121`		`- LogAssert.Expect(LogType.Error, new Regex($"A {nameof(ConnectionRequestMessage)} was received from the server on the client side\\. This should not happen\\."));`
	`121`	`+ LogAssert.Expect(LogType.Error, new Regex($"A {nameof(ConnectionRequestMessage)} was received from the server on the client side\\. NetworkTransport: Unity.Netcode.Transports.UTP.UnityTransport UnityTransportProtocol: UnityTransport. This should not happen\\."));`
`122`	`122`
`123`	`123`	`yield return WaitForMessageReceived<UnnamedMessage>(m_ClientNetworkManagers.ToList());`
`124`	`124`	`}`
`@@ -140,7 +140,7 @@ public IEnumerator WhenSendingConnectionRequestFromAlreadyConnectedClient_Connec`
`140`	`140`
`141`	`141`	`m_ServerNetworkManager.ConnectionManager.MessageManager.Hook(new Hooks<ConnectionRequestMessage>());`
`142`	`142`
`143`		`- LogAssert.Expect(LogType.Error, new Regex($"A {nameof(ConnectionRequestMessage)} was received from a client when the connection has already been established\\. This should not happen\\."));`
	`143`	`+ LogAssert.Expect(LogType.Error, new Regex($"A {nameof(ConnectionRequestMessage)} was received from a client when the connection has already been established\\. NetworkTransport: Unity.Netcode.Transports.UTP.UnityTransport UnityTransportProtocol: UnityTransport. This should not happen\\."));`
`144`	`144`
`145`	`145`	`yield return WaitForMessageReceived<UnnamedMessage>(new List<NetworkManager> { m_ServerNetworkManager });`
`146`	`146`	`}`
`@@ -165,7 +165,7 @@ public IEnumerator WhenSendingConnectionApprovedFromAnyClient_ConnectionApproved`
`165`	`165`
`166`	`166`	`m_ServerNetworkManager.ConnectionManager.MessageManager.Hook(new Hooks<ConnectionApprovedMessage>());`
`167`	`167`
`168`		`- LogAssert.Expect(LogType.Error, new Regex($"A {nameof(ConnectionApprovedMessage)} was received from a client on the server side\\. This should not happen\\."));`
	`168`	`+ LogAssert.Expect(LogType.Error, new Regex($"A {nameof(ConnectionApprovedMessage)} was received from a client on the server side\\. NetworkTransport: Unity.Netcode.Transports.UTP.UnityTransport UnityTransportProtocol: UnityTransport. This should not happen\\."));`
`169`	`169`
`170`	`170`	`yield return WaitForMessageReceived<UnnamedMessage>(new List<NetworkManager> { m_ServerNetworkManager });`
`171`	`171`	`}`