feat!: Handle relay allocation timeouts [MTT-3706] (#1994)

Author: simon-lemay-unity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -10,6 +10,8 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Added
 
+- Added a new `OnTransportFailure` callback to `NetworkManager`. This callback is invoked when the manager's `NetworkTransport` encounters an unrecoverable error. Transport failures also cause the `NetworkManager` to shut down. Currently, this is only used by `UnityTransport` to signal a timeout of its connection to the Unity Relay servers. (#1994)
+- Added `NetworkEvent.TransportFailure`, which can be used by implementations of `NetworkTransport` to signal to `NetworkManager` that an unrecoverable error was encountered. (#1994)
 - Added test to ensure a warning occurs when nesting NetworkObjects in a NetworkPrefab (#1969)
 - Added `NetworkManager.RemoveNetworkPrefab(...)` to remove a prefab from the prefabs list (#1950)
 

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

@@ -361,6 +361,16 @@ public IReadOnlyList<ulong> ConnectedClientsIds
         /// </summary>
         public event Action OnServerStarted = null;
 
+        /// <summary>
+        /// The callback to invoke if the <see cref="NetworkTransport"/> fails.
+        /// </summary>
+        /// <remarks>
+        /// A failure of the transport is always followed by the <see cref="NetworkManager"/> shutting down. Recovering
+        /// from a transport failure would normally entail reconfiguring the transport (e.g. re-authenticating, or
+        /// recreating a new service allocation depending on the transport) and restarting the client/server/host.
+        /// </remarks>
+        public event Action OnTransportFailure = null;
+
         /// <summary>
         /// Connection Approval Response
         /// </summary>
@@ -980,6 +990,7 @@ public bool StartServer()
             else
             {
                 Debug.LogError($"Server is shutting down due to network transport start failure of {NetworkConfig.NetworkTransport.GetType().Name}!");
+                OnTransportFailure?.Invoke();
                 Shutdown();
             }
 
@@ -1007,6 +1018,7 @@ public bool StartClient()
             if (!NetworkConfig.NetworkTransport.StartClient())
             {
                 Debug.LogError($"Client is shutting down due to network transport start failure of {NetworkConfig.NetworkTransport.GetType().Name}!");
+                OnTransportFailure?.Invoke();
                 Shutdown();
                 return false;
             }
@@ -1039,6 +1051,7 @@ public bool StartHost()
             if (!NetworkConfig.NetworkTransport.StartServer())
             {
                 Debug.LogError($"Server is shutting down due to network transport start failure of {NetworkConfig.NetworkTransport.GetType().Name}!");
+                OnTransportFailure?.Invoke();
                 Shutdown();
                 return false;
             }
@@ -1675,6 +1688,12 @@ private void HandleRawTransportPoll(NetworkEvent networkEvent, ulong clientId, A
                     s_TransportDisconnect.End();
 #endif
                     break;
+
+                case NetworkEvent.TransportFailure:
+                    Debug.LogError($"Shutting down due to network transport failure of {NetworkConfig.NetworkTransport.GetType().Name}!");
+                    OnTransportFailure?.Invoke();
+                    Shutdown(true);
+                    break;
             }
         }
 

com.unity.netcode.gameobjects/Runtime/Transports/NetworkEvent.cs

@@ -20,6 +20,11 @@ public enum NetworkEvent
         /// </summary>
         Disconnect,
 
+        /// <summary>
+        /// Transport has encountered an unrecoverable failure
+        /// </summary>
+        TransportFailure,
+
         /// <summary>
         /// No new event
         /// </summary>

com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs

@@ -722,6 +722,15 @@ private void Update()
 
                 m_Driver.ScheduleUpdate().Complete();
 
+                if (m_ProtocolType == ProtocolType.RelayUnityTransport && m_Driver.GetRelayConnectionStatus() == RelayConnectionStatus.AllocationInvalid)
+                {
+                    Debug.LogError("Transport failure! Relay allocation needs to be recreated, and NetworkManager restarted. " +
+                        "Use NetworkManager.OnTransportFailure to be notified of such events programmatically.");
+
+                    InvokeOnTransportEvent(NetcodeNetworkEvent.TransportFailure, 0, default, Time.realtimeSinceStartup);
+                    return;
+                }
+
                 while (AcceptConnection() && m_Driver.IsCreated)
                 {
                     ;

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

@@ -3,131 +3,158 @@
 using UnityEngine;
 using UnityEngine.TestTools;
 using NUnit.Framework;
-using Unity.Netcode.TestHelpers.Runtime;
-using Object = UnityEngine.Object;
 
 namespace Unity.Netcode.RuntimeTests
 {
-    [TestFixture(HostOrServer.Host)]
-    [TestFixture(HostOrServer.Server)]
-    public class NetworkManagerTransportTests : NetcodeIntegrationTest
+    public class NetworkManagerTransportTests
     {
-        protected override int NumberOfClients => 1;
+        [Test]
+        public void ClientDoesNotStartWhenTransportFails()
+        {
+            bool callbackInvoked = false;
+            Action onTransportFailure = () => { callbackInvoked = true; };
+
+            var manager = new GameObject().AddComponent<NetworkManager>();
+            manager.OnTransportFailure += onTransportFailure;
+
+            var transport = manager.gameObject.AddComponent<FailedTransport>();
+            transport.FailOnStart = true;
 
-        private bool m_CanStartServerAndClients = false;
+            manager.NetworkConfig = new NetworkConfig() { NetworkTransport = transport };
 
-        public NetworkManagerTransportTests(HostOrServer hostOrServer) : base(hostOrServer) { }
+            LogAssert.Expect(LogType.Error, $"Client is shutting down due to network transport start failure of {transport.GetType().Name}!");
 
-        protected override IEnumerator OnSetup()
+            Assert.False(manager.StartClient());
+            Assert.False(manager.IsListening);
+            Assert.False(manager.IsConnectedClient);
+
+            Assert.True(callbackInvoked);
+        }
+
+        [Test]
+        public void HostDoesNotStartWhenTransportFails()
         {
-            m_CanStartServerAndClients = false;
-            return base.OnSetup();
+            bool callbackInvoked = false;
+            Action onTransportFailure = () => { callbackInvoked = true; };
+
+            var manager = new GameObject().AddComponent<NetworkManager>();
+            manager.OnTransportFailure += onTransportFailure;
+
+            var transport = manager.gameObject.AddComponent<FailedTransport>();
+            transport.FailOnStart = true;
+
+            manager.NetworkConfig = new NetworkConfig() { NetworkTransport = transport };
+
+            LogAssert.Expect(LogType.Error, $"Server is shutting down due to network transport start failure of {transport.GetType().Name}!");
+
+            Assert.False(manager.StartHost());
+            Assert.False(manager.IsListening);
+
+            Assert.True(callbackInvoked);
         }
 
-        protected override bool CanStartServerAndClients()
+        [Test]
+        public void ServerDoesNotStartWhenTransportFails()
         {
-            return m_CanStartServerAndClients;
+            bool callbackInvoked = false;
+            Action onTransportFailure = () => { callbackInvoked = true; };
+
+            var manager = new GameObject().AddComponent<NetworkManager>();
+            manager.OnTransportFailure += onTransportFailure;
+
+            var transport = manager.gameObject.AddComponent<FailedTransport>();
+            transport.FailOnStart = true;
+
+            manager.NetworkConfig = new NetworkConfig() { NetworkTransport = transport };
+
+            LogAssert.Expect(LogType.Error, $"Server is shutting down due to network transport start failure of {transport.GetType().Name}!");
+
+            Assert.False(manager.StartServer());
+            Assert.False(manager.IsListening);
+
+            Assert.True(callbackInvoked);
+        }
+
+        [UnityTest]
+        public IEnumerator ShutsDownWhenTransportFails()
+        {
+            bool callbackInvoked = false;
+            Action onTransportFailure = () => { callbackInvoked = true; };
+
+            var manager = new GameObject().AddComponent<NetworkManager>();
+            manager.OnTransportFailure += onTransportFailure;
+
+            var transport = manager.gameObject.AddComponent<FailedTransport>();
+            transport.FailOnNextPoll = true;
+
+            manager.NetworkConfig = new NetworkConfig() { NetworkTransport = transport };
+
+            Assert.True(manager.StartServer());
+            Assert.True(manager.IsListening);
+
+            LogAssert.Expect(LogType.Error, $"Shutting down due to network transport failure of {transport.GetType().Name}!");
+
+            // Need two updates to actually shut down. First one to see the transport failing, which
+            // marks the NetworkManager as shutting down. Second one where actual shutdown occurs.
+            yield return null;
+            yield return null;
+
+            Assert.False(manager.IsListening);
+            Assert.True(callbackInvoked);
         }
 
         /// <summary>
-        /// Validate that if the NetworkTransport fails to start the NetworkManager
-        /// will not continue the startup process and will shut itself down.
+        /// Does nothing but simulate a transport that can fail at startup and/or when polling events.
         /// </summary>
-        /// <param name="testClient">if true it will test the client side</param>
-        [UnityTest]
-        public IEnumerator DoesNotStartWhenTransportFails([Values] bool testClient)
+        public class FailedTransport : TestingNetworkTransport
         {
-            // The error message we should expect
-            var messageToCheck = "";
-            if (!testClient)
-            {
-                Object.DestroyImmediate(m_ServerNetworkManager.NetworkConfig.NetworkTransport);
-                m_ServerNetworkManager.NetworkConfig.NetworkTransport = m_ServerNetworkManager.gameObject.AddComponent<FailedTransport>();
-                m_ServerNetworkManager.NetworkConfig.NetworkTransport.Initialize(m_ServerNetworkManager);
-                // The error message we should expect
-                messageToCheck = $"Server is shutting down due to network transport start failure of {m_ServerNetworkManager.NetworkConfig.NetworkTransport.GetType().Name}!";
-            }
-            else
+            public bool FailOnStart = false;
+            public bool FailOnNextPoll = false;
+
+            public override bool StartClient() => !FailOnStart;
+
+            public override bool StartServer() => !FailOnStart;
+
+            public override NetworkEvent PollEvent(out ulong clientId, out ArraySegment<byte> payload, out float receiveTime)
             {
-                foreach (var client in m_ClientNetworkManagers)
+                clientId = 0;
+                payload = new ArraySegment<byte>();
+                receiveTime = 0;
+
+                if (FailOnNextPoll)
                 {
-                    Object.DestroyImmediate(client.NetworkConfig.NetworkTransport);
-                    client.NetworkConfig.NetworkTransport = client.gameObject.AddComponent<FailedTransport>();
-                    client.NetworkConfig.NetworkTransport.Initialize(m_ServerNetworkManager);
+                    FailOnNextPoll = false;
+                    return NetworkEvent.TransportFailure;
                 }
-                // The error message we should expect
-                messageToCheck = $"Client is shutting down due to network transport start failure of {m_ClientNetworkManagers[0].NetworkConfig.NetworkTransport.GetType().Name}!";
-            }
-
-            // Trap for the nested NetworkManager exception
-            LogAssert.Expect(LogType.Error, messageToCheck);
-            m_CanStartServerAndClients = true;
-            // Due to other errors, we must not send clients if testing the server-host side
-            // We can test both server and client(s) when testing client-side only
-            if (testClient)
-            {
-                NetcodeIntegrationTestHelpers.Start(m_UseHost, m_ServerNetworkManager, m_ClientNetworkManagers);
-                yield return s_DefaultWaitForTick;
-                foreach (var client in m_ClientNetworkManagers)
+                else
                 {
-                    Assert.False(client.IsListening);
-                    Assert.False(client.IsConnectedClient);
+                    return NetworkEvent.Nothing;
                 }
             }
-            else
+
+            public override ulong ServerClientId => 0;
+
+            public override void Send(ulong clientId, ArraySegment<byte> payload, NetworkDelivery networkDelivery)
             {
-                NetcodeIntegrationTestHelpers.Start(m_UseHost, m_ServerNetworkManager, new NetworkManager[] { });
-                yield return s_DefaultWaitForTick;
-                Assert.False(m_ServerNetworkManager.IsListening);
             }
-        }
-    }
 
-    /// <summary>
-    /// Does nothing but simulate a transport that failed to start
-    /// </summary>
-    public class FailedTransport : TestingNetworkTransport
-    {
-        public override void Shutdown()
-        {
-        }
+            public override void Initialize(NetworkManager networkManager = null)
+            {
+            }
 
-        public override ulong ServerClientId => 0;
+            public override void Shutdown()
+            {
+            }
 
-        public override NetworkEvent PollEvent(out ulong clientId, out ArraySegment<byte> payload, out float receiveTime)
-        {
-            clientId = 0;
-            payload = new ArraySegment<byte>();
-            receiveTime = 0;
-            return NetworkEvent.Nothing;
-        }
-        public override bool StartClient()
-        {
-            // Simulate failure, always return false
-            return false;
-        }
-        public override bool StartServer()
-        {
-            // Simulate failure, always return false
-            return false;
-        }
-        public override void Send(ulong clientId, ArraySegment<byte> payload, NetworkDelivery networkDelivery)
-        {
-        }
+            public override ulong GetCurrentRtt(ulong clientId) => 0;
 
-        public override void DisconnectRemoteClient(ulong clientId)
-        {
-        }
+            public override void DisconnectRemoteClient(ulong clientId)
+            {
+            }
 
-        public override void Initialize(NetworkManager networkManager = null)
-        {
-        }
-        public override ulong GetCurrentRtt(ulong clientId)
-        {
-            return 0;
-        }
-        public override void DisconnectLocalClient()
-        {
+            public override void DisconnectLocalClient()
+            {
+            }
         }
     }
 }