NetworkManager.DisconnectReason is always Empty String

[Reag]

Description

The variable NetworkManager.DisconnectReason is always empty. The code that sets this variable in the NetworkManager is never activated with UTP.

Reproduce Steps

1. Build a new project with netcode
2. Place a break point on line 47 of DisconnectReasonMessage.cs, the only function that accesses NetworkManager.DisconnectReason
3. Perform various Disconnects (connection interruption, timeout, client stopped, ect)

Actual Outcome

Breakpoint that sets the DisconnectReason is never tripped, leaving the variable as empty string

Expected Outcome

The variable will get some information about why the server disconnected the client

Environment

• OS: Windows 12
• Unity Version: 2021.3.12f1
• Netcode Version: 1.2.0
• Unity Transport Version: 1.3.0

[NoelStephensUnity]

@Reag
I think the property name is possibly misleading. The NetworkManager.DisconnectReason is something you would set if you are using connection approval.

I am going to mark this as a feature request since I can see a few spots where we could populate this string with known disconnection reasons (time out, client stopped/disconnected itself, and I think we might be able to determine if the transport just loses the connection...which would be a "connection interruption").

[Reag]

Would definitely be useful. The tooltip from Intellisense lead me to believe that this property would always be populated with a disconnect reason.

Both of my reports stem from a few hours of trying to track down the reason for a client disconnect. It turns out that after I fixed up a bad RPC, the scene had no network event traffic for idle clients. As, for some reason, the heartbeat in the project was misconfigured to be longer than the timeout, it lead to the clients disconnecting. However, I had no way to tell this (I only recieved a generic disconnect message from the console), and spent a few hours with wireshark and the unity debugger as a result.

Some kind of way of telling 'why' a client disconnected would definitely be a useful feature for me as a developer using this feature.

[NoelStephensUnity]

@Reag
Yikes! We already know this area needs improvement and don't want users spending several hours figuring out things like this.
I have marked this as a feature request and as we are planning our 2023 roadmap we will definitely be looking at things just like this.
Glad to hear you got past the issue... sorry you had to spend a few hours figuring it out.

[cantaspinar]

I am trying to use this with the connection approval process:

private void ApprovalCheck(NetworkManager.ConnectionApprovalRequest request, NetworkManager.ConnectionApprovalResponse response) { response.Approved = false; response.Reason = "Go away!"; }

My client indeed gets disconnected but the NetworkManager.Singleton.DisconnectReason returns an empty string on the client side. Am I missing sth?

Update: This happens when using Facepunch Transport. If I use Unity Transport, it works fine.

[NoelStephensUnity]

@cantaspinar
Is this the transport you are using?
If so, then it looks like it bypasses the NGO disconnect process if you are using that specific API...so I am not sure setting that in the approval check will have any impact.

If that is the third party library you are using, then you might file a github issue there.
If that is not the third party library you are using, then you could post it here and I could take a look when time allows.

[cantaspinar]

@NoelStephensUnity

I have been using the community facepunch transport.

issue

I have also tried the other community transport called SteamNetworkingSockets. Same issue with that one as well. :/

issue

Btw, I am just selecting the transport in the Netcode's NetworkManager component. Not using a custom NetworkManager implementation. I have created 2 issues

[NoelStephensUnity]

Ahh... sorry about that misunderstanding.
Looking at the community facepunch transport, the only thing I can think of is that the connection is closed before the message is actually sent.

So, you are checking the client-side's NetworkManager.DisconnectReason and that is either null or String.Empty when the client is not approved and disconnected?

[cantaspinar]

Ahh... sorry about that misunderstanding. Looking at the community facepunch transport, the only thing I can think of is that the connection is closed before the message is actually sent.

So, you are checking the client-side's NetworkManager.DisconnectReason and that is either null or String.Empty when the client is not approved and disconnected?

That is correct. It also appears like both transports does not terminate the connection properly? Facepunch goes to a timeout and the SteamNetworkingSockets gives this error: Can't disconect client, client not connected, clientId: XXXXXX

[NoelStephensUnity]

