Skip to content

Commit c319570

Browse files
EmandMEmandM
committed
fix: Harden error handling
1 parent ed893a1 commit c319570

18 files changed

Lines changed: 607 additions & 174 deletions

‎com.unity.netcode.gameobjects/Runtime/Components/Helpers/ComponentController.cs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -396,17 +396,21 @@ protected override void OnNetworkPostSpawn()
396396
}
397397

398398
/// <inheritdoc/>
399-
/// <remarks>
400-
/// If overriding this method, it is required that you invoke this base method.
401-
/// </remarks>
399+
// TODO: Not used anymore
402400
public override void OnDestroy()
401+
{
402+
base.OnDestroy();
403+
}
404+
405+
406+
internal override void InternalOnDestroy()
403407
{
404408
if (m_CoroutineObject.IsRunning)
405409
{
406410
StopCoroutine(m_CoroutineObject.Coroutine);
407411
m_CoroutineObject.IsRunning = false;
408412
}
409-
base.OnDestroy();
413+
base.InternalOnDestroy();
410414
}
411415

412416
/// <summary>

‎com.unity.netcode.gameobjects/Runtime/Components/Helpers/UnifiedBootstrap.cs‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,16 @@ public override bool Initialize(string defaultWorldName)
7575
LastCreatedWorld = CreateLocalWorld("LocalWorld");
7676
}
7777

78-
OnInitialized?.Invoke();
78+
// Always wrap events that can invoke user script in a try-catch to assure any
79+
// proceeding script is still executed.
80+
try
81+
{
82+
OnInitialized?.Invoke();
83+
}
84+
catch (Exception ex)
85+
{
86+
Debug.LogException(ex);
87+
}
7988

8089
return true;
8190
}

‎com.unity.netcode.gameobjects/Runtime/Components/Helpers/UnifiedUpdateConnections.cs‎

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,14 @@ protected override void OnUpdate()
5454

5555
foreach (var con in m_TempConnections)
5656
{
57-
NetworkManager.OnNetCodeDisconnect?.Invoke(con);
57+
try
58+
{
59+
NetworkManager.OnNetCodeDisconnect?.Invoke(con);
60+
}
61+
catch (System.Exception ex)
62+
{
63+
Debug.LogException(ex);
64+
}
5865
}
5966

6067
m_TempConnections.Clear();
@@ -83,7 +90,15 @@ protected override void OnUpdate()
8390
// Set the connection in-game
8491
commandBuffer.AddComponent<NetworkStreamInGame>(entry.Value.Entity);
8592
commandBuffer.AddComponent(entry.Value.Entity, default(ConnectionState));
86-
NetworkManager.OnNetCodeConnect?.Invoke(entry.Value);
93+
94+
try
95+
{
96+
NetworkManager.OnNetCodeConnect?.Invoke(entry.Value);
97+
}
98+
catch (System.Exception ex)
99+
{
100+
Debug.LogException(ex);
101+
}
87102
m_TempConnections.Add(entry.Value);
88103
}
89104
}
@@ -104,8 +119,16 @@ protected override void OnUpdate()
104119
foreach (var (networkId, entity) in SystemAPI.Query<NetworkId>().WithEntityAccess())
105120
{
106121
commandBuffer.RemoveComponent<ConnectionState>(entity);
107-
NetworkManager.OnNetCodeDisconnect?.Invoke(new NetcodeConnection
108-
{ World = World, Entity = entity, NetworkId = networkId.Value });
122+
123+
try
124+
{
125+
NetworkManager.OnNetCodeDisconnect?.Invoke(new NetcodeConnection
126+
{ World = World, Entity = entity, NetworkId = networkId.Value });
127+
}
128+
catch (System.Exception ex)
129+
{
130+
Debug.LogException(ex);
131+
}
109132
}
110133
}
111134
}
@@ -121,7 +144,15 @@ protected override void OnDestroy()
121144
foreach (var (networkId, entity) in SystemAPI.Query<NetworkId>().WithEntityAccess())
122145
{
123146
commandBuffer.RemoveComponent<ConnectionState>(entity);
124-
NetworkManager.OnNetCodeDisconnect?.Invoke(new NetcodeConnection { World = World, Entity = entity, NetworkId = networkId.Value });
147+
148+
try
149+
{
150+
NetworkManager.OnNetCodeDisconnect?.Invoke(new NetcodeConnection { World = World, Entity = entity, NetworkId = networkId.Value });
151+
}
152+
catch (System.Exception ex)
153+
{
154+
Debug.LogException(ex);
155+
}
125156
}
126157
commandBuffer.Playback(EntityManager);
127158
base.OnDestroy();

‎com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs‎

Lines changed: 42 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -711,7 +711,15 @@ internal void TransportFailureEventHandler(bool duringStart = false)
711711
var clientSeverOrHost = LocalClient.IsServer ? LocalClient.IsHost ? "Host" : "Server" : "Client";
712712
var whenFailed = duringStart ? "start failure" : "failure";
713713
NetworkLog.LogError($"{clientSeverOrHost} is shutting down due to network transport {whenFailed} of {NetworkManager.NetworkConfig.NetworkTransport.GetType().Name}!");
714-
OnTransportFailure?.Invoke();
714+
715+
try
716+
{
717+
OnTransportFailure?.Invoke();
718+
}
719+
catch (Exception ex)
720+
{
721+
Debug.LogException(ex);
722+
}
715723

