Rare NRE on Telepathy.Server.GetClientAddress()

[cdanek]

I apologize for being rather light on information, this issue crops up extremely rarely for us in production. It appears as if something is causing a null reference exception on connection events line 326 in Server.cs. My server is containerized and runs in Azure's cloud, but I don't have a lot of excellent infrastructure for tracking these sorts of issues. All I have is a stack trace from the event:

System.NullReferenceException: Object reference not set to an instance of an object.
   at Telepathy.Server.GetClientAddress(Int32 connectionId) in /home/runner/work/ISG-Master/ISG-Master/Telepathy/Server.cs:line 326
   at <my code>.HandleConnectedEvent(Int32 connectionId) in <my code>/TelepathyServerSocket.cs:line 108
   at Telepathy.Server.Tick(Int32 processLimit, Func_1 checkEnabled) in /home/runner/work/ISG-Master/ISG-Master/Telepathy/Server.cs:line 378
   at <my code>.Poll() in <my code>/TelepathyServerSocket.cs:line 67
   at <my code>.RunAsync() in <my code>/NetworkManager.cs:line 86
   at Program.<Main>$(String[] args) in <my code>/Program.cs:line 48

I don't quite know what's causing the NRE, but might propose that the line is rewritten with fewer chained method calls to make it more obvious in the future? I'll submit a PR for it, but if it's obvious to the author what the cause is, great. :)

[PyrateAkananto]

I can consistently reproduce a NullReferenceException (NRE) by calling GetClientAddress within the callback OnDisconnected of Telepathy.Server. In the line return ((IPEndPoint)connection.client.Client.RemoteEndPoint).Address.ToString(); the second Client (with capital C) is null in this case. I assume when the callback OnDisconnected is called the connection is still within readonly ConcurrentDictionary<int, ConnectionState> clients but public Socket Client { get; set; } of System.Net.Sockets.TcpClient is already null.

In your pull request https://github.com/vis2k/Telepathy/pull/124 I would not only split the call chain but even add if (... != null) checks. If one of those actually is null, I would simply return an empty string.

[cdanek]

Thanks for that. I've updated my code locally and added some NRE checks to the PR. They might not all be necessary, but they're inexpensive checks.

[PyrateAkananto]

GetClientAddress was improved in 2c9d26b and 6ffca35 and I assume this issue here is obsolete now.