Skip to content

Commit

Permalink
fix: kcp2k V1.18
Browse files Browse the repository at this point in the history 
- feature: OnError to allow higher level to show popups etc.
- feature: KcpServer.GetClientAddress is now GetClientEndPoint in order to
  expose more details
- ResolveHostname: include exception in log for easier debugging
- fix: KcpClientConnection.RawReceive now logs the SocketException even if
  it was expected. makes debugging easier.
- fix: KcpServer.TickIncoming now logs the SocketException even if it was
  expected. makes debugging easier.
- fix: KcpClientConnection.RawReceive now calls Disconnect() if the other end
  has closed the connection. better than just remaining in a state with unusable
  sockets.

=> error handling based on MirrorNetworking#3155
=> fixes MirrorNetworking#3143
  • Loading branch information
vis2k committed May 8, 2022
1 parent c65eed8 commit 35f1f22
Show file tree
Hide file tree
Showing 10 changed files with 126 additions and 42 deletions.
14 changes: 11 additions & 3 deletions Assets/Mirror/Runtime/Transports/KCP/MirrorTransport/KcpTransport.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,18 +90,21 @@ void Awake()
? new KcpClientNonAlloc(
() => OnClientConnected.Invoke(),
(message, channel) => OnClientDataReceived.Invoke(message, FromKcpChannel(channel)),
() => OnClientDisconnected.Invoke())
() => OnClientDisconnected.Invoke(),
(error) => OnClientError.Invoke(new Exception(error)))
: new KcpClient(
() => OnClientConnected.Invoke(),
(message, channel) => OnClientDataReceived.Invoke(message, FromKcpChannel(channel)),
() => OnClientDisconnected.Invoke());
() => OnClientDisconnected.Invoke(),
(error) => OnClientError.Invoke(new Exception(error)));