So here is the delta between UnityTransport and the community Facepunch:
UnityTransport flushes (sends) any pending messages before it disconnects the client:

        public override void DisconnectRemoteClient(ulong clientId)
        {
            Debug.Assert(m_State == State.Listening, "DisconnectRemoteClient should be called on a listening server");

            if (m_State == State.Listening)
            {
                FlushSendQueuesForClientId(clientId);

The community Facepunch source code looks like it just closes the connection:

public override void DisconnectRemoteClient(ulong clientId)
{
    if (connectedClients.TryGetValue(clientId, out Client user))
    {
        user.connection.Close();
        connectedClients.Remove(clientId);

So, I am going to bet that is why the disconnect message isn't sent. On the client side, the transport level "disconnect" event is what is most likely happening (which will not have that message).

Hmm...

[NoelStephensUnity]

@cantaspinar
In the FacepunchTransport.cs file, make this modification and let me know if it starts working for you:

        public override void DisconnectRemoteClient(ulong clientId)
        {
            if (connectedClients.TryGetValue(clientId, out Client user))
            {
               // Flush any pending messages before closing the connection
                user.connection.Flush();
                user.connection.Close();
                connectedClients.Remove(clientId);

                if (LogLevel <= LogLevel.Developer)
                    Debug.Log($"[{nameof(FacepunchTransport)}] - Disconnecting remote client with ID {clientId}.");
            }
            else if (LogLevel <= LogLevel.Normal)
                Debug.LogWarning($"[{nameof(FacepunchTransport)}] - Failed to disconnect remote client with ID {clientId}, client not connected.");
        }

[cantaspinar]

It worked out, huge thanks! I can get the disconnect messages on the client side now. I still get the following error though when approved is set to false.

KeyNotFoundException: The given key '1' was not present in the dictionary.
System.Collections.Generic.Dictionary`2[TKey,TValue].get_Item (TKey key) (at <d6232873609549b8a045fa15811a5bd3>:0)
Unity.Netcode.NetworkManager.ClientIdToTransportId (System.UInt64 clientId) (at Library/PackageCache/[email protected]/Runtime/Core/NetworkManager.cs:1781)
Unity.Netcode.NetworkManager.DisconnectRemoteClient (System.UInt64 clientId) (at Library/PackageCache/[email protected]/Runtime/Core/NetworkManager.cs:1361)
Unity.Netcode.NetworkManager.DisconnectClient (System.UInt64 clientId, System.String reason) (at Library/PackageCache/[email protected]/Runtime/Core/NetworkManager.cs:2063)
Unity.Netcode.NetworkManager.DisconnectClient (System.UInt64 clientId) (at Library/PackageCache/[email protected]/Runtime/Core/NetworkManager.cs:2038)
Unity.Netcode.NetworkManager+<ApprovalTimeout>d__180.MoveNext () (at Library/PackageCache/[email protected]/Runtime/Core/NetworkManager.cs:1765)
UnityEngine.SetupCoroutine.InvokeMoveNext (System.Collections.IEnumerator enumerator, System.IntPtr returnValueAddress) (at <bae255e3e08e46f7bc2fbd23dde96338>:0)

To be clear, I was getting this error before your modification too. It doesn't seem to break anything else though. This error comes after 10 seconds, which is the default Client Connection Buffer Timeout duration I guess.

[NoelStephensUnity]

@cantaspinar This happens on the server or client side?
(It looks like server side... never mind...let me see if there is something else the Facepunch integration is not doing that could cause this issue)

[NoelStephensUnity]

@cantaspinar
Actually, that looks like a valid bug on the NGO side of things.
If the client is not approved before the timeout then it looks like it will try to disconnect it again.

[cantaspinar]

Right, I had a look at the call trace and I guess this is the place. (NetworkManager, Line 1737)

            // If the client timed out or was not approved
            if (timedOut || connectionNotApproved)
            {
                // Timeout
                if (NetworkLog.CurrentLogLevel <= LogLevel.Developer)
                {
                    if (timedOut)
                    {
                        if (IsServer)
                        {
                            // Log a warning that the transport detected a connection but then did not receive a follow up connection request message.
                            // (hacking or something happened to the server's network connection)
                            NetworkLog.LogWarning($"Server detected a transport connection from Client-{clientId}, but timed out waiting for the connection request message.");
                        }
                        else
                        {
                            // We only provide informational logging for the client side
                            NetworkLog.LogInfo("Timed out waiting for the server to approve the connection request.");
                        }
                    }
                    else if (connectionNotApproved)
                    {
                        NetworkLog.LogInfo($"Client-{clientId} was either denied approval or disconnected while being approved.");
                    }
                }

                if (IsServer)
                {
                    DisconnectClient(clientId);
                }
                else
                {
                    Shutdown(true);
                }
            }

I guess the Unity transport is handling this as well?

[NoelStephensUnity]

@cantaspinar
Indeed UnityTransport handles this differently, but we did require an adjustment to account for 3rd party implementations that might handle disconnection differently (and notification of disconnection).
Also pushed the fix for Facepunch not flushing the send queue upon disconnecting a client. 👍