Assign/Remove client authority now throws exception

BREAKING CHANGE: Assign/Remove client authority throws exception instead of returning boolean

Assets/Mirror/Runtime/NetworkIdentity.cs

@@ -1052,25 +1052,21 @@ public void RebuildObservers(bool initialize)
         /// <para>Authority can be removed with RemoveClientAuthority. Only one client can own an object at any time. This does not need to be called for player objects, as their authority is setup automatically.</para>
         /// </summary>
         /// <param name="conn">	The connection of the client to assign authority to.</param>
-        /// <returns>True if authority was assigned.</returns>
-        public bool AssignClientAuthority(NetworkConnection conn)
+        public void AssignClientAuthority(NetworkConnection conn)
         {
             if (!isServer)
             {
-                Debug.LogError("AssignClientAuthority can only be called on the server for spawned objects.");
-                return false;
+                throw new InvalidOperationException("AssignClientAuthority can only be called on the server for spawned objects.");
             }
 
             if (conn == null)
             {
-                Debug.LogError("AssignClientAuthority for " + gameObject + " owner cannot be null. Use RemoveClientAuthority() instead.");
-                return false;
+                throw new InvalidOperationException("AssignClientAuthority for " + gameObject + " owner cannot be null. Use RemoveClientAuthority() instead.");
             }
 
             if (connectionToClient != null && conn != connectionToClient)
             {
-                Debug.LogError("AssignClientAuthority for " + gameObject + " already has an owner. Use RemoveClientAuthority() first.");
-                return false;
+                throw new InvalidOperationException("AssignClientAuthority for " + gameObject + " already has an owner. Use RemoveClientAuthority() first.");
             }
 
             SetClientOwner(conn);
@@ -1080,8 +1076,6 @@ public bool AssignClientAuthority(NetworkConnection conn)
             server.SendSpawnMessage(this, conn);
 
             clientAuthorityCallback?.Invoke(conn, this, true);
-
-            return true;
         }
 
         // Deprecated 09/25/2019
@@ -1094,14 +1088,12 @@ public void RemoveClientAuthority()
         {
             if (!isServer)
             {
-                Debug.LogError("RemoveClientAuthority can only be called on the server for spawned objects.");
-                return;
+                throw new InvalidOperationException("RemoveClientAuthority can only be called on the server for spawned objects.");
             }
 
             if (connectionToClient?.identity == this)
             {
-                Debug.LogError("RemoveClientAuthority cannot remove authority for a player object");
-                return;
+                throw new InvalidOperationException("RemoveClientAuthority cannot remove authority for a player object");
             }
 
             if (connectionToClient != null)

Assets/Mirror/Tests/Editor/NetworkIdentityTests.cs

@@ -626,14 +626,16 @@ public void AssignAndRemoveClientAuthority()
             // create a networkidentity with our test component
             GameObject gameObject = new GameObject();
             NetworkIdentity identity = gameObject.AddComponent<NetworkIdentity>();
+            identity.server = server;
             identity.netId = 42; // needed for isServer to be true
 
             // test the callback too
             int callbackCalled = 0;
             NetworkConnection callbackConnection = null;
             NetworkIdentity callbackIdentity = null;
             bool callbackState = false;
-            NetworkIdentity.clientAuthorityCallback += (conn, networkIdentity, state) => {
+            NetworkIdentity.clientAuthorityCallback += (conn, networkIdentity, state) =>
+            {
                 ++callbackCalled;
                 callbackConnection = conn;
                 callbackIdentity = identity;
@@ -646,17 +648,18 @@ public void AssignAndRemoveClientAuthority()
             // add client handlers
             owner.connectionToServer = new ULocalConnectionToServer();
             int spawnCalled = 0;
-            owner.connectionToServer.SetHandlers(new Dictionary<int, NetworkMessageDelegate>{
+            owner.connectionToServer.SetHandlers(new Dictionary<int, NetworkMessageDelegate>
+            {
                 { MessagePacker.GetId<SpawnMessage>(), (conn, msg, channel) => ++spawnCalled }
             });
 
             // assigning authority should only work on server.
             // if isServer is false because server isn't running yet then it
             // should fail.
-            LogAssert.ignoreFailingMessages = true; // error log is expected
-            bool result = identity.AssignClientAuthority(owner);
-            LogAssert.ignoreFailingMessages = false;
-            Assert.That(result, Is.False);
+            Assert.Throws<InvalidOperationException>(() =>
+           {
+               identity.AssignClientAuthority(owner);
+           });
 
             // we can only handle authority on the server.
             // start the server so that isServer is true.
@@ -665,8 +668,7 @@ public void AssignAndRemoveClientAuthority()
             Assert.That(identity.isServer, Is.True);
 
             // assign authority
-            result = identity.AssignClientAuthority(owner);
-            Assert.That(result, Is.True);
+            identity.AssignClientAuthority(owner);
             Assert.That(identity.connectionToClient, Is.EqualTo(owner));
             Assert.That(callbackCalled, Is.EqualTo(1));
             Assert.That(callbackConnection, Is.EqualTo(owner));
@@ -680,35 +682,42 @@ public void AssignAndRemoveClientAuthority()
 
             // shouldn't be able to assign authority while already owned by
             // another connection
-            LogAssert.ignoreFailingMessages = true; // error log is expected
-            result = identity.AssignClientAuthority(new NetworkConnectionToClient(43));
-            LogAssert.ignoreFailingMessages = false;
-            Assert.That(result, Is.False);
+            Assert.Throws<InvalidOperationException>(() =>
+           {
+               identity.AssignClientAuthority(new NetworkConnectionToClient(43));
+           });
+
             Assert.That(identity.connectionToClient, Is.EqualTo(owner));
             Assert.That(callbackCalled, Is.EqualTo(1));
 
             // someone might try to remove authority by assigning null.
             // make sure this fails.
-            LogAssert.ignoreFailingMessages = true; // error log is expected
-            result = identity.AssignClientAuthority(null);
-            LogAssert.ignoreFailingMessages = false;
-            Assert.That(result, Is.False);
+            Assert.Throws<InvalidOperationException>(() =>
+            {
+                identity.AssignClientAuthority(null);
+            });
 
             // removing authority while not isServer shouldn't work.
             // only allow it on server.
             server.Shutdown();
-            LogAssert.ignoreFailingMessages = true; // error log is expected
-            identity.RemoveClientAuthority();
-            LogAssert.ignoreFailingMessages = false;
+
+            Assert.Throws<InvalidOperationException>(() =>
+            {
+                // shoud fail because the server is not active
+                identity.RemoveClientAuthority();
+            });
             Assert.That(identity.connectionToClient, Is.EqualTo(owner));
             Assert.That(callbackCalled, Is.EqualTo(1));
             server.Listen(1); // restart it gain
 
             // removing authority for the main player object shouldn't work
             owner.identity = identity; // set connection's player object
-            LogAssert.ignoreFailingMessages = true; // error log is expected
-            identity.RemoveClientAuthority();
-            LogAssert.ignoreFailingMessages = false;
+            Assert.Throws<InvalidOperationException>(() =>
+           {
+               identity.RemoveClientAuthority();
+
+           });
+
             Assert.That(identity.connectionToClient, Is.EqualTo(owner));
             Assert.That(callbackCalled, Is.EqualTo(1));