// server
server = NonAlloc
? new KcpServerNonAlloc(
(connectionId) => OnServerConnected.Invoke(connectionId),
(connectionId, message, channel) => OnServerDataReceived.Invoke(connectionId, message, FromKcpChannel(channel)),
(connectionId) => OnServerDisconnected.Invoke(connectionId),
(connectionId, error) => OnServerError.Invoke(connectionId, new Exception(error)),
DualMode,
NoDelay,
Interval,
Expand All @@ -116,6 +119,7 @@ void Awake()
(connectionId) => OnServerConnected.Invoke(connectionId),
(connectionId, message, channel) => OnServerDataReceived.Invoke(connectionId, message, FromKcpChannel(channel)),
(connectionId) => OnServerDisconnected.Invoke(connectionId),
(connectionId, error) => OnServerError.Invoke(connectionId, new Exception(error)),
DualMode,
NoDelay,
Interval,
Expand Down Expand Up @@ -196,7 +200,11 @@ public override void ServerSend(int connectionId, ArraySegment<byte> segment, in
OnServerDataSent?.Invoke(connectionId, segment, channelId);
}
public override void ServerDisconnect(int connectionId) => server.Disconnect(connectionId);
public override string ServerGetClientAddress(int connectionId) => server.GetClientAddress(connectionId);
public override string ServerGetClientAddress(int connectionId)
{
IPEndPoint endPoint = server.GetClientEndPoint(connectionId);
return endPoint != null ? endPoint.Address.ToString() : "";
}
public override void ServerStop() => server.Stop();
public override void ServerEarlyUpdate()
{
Expand Down
0 ...Runtime/Transports/KCP/kcp2k/kcp2k.asmdef → ...r/Runtime/Transports/KCP/kcp2k/KCP.asmdef
View file
File renamed without changes.
0 ...me/Transports/KCP/kcp2k/kcp2k.asmdef.meta → ...time/Transports/KCP/kcp2k/KCP.asmdef.meta
View file
File renamed without changes.
13 changes: 13 additions & 0 deletions Assets/Mirror/Runtime/Transports/KCP/kcp2k/VERSION
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,16 @@
V1.18 [2022-05-08]
- feature: OnError to allow higher level to show popups etc.
- feature: KcpServer.GetClientAddress is now GetClientEndPoint in order to
expose more details
- ResolveHostname: include exception in log for easier debugging
- fix: KcpClientConnection.RawReceive now logs the SocketException even if
it was expected. makes debugging easier.
- fix: KcpServer.TickIncoming now logs the SocketException even if it was
expected. makes debugging easier.
- fix: KcpClientConnection.RawReceive now calls Disconnect() if the other end
has closed the connection. better than just remaining in a state with unusable
sockets.

V1.17 [2022-01-09]
- perf: server/client MaximizeSendReceiveBuffersToOSLimit option to set send/recv
buffer sizes to OS limit. avoids drops due to small buffers under heavy load.
Expand Down
17 changes: 13 additions & 4 deletions Assets/Mirror/Runtime/Transports/KCP/kcp2k/highlevel/KcpClient.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,21 @@ public class KcpClient
public Action OnConnected;
public Action<ArraySegment<byte>, KcpChannel> OnData;
public Action OnDisconnected;
// error callback instead of logging.
// allows libraries to show popups etc.
// (string instead of Exception for ease of use)
public Action<string> OnError;

// state
public KcpClientConnection connection;
public bool connected;

public KcpClient(Action OnConnected, Action<ArraySegment<byte>, KcpChannel> OnData, Action OnDisconnected)
public KcpClient(Action OnConnected, Action<ArraySegment<byte>, KcpChannel> OnData, Action OnDisconnected, Action<string> OnError)
{
this.OnConnected = OnConnected;
this.OnData = OnData;
this.OnDisconnected = OnDisconnected;
this.OnError = OnError;
}

// CreateConnection can be overwritten for where-allocation:
Expand Down Expand Up @@ -53,19 +58,23 @@ public void Connect(string address,
{
Log.Info($"KCP: OnClientConnected");
connected = true;
OnConnected.Invoke();
OnConnected();
};
connection.OnData = (message, channel) =>
{
//Log.Debug($"KCP: OnClientData({BitConverter.ToString(message.Array, message.Offset, message.Count)})");
OnData.Invoke(message, channel);
OnData(message, channel);
};
connection.OnDisconnected = () =>
{
Log.Info($"KCP: OnClientDisconnected");
connected = false;
connection = null;
OnDisconnected.Invoke();
OnDisconnected();
};
connection.OnError = (exception) =>
{
OnError(exception);
};

// connect
Expand Down
23 changes: 18 additions & 5 deletions Assets/Mirror/Runtime/Transports/KCP/kcp2k/highlevel/KcpClientConnection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ public static bool ResolveHostname(string hostname, out IPAddress[] addresses)
addresses = Dns.GetHostAddresses(hostname);
return addresses.Length >= 1;
}
catch (SocketException)
catch (SocketException exception)
{
Log.Info($"Failed to resolve host: {hostname}");
Log.Info($"Failed to resolve host: {hostname} reason: {exception}");
addresses = null;
return false;
}
Expand Down Expand Up @@ -95,7 +95,12 @@ public void Connect(string host,
RawReceive();
}
// otherwise call OnDisconnected to let the user know.
else OnDisconnected();
else
{
// pass error to user callback. no need to log it manually.
OnError($"Failed to resolve host: {host}");
OnDisconnected();
}
}

// call from transport update
Expand All @@ -119,14 +124,22 @@ public void RawReceive()
}
else
{
Log.Error($"KCP ClientConnection: message of size {msgLength} does not fit into buffer of size {rawReceiveBuffer.Length}. The excess was silently dropped. Disconnecting.");
// pass error to user callback. no need to log it manually.
OnError($"KCP ClientConnection: message of size {msgLength} does not fit into buffer of size {rawReceiveBuffer.Length}. The excess was silently dropped. Disconnecting.");
Disconnect();
}
}
}
}
// this is fine, the socket might have been closed in the other end
catch (SocketException) {}
catch (SocketException ex)
{
// the other end closing the connection is not an 'error'.
// but connections should never just end silently.
// at least log a message for easier debugging.
Log.Info($"KCP ClientConnection: looks like the other end has closed the connection. This is fine: {ex}");
Disconnect();
}
}

