Skip to content

fix: Harden error handling #4161

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
EmandM wants to merge 1 commit into
base: develop-3.x.x
Choose a base branch
from
Open

fix: Harden error handling #4161

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions com.unity.netcode.gameobjects/Runtime/Components/Helpers/ComponentController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,17 +396,21 @@ protected override void OnNetworkPostSpawn()
}

/// <inheritdoc/>
/// <remarks>
/// If overriding this method, it is required that you invoke this base method.
/// </remarks>
// TODO: Not used anymore
public override void OnDestroy()
{
base.OnDestroy();
}


internal override void InternalOnDestroy()
{
if (m_CoroutineObject.IsRunning)
{
StopCoroutine(m_CoroutineObject.Coroutine);
m_CoroutineObject.IsRunning = false;
}
base.OnDestroy();
base.InternalOnDestroy();
}

/// <summary>
Expand Down
11 changes: 10 additions & 1 deletion com.unity.netcode.gameobjects/Runtime/Components/Helpers/UnifiedBootstrap.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,16 @@ public override bool Initialize(string defaultWorldName)
LastCreatedWorld = CreateLocalWorld("LocalWorld");
}

OnInitialized?.Invoke();
// Always wrap events that can invoke user script in a try-catch to assure any
// proceeding script is still executed.
try
{
OnInitialized?.Invoke();
}
catch (Exception ex)
{
Debug.LogException(ex);
}

return true;
}
Expand Down
41 changes: 36 additions & 5 deletions com.unity.netcode.gameobjects/Runtime/Components/Helpers/UnifiedUpdateConnections.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,14 @@ protected override void OnUpdate()

foreach (var con in m_TempConnections)
{
NetworkManager.OnNetCodeDisconnect?.Invoke(con);
try
{
NetworkManager.OnNetCodeDisconnect?.Invoke(con);
}
catch (System.Exception ex)
{
Debug.LogException(ex);
}
}

m_TempConnections.Clear();
Expand Down Expand Up @@ -83,7 +90,15 @@ protected override void OnUpdate()
// Set the connection in-game
commandBuffer.AddComponent<NetworkStreamInGame>(entry.Value.Entity);
commandBuffer.AddComponent(entry.Value.Entity, default(ConnectionState));
NetworkManager.OnNetCodeConnect?.Invoke(entry.Value);

try
{
NetworkManager.OnNetCodeConnect?.Invoke(entry.Value);
}
catch (System.Exception ex)
{
Debug.LogException(ex);
}
m_TempConnections.Add(entry.Value);
}
}
Expand All @@ -104,8 +119,16 @@ protected override void OnUpdate()
foreach (var (networkId, entity) in SystemAPI.Query<NetworkId>().WithEntityAccess())
{
commandBuffer.RemoveComponent<ConnectionState>(entity);
NetworkManager.OnNetCodeDisconnect?.Invoke(new NetcodeConnection
{ World = World, Entity = entity, NetworkId = networkId.Value });

try
{
NetworkManager.OnNetCodeDisconnect?.Invoke(new NetcodeConnection
{ World = World, Entity = entity, NetworkId = networkId.Value });
}
catch (System.Exception ex)
{
Debug.LogException(ex);
}
}
}
}
Expand All @@ -121,7 +144,15 @@ protected override void OnDestroy()
foreach (var (networkId, entity) in SystemAPI.Query<NetworkId>().WithEntityAccess())
{
commandBuffer.RemoveComponent<ConnectionState>(entity);
NetworkManager.OnNetCodeDisconnect?.Invoke(new NetcodeConnection { World = World, Entity = entity, NetworkId = networkId.Value });

try
{
NetworkManager.OnNetCodeDisconnect?.Invoke(new NetcodeConnection { World = World, Entity = entity, NetworkId = networkId.Value });
}
catch (System.Exception ex)
{
Debug.LogException(ex);
}
}
commandBuffer.Playback(EntityManager);
base.OnDestroy();
Expand Down
54 changes: 42 additions & 12 deletions com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -711,7 +711,15 @@ internal void TransportFailureEventHandler(bool duringStart = false)
var clientSeverOrHost = LocalClient.IsServer ? LocalClient.IsHost ? "Host" : "Server" : "Client";
var whenFailed = duringStart ? "start failure" : "failure";
NetworkLog.LogError($"{clientSeverOrHost} is shutting down due to network transport {whenFailed} of {NetworkManager.NetworkConfig.NetworkTransport.GetType().Name}!");
OnTransportFailure?.Invoke();

try
{
OnTransportFailure?.Invoke();
}
catch (Exception ex)
{
Debug.LogException(ex);
}

