fix: Fixed ConnectionApprovalTimeout not working on the client side [MTT-3451]

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -17,6 +17,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 - Fixed issue where `NetworkTransform` was not honoring the InLocalSpace property on the authority side during OnNetworkSpawn. (#2170)
 - Implicit conversion of NetworkObjectReference to GameObject will now return null instead of throwing an exception if the referenced object could not be found (i.e., was already despawned) (#2158)
 - Fixed warning resulting from a stray NetworkAnimator.meta file (#2153)
+- Fixed Connection Approval Timeout not working client side. (#2164)
 - Fixed ClientRpcs always reporting in the profiler view as going to all clients, even when limited to a subset of clients by `ClientRpcParams`. (#2144)
 - Fixed RPC codegen failing to choose the correct extension methods for `FastBufferReader` and `FastBufferWriter` when the parameters were a generic type (i.e., List<int>) and extensions for multiple instantiations of that type have been defined (i.e., List<int> and List<string>) (#2142)
 - Fixed throwing an exception in `OnNetworkUpdate` causing other `OnNetworkUpdate` calls to not be executed. (#1739)

com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs

@@ -1623,23 +1623,56 @@ private void SendConnectionRequest()
 
         private IEnumerator ApprovalTimeout(ulong clientId)
         {
-            NetworkTime timeStarted = LocalTime;
-
-            //We yield every frame incase a pending client disconnects and someone else gets its connection id
-            while ((LocalTime - timeStarted).Time < NetworkConfig.ClientConnectionBufferTimeout && PendingClients.ContainsKey(clientId))
+            if (IsServer)
             {
-                yield return null;
-            }
+                NetworkTime timeStarted = LocalTime;
+
+                //We yield every frame incase a pending client disconnects and someone else gets its connection id
+                while (IsListening && (LocalTime - timeStarted).Time < NetworkConfig.ClientConnectionBufferTimeout && PendingClients.ContainsKey(clientId))
+                {
+                    yield return null;
+                }
+
+                if (!IsListening)
+                {
+                    yield break;
+                }
+
+                if (PendingClients.ContainsKey(clientId) && !ConnectedClients.ContainsKey(clientId))
+                {
+                    // Timeout
+                    if (NetworkLog.CurrentLogLevel <= LogLevel.Developer)
+                    {
+                        NetworkLog.LogInfo($"Client {clientId} Handshake Timed Out");
+                    }
 
-            if (PendingClients.ContainsKey(clientId) && !ConnectedClients.ContainsKey(clientId))
+                    DisconnectClient(clientId);
+                }
+            }
+            else
             {
-                // Timeout
-                if (NetworkLog.CurrentLogLevel <= LogLevel.Developer)
+                float timeStarted = Time.realtimeSinceStartup;
+
+                //We yield every frame incase a pending client disconnects and someone else gets its connection id
+                while (IsListening && (Time.realtimeSinceStartup - timeStarted) < NetworkConfig.ClientConnectionBufferTimeout && !IsConnectedClient)
+                {
+                    yield return null;
+                }
+
+                if (!IsListening)
                 {
-                    NetworkLog.LogInfo($"Client {clientId} Handshake Timed Out");
+                    yield break;
                 }
 
-                DisconnectClient(clientId);
+                if (!IsConnectedClient)
+                {
+                    // Timeout
+                    if (NetworkLog.CurrentLogLevel <= LogLevel.Developer)
+                    {
+                        NetworkLog.LogInfo("Server Handshake Timed Out");
+                    }
+                    Shutdown(true);
+                }
             }
         }
 

com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs

@@ -7,7 +7,7 @@
 using UnityEngine.SceneManagement;
 using UnityEngine.TestTools;
 using System.Runtime.CompilerServices;
-
+using Unity.Netcode.RuntimeTests;
 using Object = UnityEngine.Object;
 
 namespace Unity.Netcode.TestHelpers.Runtime
@@ -25,6 +25,8 @@ public abstract class NetcodeIntegrationTest
         protected static TimeoutHelper s_GlobalTimeoutHelper = new TimeoutHelper(8.0f);
         protected static WaitForSeconds s_DefaultWaitForTick = new WaitForSeconds(1.0f / k_DefaultTickRate);
 
+        public NetcodeLogAssert NetcodeLogAssert;
+
         /// <summary>
         /// Registered list of all NetworkObjects spawned.
         /// Format is as follows:
@@ -207,6 +209,7 @@ public IEnumerator SetUp()
         {
             VerboseDebug($"Entering {nameof(SetUp)}");
 
+            NetcodeLogAssert = new NetcodeLogAssert();
             yield return OnSetup();
             if (m_NetworkManagerInstatiationMode == NetworkManagerInstatiationMode.AllTests && m_ServerNetworkManager == null ||
                 m_NetworkManagerInstatiationMode == NetworkManagerInstatiationMode.PerTest)
@@ -596,6 +599,7 @@ public IEnumerator TearDown()
             }
 
             VerboseDebug($"Exiting {nameof(TearDown)}");
+            NetcodeLogAssert.Dispose();
         }
 
         /// <summary>

com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeLogAssert.cs

@@ -0,0 +1,152 @@
+using System;
+using System.Collections.Generic;
+using System.Text.RegularExpressions;
+using NUnit.Framework;
+using UnityEngine;
+
+namespace Unity.Netcode.RuntimeTests
+{
+    public class NetcodeLogAssert
+    {
+        private struct LogData
+        {
+            public LogType LogType;
+            public string Message;
+            public string StackTrace;
+        }
+
+        private readonly object m_Lock = new object();
+        private bool m_Disposed;
+
+        private List<LogData> AllLogs { get; }
+
+        public NetcodeLogAssert()
+        {
+            AllLogs = new List<LogData>();
+            Activate();
+        }
+
+        private void Activate()
+        {
+            Application.logMessageReceivedThreaded += AddLog;
+        }
+
+        private void Deactivate()
+        {
+            Application.logMessageReceivedThreaded -= AddLog;
+        }
+
+        public void AddLog(string message, string stacktrace, LogType type)
+        {
+            lock (m_Lock)
+            {
+                var log = new LogData
+                {
+                    LogType = type,
+                    Message = message,
+                    StackTrace = stacktrace,
+                };
+
+                AllLogs.Add(log);
+            }
+        }
+
+        public void Dispose()
+        {
+            Dispose(true);
+            GC.SuppressFinalize(this);
+        }
+
+        private void Dispose(bool disposing)
+        {
+            if (m_Disposed)
+            {
+                return;
+            }
+
+            m_Disposed = true;
+
+            if (disposing)
+            {
+                Deactivate();
+            }
+        }
+
+        public void LogWasNotReceived(LogType type, string message)
+        {
+            lock (m_Lock)
+            {
+                foreach (var logEvent in AllLogs)
+                {
+                    if (logEvent.LogType == type && message.Equals(logEvent.Message))
+                    {
+                        Assert.Fail($"Unexpected log: [{logEvent.LogType}] {logEvent.Message}");
+                    }
+                }
+            }
+        }
+
+        public void LogWasNotReceived(LogType type, Regex messageRegex)
+        {
+            lock (m_Lock)
+            {
+                foreach (var logEvent in AllLogs)
+                {
+                    if (logEvent.LogType == type && messageRegex.IsMatch(logEvent.Message))
+                    {
+                        Assert.Fail($"Unexpected log: [{logEvent.LogType}] {logEvent.Message}");
+                    }
+                }
+            }
+        }
+
+        public void LogWasReceived(LogType type, string message)
+        {
+            lock (m_Lock)
+            {
+                var found = false;
+                foreach (var logEvent in AllLogs)
+                {
+                    if (logEvent.LogType == type && message.Equals(logEvent.Message))
+                    {
+                        found = true;
+                        break;
+                    }
+                }
+
+                if (!found)
+                {
+                    Assert.Fail($"Expected log was not received: [{type}] {message}");
+                }
+            }
+        }
+
+        public void LogWasReceived(LogType type, Regex messageRegex)
+        {
+            lock (m_Lock)
+            {
+                var found = false;
+                foreach (var logEvent in AllLogs)
+                {
+                    if (logEvent.LogType == type && messageRegex.IsMatch(logEvent.Message))
+                    {
+                        found = true;
+                    }
+                }
+
+                if (!found)
+                {
+                    Assert.Fail($"Expected log was not received: [{type}] {messageRegex}");
+                }
+            }
+        }
+
+        public void Reset()
+        {
+            lock (m_Lock)
+            {
+                AllLogs.Clear();
+            }
+        }
+    }
+}

com.unity.netcode.gameobjects/Tests/Runtime/ConnectionApprovalTimeoutTests.cs

@@ -0,0 +1,104 @@
+using System.Collections;
+using System.Linq;
+using System.Text.RegularExpressions;
+using NUnit.Framework;
+using Unity.Netcode.TestHelpers.Runtime;
+using UnityEngine;
+using UnityEngine.TestTools;
+
+namespace Unity.Netcode.RuntimeTests
+{
+    [TestFixture(true)]
+    [TestFixture(false)]
+    public class ConnectionApprovalTimeoutTests : NetcodeIntegrationTest
+    {
+        protected override int NumberOfClients => 1;
+
+        protected override bool CanStartServerAndClients() => false;
+
+        private bool m_UseSceneManagement;
+        public ConnectionApprovalTimeoutTests(bool useSceneManagement)
+        {
+            m_UseSceneManagement = useSceneManagement;
+        }
+
+        // Must be >= 2 since this is an int value and the test waits for timeout - 1 to try to verify it doesn't
+        // time out early
+        private const int k_TestTimeoutPeriod = 2;
+
+        private void Start()
+        {
+            m_ServerNetworkManager.NetworkConfig.EnableSceneManagement = m_UseSceneManagement;
+            m_ClientNetworkManagers[0].NetworkConfig.EnableSceneManagement = m_UseSceneManagement;
+            if (!NetcodeIntegrationTestHelpers.Start(false, m_ServerNetworkManager, m_ClientNetworkManagers))
+            {
+                Debug.LogError("Failed to start instances");
+                Assert.Fail("Failed to start instances");
+            }
+        }
+
+        [UnityTest]
+        public IEnumerator WhenClientDoesntRequestApproval_ServerTimesOut()
+        {
+            Start();
+            var hook = new MessageCatcher<ConnectionRequestMessage>(m_ServerNetworkManager);
+            m_ServerNetworkManager.MessagingSystem.Hook(hook); ;
+
+            m_ServerNetworkManager.NetworkConfig.ClientConnectionBufferTimeout = k_TestTimeoutPeriod;
+            m_ServerNetworkManager.LogLevel = LogLevel.Developer;
+            m_ClientNetworkManagers[0].LogLevel = LogLevel.Developer;
+
+            yield return new WaitForSeconds(m_ServerNetworkManager.NetworkConfig.ClientConnectionBufferTimeout - 1);
+
+            Assert.AreEqual(0, m_ServerNetworkManager.ConnectedClients.Count);
+            Assert.AreEqual(1, m_ServerNetworkManager.PendingClients.Count);
+
+            var expectedLogMessage = new Regex($"Client {m_ServerNetworkManager.PendingClients.FirstOrDefault().Key} Handshake Timed Out");
+
+            NetcodeLogAssert.LogWasNotReceived(LogType.Log, expectedLogMessage);
+
+            yield return new WaitForSeconds(2);
+
+            NetcodeLogAssert.LogWasReceived(LogType.Log, expectedLogMessage);
+
+            Assert.AreEqual(0, m_ServerNetworkManager.ConnectedClients.Count);
+            Assert.AreEqual(0, m_ServerNetworkManager.PendingClients.Count);
+        }
+
+        [UnityTest]
+        public IEnumerator WhenServerDoesntRespondWithApproval_ClientTimesOut()
+        {
+            Start();
+
+            if (m_UseSceneManagement)
+            {
+                var sceneEventHook = new MessageCatcher<SceneEventMessage>(m_ClientNetworkManagers[0]);
+                m_ClientNetworkManagers[0].MessagingSystem.Hook(sceneEventHook);
+            }
+            else
+            {
+                var approvalHook = new MessageCatcher<ConnectionApprovedMessage>(m_ClientNetworkManagers[0]);
+                m_ClientNetworkManagers[0].MessagingSystem.Hook(approvalHook);
+            }
+
+            m_ClientNetworkManagers[0].NetworkConfig.ClientConnectionBufferTimeout = k_TestTimeoutPeriod;
+            m_ServerNetworkManager.LogLevel = LogLevel.Developer;
+            m_ClientNetworkManagers[0].LogLevel = LogLevel.Developer;
+
+            yield return new WaitForSeconds(m_ClientNetworkManagers[0].NetworkConfig.ClientConnectionBufferTimeout - 1);
+
+            Assert.IsFalse(m_ClientNetworkManagers[0].IsConnectedClient);
+            Assert.IsTrue(m_ClientNetworkManagers[0].IsListening);
+
+            var expectedLogMessage = new Regex("Server Handshake Timed Out");
+            NetcodeLogAssert.LogWasNotReceived(LogType.Log, expectedLogMessage);
+
+            yield return new WaitForSeconds(2);
+
+            NetcodeLogAssert.LogWasReceived(LogType.Log, expectedLogMessage);
+
+            Assert.IsFalse(m_ClientNetworkManagers[0].IsConnectedClient);
+            Assert.IsFalse(m_ClientNetworkManagers[0].IsListening);
+        }
+    }
+}