Code review comments

Author: EmandM

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -18,6 +18,8 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Ensured all callbacks are wrapped with exception handling to avoid silent errors. (#4161)
+
 ### Security
 
 ### Obsolete

com.unity.netcode.gameobjects/Documentation~/upgrade-guide.md

@@ -26,7 +26,7 @@ To upgrade an existing project from version 2.x to version 3.x, follow these ste
 4. The API updater should catch any issues and ask if you want to allow it to make changes to your script(s).
 5. If you allow the API updater to make changes for you, then it should auto-update your project's scripts with the correct namespace changes.
 6. If you do not allow the API updater to make changes for you, then the editor will enter safe mode. Open the **Console** window to review the remaining compile errors and resolve the errors. (_[Review the Update Editor assembly definition references section below.](#update-editor-assembly-definition-references)_).
-   
+
 After the API updater finishes and you resolve the compile errors, your project compiles against version 3.x. If the API updater doesn't resolve every reference, refer to [Continue an incomplete API update](#continue-an-incomplete-api-update).
 
 _** If, at any point, you decide to downgrade to the editor version you were using prior to updating to 6.7, then make sure to restore or delete the packages-lock.json file (_assures you are not referencing 6.7 specific packages_), restore your backed up version, and delete your Library folder prior to opening your project with the editor version you were using prior to upgrading to 6.7._

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

@@ -1501,6 +1501,9 @@ internal bool InternalStartServer()
                     catch (Exception ex)
                     {
                         Log.Exception(ex);
+                        // Shutdown on exception to assure everything is cleaned up correctly
+                        ShutdownInternal();
+                        return false;
                     }
                     ConnectionManager.LocalClient.IsApproved = true;
                     return true;
@@ -1584,6 +1587,9 @@ internal bool InternalStartClient()
                     catch (Exception ex)
                     {
                         Log.Exception(ex);
+                        // Shutdown on exception to assure everything is cleaned up correctly
+                        ShutdownInternal();
+                        return false;
                     }
                 }
             }
@@ -1705,16 +1711,10 @@ private void HostServerInitialize()
             // Notify the host that everything should be synchronized/spawned at this time.
             SpawnManager.NotifyNetworkObjectsSynchronized();
 
-            try
-            {
-                OnServerStarted?.Invoke();
-                OnClientStarted?.Invoke();
-                OnStarted?.Invoke();
-            }
-            catch (Exception ex)
-            {
-                Log.Exception(ex);
-            }
+            // No need to try/catch these callbacks because this function is already wrapped.
+            OnServerStarted?.Invoke();
+            OnClientStarted?.Invoke();
+            OnStarted?.Invoke();
 
             // This assures that any in-scene placed NetworkObject is spawned and
             // any associated NetworkBehaviours' netcode related properties are

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

@@ -818,11 +818,11 @@ public enum OwnershipPermissionsFailureStatus
         public OnOwnershipPermissionsFailureDelegateHandler OnOwnershipPermissionsFailure;
 
 
-        internal void InvokeOwnershipPermissionsFailure()
+        internal void InvokeOwnershipPermissionsFailure(OwnershipPermissionsFailureStatus failureStatus)
         {
             try
             {
-                OnOwnershipPermissionsFailure?.Invoke(OwnershipPermissionsFailureStatus.SessionOwnerOnly);
+                OnOwnershipPermissionsFailure?.Invoke(failureStatus);
             }
             catch (Exception ex)
             {

com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs

@@ -522,7 +522,7 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId, bool
                     {
                         NetworkLog.LogErrorServer($"[{networkObject.name}][Session Owner Only] You cannot change ownership of a {nameof(NetworkObject)} that has the {NetworkObject.OwnershipStatus.SessionOwner} flag set!");
                     }
-                    networkObject.InvokeOwnershipPermissionsFailure();
+                    networkObject.InvokeOwnershipPermissionsFailure(NetworkObject.OwnershipPermissionsFailureStatus.SessionOwnerOnly);
                     return;
                 }
 
@@ -535,7 +535,7 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId, bool
                         {
                             NetworkLog.LogErrorServer($"[{networkObject.name}][Locked] You cannot change ownership while a {nameof(NetworkObject)} is locked!");
                         }
-                        networkObject.InvokeOwnershipPermissionsFailure();
+                        networkObject.InvokeOwnershipPermissionsFailure(NetworkObject.OwnershipPermissionsFailureStatus.Locked);
                         return;
                     }
                     if (networkObject.IsRequestInProgress)
@@ -544,7 +544,7 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId, bool
                         {
                             NetworkLog.LogErrorServer($"[{networkObject.name}][Request Pending] You cannot change ownership while a {nameof(NetworkObject)} has a pending ownership request!");
                         }
-                        networkObject.InvokeOwnershipPermissionsFailure();
+                        networkObject.InvokeOwnershipPermissionsFailure(NetworkObject.OwnershipPermissionsFailureStatus.RequestInProgress);
                         return;
                     }
                     if (networkObject.IsOwnershipRequestRequired)
@@ -553,7 +553,7 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId, bool
                         {
                             NetworkLog.LogErrorServer($"[{networkObject.name}][Request Required] You cannot change ownership directly if a {nameof(NetworkObject)} has the {NetworkObject.OwnershipStatus.RequestRequired} flag set!");
                         }
-                        networkObject.InvokeOwnershipPermissionsFailure();
+                        networkObject.InvokeOwnershipPermissionsFailure(NetworkObject.OwnershipPermissionsFailureStatus.RequestRequired);
                         return;
                     }
                     if (!networkObject.IsOwnershipTransferable)
@@ -562,7 +562,7 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId, bool
                         {
                             NetworkLog.LogErrorServer($"[{networkObject.name}][Not transferrable] You cannot change ownership of a {nameof(NetworkObject)} that does not have the {NetworkObject.OwnershipStatus.Transferable} flag set!");
                         }
-                        networkObject.InvokeOwnershipPermissionsFailure();
+                        networkObject.InvokeOwnershipPermissionsFailure(NetworkObject.OwnershipPermissionsFailureStatus.NotTransferrable);
                         return;
                     }
                 }