// If we had a transport failure when trying to start, reset the local client roles and directly invoke the internal shutdown.
if (duringStart)
Expand Down Expand Up @@ -854,12 +862,25 @@ internal void ApproveConnection(ref ConnectionRequestMessage connectionRequestMe
// Note: ToArray() also allocates. :(
var response = new NetworkManager.ConnectionApprovalResponse();
ClientsToApprove[context.SenderId] = response;
ConnectionApprovalCallback?.Invoke(
new NetworkManager.ConnectionApprovalRequest
{
Payload = connectionRequestMessage.ConnectionData,
ClientNetworkId = context.SenderId
}, response);
try
{
ConnectionApprovalCallback?.Invoke(
new NetworkManager.ConnectionApprovalRequest
{
Payload = connectionRequestMessage.ConnectionData,
ClientNetworkId = context.SenderId
}, response);
}
catch (Exception ex)
{
// A throwing approval handler would otherwise leave a Pending response stranded in
// ClientsToApprove, hanging the connecting client until it times out. Deny instead.
Debug.LogException(ex);
response.Approved = false;
response.Pending = false;
response.CreatePlayerObject = false;
response.Reason = "Connection approval handler threw an exception.";
}
}

/// <summary>
Expand Down Expand Up @@ -1748,13 +1769,22 @@ internal void Shutdown()
{
//The Transport is set during initialization, thus it is possible for the Transport to be null
var transport = NetworkManager.NetworkConfig?.NetworkTransport;
if (transport != null)
if (transport == null)
{
return;
}
// if the transport throws we need to ensure we finish the shutdown sequence.
try
{
transport.Shutdown();
if (NetworkManager.LogLevel <= LogLevel.Developer)
{
NetworkLog.LogInfo($"{nameof(NetworkConnectionManager)}.{nameof(Shutdown)}() -> {nameof(IsListening)} && {nameof(NetworkManager.NetworkConfig.NetworkTransport)} != null -> {nameof(NetworkTransport)}.{nameof(NetworkTransport.Shutdown)}()");
}
}
catch (Exception ex)
{
Debug.LogException(ex);
}
if (NetworkManager.LogLevel <= LogLevel.Developer)
{
NetworkLog.LogInfo($"{nameof(NetworkConnectionManager)}.{nameof(Shutdown)}() -> {nameof(IsListening)} && {nameof(NetworkManager.NetworkConfig.NetworkTransport)} != null -> {nameof(NetworkTransport)}.{nameof(NetworkTransport.Shutdown)}()");
}
}
}
Expand Down
48 changes: 42 additions & 6 deletions com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,14 @@ internal void __endSendServerRpc(ref FastBufferWriter bufferWriter, uint rpcMeth
MessageSize = 0
};
serverRpcMessage.ReadBuffer = tempBuffer;
serverRpcMessage.Handle(ref context);
try
{
serverRpcMessage.Handle(ref context);
}
catch (Exception e)
{
Debug.LogException(e);
}
rpcWriteSize = tempBuffer.Length;
}
else
Expand Down Expand Up @@ -267,7 +274,14 @@ internal void __endSendClientRpc(ref FastBufferWriter bufferWriter, uint rpcMeth
MessageSize = 0
};
clientRpcMessage.ReadBuffer = tempBuffer;
clientRpcMessage.Handle(ref context);
try
{
clientRpcMessage.Handle(ref context);
}
catch (Exception e)
{
Debug.LogException(e);
}
}

bufferWriter.Dispose();
Expand Down Expand Up @@ -640,8 +654,16 @@ protected internal virtual void OnIsDestroying()
/// </remarks>
internal void SetIsDestroying()
{
// We intentionally invoke this before setting the IsDestroying flag.
OnIsDestroying();
try
{
// We intentionally invoke this before setting the IsDestroying flag.
OnIsDestroying();
}
catch (Exception e)
{
Debug.LogException(e);
}
// Set outside of the try-catch: a throwing override must not leave this flag false.
IsDestroying = true;
}

Expand Down Expand Up @@ -931,7 +953,14 @@ internal void InternalOnGainedOwnership()
{
UpdateNetworkVariableOnOwnershipChanged();
}
OnGainedOwnership();
try
{
OnGainedOwnership();
}
catch (Exception e)
{
Debug.LogException(e);
}
}

/// <summary>
Expand All @@ -948,7 +977,14 @@ protected virtual void OnOwnershipChanged(ulong previous, ulong current)

internal void InternalOnOwnershipChanged(ulong previous, ulong current)
{
OnOwnershipChanged(previous, current);
try
{
OnOwnershipChanged(previous, current);
}
catch (Exception e)
{
Debug.LogException(e);
}
}

/// <summary>
Expand Down
Loading
Loading