protected override void Dispose()
Expand Down
64 changes: 42 additions & 22 deletions Assets/Mirror/Runtime/Transports/KCP/kcp2k/highlevel/KcpConnection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ public abstract class KcpConnection
public Action OnAuthenticated;
public Action<ArraySegment<byte>, KcpChannel> OnData;
public Action OnDisconnected;
// error callback instead of logging.
// allows libraries to show popups etc.
// (string instead of Exception for ease of use)
public Action<string> OnError;

// If we don't receive anything these many milliseconds
// then consider us disconnected
Expand Down Expand Up @@ -166,7 +170,8 @@ void HandleTimeout(uint time)
// only ever happen if the connection is truly gone.
if (time >= lastReceiveTime + timeout)
{
Log.Warning($"KCP: Connection timed out after not receiving any message for {timeout}ms. Disconnecting.");
// pass error to user callback. no need to log it manually.
OnError($"KCP: Connection timed out after not receiving any message for {timeout}ms. Disconnecting.");
Disconnect();
}
}
Expand All @@ -176,7 +181,8 @@ void HandleDeadLink()
// kcp has 'dead_link' detection. might as well use it.
if (kcp.state == -1)
{
Log.Warning($"KCP Connection dead_link detected: a message was retransmitted {kcp.dead_link} times without ack. Disconnecting.");
// pass error to user callback. no need to log it manually.
OnError($"KCP Connection dead_link detected: a message was retransmitted {kcp.dead_link} times without ack. Disconnecting.");
Disconnect();
}
}
Expand All @@ -203,10 +209,11 @@ void HandleChoked()
kcp.rcv_buf.Count + kcp.snd_buf.Count;
if (total >= QueueDisconnectThreshold)
{
Log.Warning($"KCP: disconnecting connection because it can't process data fast enough.\n" +
$"Queue total {total}>{QueueDisconnectThreshold}. rcv_queue={kcp.rcv_queue.Count} snd_queue={kcp.snd_queue.Count} rcv_buf={kcp.rcv_buf.Count} snd_buf={kcp.snd_buf.Count}\n" +
$"* Try to Enable NoDelay, decrease INTERVAL, disable Congestion Window (= enable NOCWND!), increase SEND/RECV WINDOW or compress data.\n" +
$"* Or perhaps the network is simply too slow on our end, or on the other end.\n");
// pass error to user callback. no need to log it manually.
OnError($"KCP: disconnecting connection because it can't process data fast enough.\n" +
$"Queue total {total}>{QueueDisconnectThreshold}. rcv_queue={kcp.rcv_queue.Count} snd_queue={kcp.snd_queue.Count} rcv_buf={kcp.rcv_buf.Count} snd_buf={kcp.snd_buf.Count}\n" +
$"* Try to Enable NoDelay, decrease INTERVAL, disable Congestion Window (= enable NOCWND!), increase SEND/RECV WINDOW or compress data.\n" +
$"* Or perhaps the network is simply too slow on our end, or on the other end.");

// let's clear all pending sends before disconnting with 'Bye'.
// otherwise a single Flush in Disconnect() won't be enough to
Expand Down Expand Up @@ -242,15 +249,17 @@ bool ReceiveNextReliable(out KcpHeader header, out ArraySegment<byte> message)
else
{
// if receive failed, close everything
Log.Warning($"Receive failed with error={received}. closing connection.");
// pass error to user callback. no need to log it manually.
OnError($"Receive failed with error={received}. closing connection.");
Disconnect();
}
}
// we don't allow sending messages > Max, so this must be an
// attacker. let's disconnect to avoid allocation attacks etc.
else
{
Log.Warning($"KCP: possible allocation attack for msgSize {msgSize} > buffer {kcpMessageBuffer.Length}. Disconnecting the connection.");
// pass error to user callback. no need to log it manually.
OnError($"KCP: possible allocation attack for msgSize {msgSize} > buffer {kcpMessageBuffer.Length}. Disconnecting the connection.");
Disconnect();
}
}
Expand Down Expand Up @@ -292,7 +301,8 @@ void TickIncoming_Connected(uint time)
case KcpHeader.Disconnect:
{
// everything else is not allowed during handshake!
Log.Warning($"KCP: received invalid header {header} while Connected. Disconnecting the connection.");
// pass error to user callback. no need to log it manually.
OnError($"KCP: received invalid header {header} while Connected. Disconnecting the connection.");
Disconnect();
break;
}
Expand Down Expand Up @@ -332,7 +342,8 @@ void TickIncoming_Authenticated(uint time)
// empty data = attacker, or something went wrong
else
{
Log.Warning("KCP: received empty Data message while Authenticated. Disconnecting the connection.");
// pass error to user callback. no need to log it manually.
OnError("KCP: received empty Data message while Authenticated. Disconnecting the connection.");
Disconnect();
}
break;
Expand Down Expand Up @@ -381,19 +392,22 @@ public void TickIncoming()
catch (SocketException exception)
{
// this is ok, the connection was closed
Log.Info($"KCP Connection: Disconnecting because {exception}. This is fine.");
// pass error to user callback. no need to log it manually.
OnError($"KCP Connection: Disconnecting because {exception}. This is fine.");
Disconnect();
}
catch (ObjectDisposedException exception)
{
// fine, socket was closed
Log.Info($"KCP Connection: Disconnecting because {exception}. This is fine.");
// pass error to user callback. no need to log it manually.
OnError($"KCP Connection: Disconnecting because {exception}. This is fine.");
Disconnect();
}
catch (Exception ex)
catch (Exception exception)
{
// unexpected
Log.Error(ex.ToString());
// pass error to user callback. no need to log it manually.
OnError($"KCP Connection: unexpected Exception: {exception}");
Disconnect();
}
}
Expand Down Expand Up @@ -423,19 +437,22 @@ public void TickOutgoing()
catch (SocketException exception)
{
// this is ok, the connection was closed
Log.Info($"KCP Connection: Disconnecting because {exception}. This is fine.");
// pass error to user callback. no need to log it manually.
OnError($"KCP Connection: Disconnecting because {exception}. This is fine.");
Disconnect();
}
catch (ObjectDisposedException exception)
{
// fine, socket was closed
Log.Info($"KCP Connection: Disconnecting because {exception}. This is fine.");
// pass error to user callback. no need to log it manually.
OnError($"KCP Connection: Disconnecting because {exception}. This is fine.");
Disconnect();
}
catch (Exception ex)
catch (Exception exception)
{
// unexpected
Log.Error(ex.ToString());
// pass error to user callback. no need to log it manually.
OnError($"KCP Connection: unexpected exception: {exception}");
Disconnect();
}
}
Expand Down Expand Up @@ -496,16 +513,18 @@ public void RawInput(byte[] buffer, int msgLength)
}
else
{
// should never
Log.Warning($"KCP: received unreliable message in state {state}. Disconnecting the connection.");
// should never happen
// pass error to user callback. no need to log it manually.
OnError($"KCP: received unreliable message in state {state}. Disconnecting the connection.");
Disconnect();
}
break;
}
default:
{
// not a valid channel. random data or attacks.
Log.Info($"Disconnecting connection because of invalid channel header: {channel}");
// pass error to user callback. no need to log it manually.
OnError($"Disconnecting connection because of invalid channel header: {channel}");
Disconnect();
break;
}
Expand Down Expand Up @@ -580,7 +599,8 @@ public void SendData(ArraySegment<byte> data, KcpChannel channel)
// let's make it obvious so it's easy to debug.
if (data.Count == 0)
{
Log.Warning("KcpConnection: tried sending empty message. This should never happen. Disconnecting.");
// pass error to user callback. no need to log it manually.
OnError("KcpConnection: tried sending empty message. This should never happen. Disconnecting.");
Disconnect();
return;
}
Expand Down
Loading

0 comments on commit 35f1f22

Please sign in to comment.