716724
// If we had a transport failure when trying to start, reset the local client roles and directly invoke the internal shutdown.
717725
if (duringStart)
@@ -854,12 +862,25 @@ internal void ApproveConnection(ref ConnectionRequestMessage connectionRequestMe
854862
// Note: ToArray() also allocates. :(
855863
var response = new NetworkManager.ConnectionApprovalResponse();
856864
ClientsToApprove[context.SenderId] = response;
857-
ConnectionApprovalCallback?.Invoke(
858-
new NetworkManager.ConnectionApprovalRequest
859-
{
860-
Payload = connectionRequestMessage.ConnectionData,
861-
ClientNetworkId = context.SenderId
862-
}, response);
865+
try
866+
{
867+
ConnectionApprovalCallback?.Invoke(
868+
new NetworkManager.ConnectionApprovalRequest
869+
{
870+
Payload = connectionRequestMessage.ConnectionData,
871+
ClientNetworkId = context.SenderId
872+
}, response);
873+
}
874+
catch (Exception ex)
875+
{
876+
// A throwing approval handler would otherwise leave a Pending response stranded in
877+
// ClientsToApprove, hanging the connecting client until it times out. Deny instead.
878+
Debug.LogException(ex);
879+
response.Approved = false;
880+
response.Pending = false;
881+
response.CreatePlayerObject = false;
882+
response.Reason = "Connection approval handler threw an exception.";
883+
}
863884
}
864885

865886
/// <summary>
@@ -1748,13 +1769,22 @@ internal void Shutdown()
17481769
{
17491770
//The Transport is set during initialization, thus it is possible for the Transport to be null
17501771
var transport = NetworkManager.NetworkConfig?.NetworkTransport;
1751-
if (transport != null)
1772+
if (transport == null)
1773+
{
1774+
return;
1775+
}
1776+
// if the transport throws we need to ensure we finish the shutdown sequence.
1777+
try
17521778
{
17531779
transport.Shutdown();
1754-
if (NetworkManager.LogLevel <= LogLevel.Developer)
1755-
{
1756-
NetworkLog.LogInfo($"{nameof(NetworkConnectionManager)}.{nameof(Shutdown)}() -> {nameof(IsListening)} && {nameof(NetworkManager.NetworkConfig.NetworkTransport)} != null -> {nameof(NetworkTransport)}.{nameof(NetworkTransport.Shutdown)}()");
1757-
}
1780+
}
1781+
catch (Exception ex)
1782+
{
1783+
Debug.LogException(ex);
1784+
}
1785+
if (NetworkManager.LogLevel <= LogLevel.Developer)
1786+
{
1787+
NetworkLog.LogInfo($"{nameof(NetworkConnectionManager)}.{nameof(Shutdown)}() -> {nameof(IsListening)} && {nameof(NetworkManager.NetworkConfig.NetworkTransport)} != null -> {nameof(NetworkTransport)}.{nameof(NetworkTransport.Shutdown)}()");
17581788
}
17591789
}
17601790
}

‎com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs‎

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,14 @@ internal void __endSendServerRpc(ref FastBufferWriter bufferWriter, uint rpcMeth
137137
MessageSize = 0
138138
};
139139
serverRpcMessage.ReadBuffer = tempBuffer;
140-
serverRpcMessage.Handle(ref context);
140+
try
141+
{
142+
serverRpcMessage.Handle(ref context);
143+
}
144+
catch (Exception e)
145+
{
146+
Debug.LogException(e);
147+
}
141148
rpcWriteSize = tempBuffer.Length;
142149
}
143150
else
@@ -267,7 +274,14 @@ internal void __endSendClientRpc(ref FastBufferWriter bufferWriter, uint rpcMeth
267274
MessageSize = 0
268275
};
269276
clientRpcMessage.ReadBuffer = tempBuffer;
270-
clientRpcMessage.Handle(ref context);
277+
try
278+
{
279+
clientRpcMessage.Handle(ref context);
280+
}
281+
catch (Exception e)
282+
{
283+
Debug.LogException(e);
284+
}
271285
}
272286

273287
bufferWriter.Dispose();
@@ -640,8 +654,16 @@ protected internal virtual void OnIsDestroying()
640654
/// </remarks>
641655
internal void SetIsDestroying()
642656
{
643-
// We intentionally invoke this before setting the IsDestroying flag.
644-
OnIsDestroying();
657+
try
658+
{
659+
// We intentionally invoke this before setting the IsDestroying flag.
660+
OnIsDestroying();
661+
}
662+
catch (Exception e)
663+
{
664+
Debug.LogException(e);
665+
}
666+
// Set outside of the try-catch: a throwing override must not leave this flag false.
645667
IsDestroying = true;
646668
}
647669

@@ -931,7 +953,14 @@ internal void InternalOnGainedOwnership()
931953
{
932954
UpdateNetworkVariableOnOwnershipChanged();
933955
}
934-
OnGainedOwnership();
956+
try
957+
{
958+
OnGainedOwnership();
959+
}
960+
catch (Exception e)
961+
{
962+
Debug.LogException(e);
963+
}
935964
}
936965

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

949978
internal void InternalOnOwnershipChanged(ulong previous, ulong current)
950979
{
951-
OnOwnershipChanged(previous, current);
980+
try
981+
{
982+
OnOwnershipChanged(previous, current);
983+
}
984+
catch (Exception e)
985+
{
986+
Debug.LogException(e);
987+
}
952988
}
953989

954990
/// <summary>

0 commit comments

Comments
 (0)