Computer monitoring system via Asynchronous TcpListener & TcpClient

[threeeye]

Hi guys,
I'm following @jmcilhinney code bank thread (I didn't want to wake up an old code bank thread...) on how to do a Server/Client messaging app, and converting it into a PC monitoring app... (thank you!)
I'm using @Johno2518 GitHub code from post # 258 (https://github.com/Johno-ACSLive/ACS-Messaging)

So far so good, I can connect multiple clients...
But I want the server to sent the clients a "heartbeat" to know if the client is still alive, and if not, the server should remove it.

Sending info from the client to the server works great, but sending something (anything) from the server to the client gives me an error:

Exception thrown: 'System.InvalidOperationException' in System.Linq.dll ("Sequence contains no elements")

Looking at the code where the error is (@Johno2518):

        public void SendData(HostInfo host, byte[] data)
        {
            try
            {
                TcpClient client = clients.Select(c => new { client = c, host = c.Value }).Where(x => x.host.Equals(host)).Select(x => x.client.Key).First();
                Send(client, data, host.SecureStream).ConfigureAwait(false);
            }
            catch (Exception Ex)
            {
                // Don't know what the hell happened, lets log the exception.
                // It was probably due to some idiot trying to send data to a client that has disconnected
                OnLog(new LogEventArgs(DateTime.Now, "ERROR", Ex.ToString()));
            }
        }

The error is on this line:

TcpClient client = clients.Select(c => new { client = c, host = c.Value }).Where(x => x.host.Equals(host)).Select(x => x.client.Key).First();

The comment "It was probably due to some idiot trying to send data to a client that has disconnected"
But my client is still connected...

Here is my code for the Server:

Imports System.Text

Public Class PCMonServer
    Dim MServer As ACS.Messaging.MessageServer
    Dim Clients As New List(Of String)
    Private Sub PCMonServer_Load(sender As Object, e As EventArgs) Handles MyBase.Load
        MServer = New ACS.Messaging.MessageServer("192.168.1.121", 64555, False)
        AddHandler MServer.ConnectionAccepted, AddressOf ConnAccept
        AddHandler MServer.ConnectionClosed, AddressOf ConnClose
        AddHandler MServer.MessageReceived, AddressOf MSGIn
    End Sub

#Region "Incoming connection / add Panel"
    Private Sub MSGIn(sender As Object, e As MessageReceivedEventArgs)
        Dim str As String = ByteArrayToStr(e.Data)
        Invoke(Sub()
                   If str.Split(New Char() {"|"c})(0) = "0" Then AddPanel(str)
               End Sub)
    End Sub
    Private Shared Function ByteArrayToStr(byteArray As Byte()) As String
        Dim str As String
        Dim enc As New UTF8Encoding()
        str = enc.GetString(byteArray)
        Return str
    End Function
    Private Sub AddPanel(str As String)
        Dim info As String() = str.Split(New Char() {"|"c})
        Dim p As New Panel

        Dim NameLabel As New Label With {.AutoSize = True,
                                         .Font = New Font("Segoe UI", 12, FontStyle.Bold),
                                         .Location = New Point(0, 0),
                                         .Text = info(1) & "   "}

        Dim IPLabel As New Label With {.AutoSize = True,
                                         .Font = New Font("Segoe UI", 9),
                                         .Location = New Point(0, NameLabel.Height - 1),
                                         .Text = info(2)}

        Dim UserLabel As New Label With {.AutoSize = True,
                                      .Font = New Font("Segoe UI", 9),
                                      .Location = New Point(0, NameLabel.Height + IPLabel.Height - 8),
                                      .Text = "Current User: " & info(3)}


        Dim LUserLabel As New Label With {.AutoSize = True,
                                      .Font = New Font("Segoe UI", 9),
                                      .Location = New Point(0, UserLabel.Bottom - 7),
                                      .Text = "Logged off User(s):" & Environment.NewLine & info(4)}

        p.Controls.Add(NameLabel)
        p.Controls.Add(IPLabel)
        p.Controls.Add(UserLabel)
        p.Controls.Add(LUserLabel)

        Dim maxWidth As Integer = -1
        For Each l As Label In p.Controls
            If l.Width > maxWidth Then maxWidth = l.Width
        Next

        p.AutoSize = False
        p.Size = New Size(183, 89)
        p.BorderStyle = BorderStyle.Fixed3D
        p.Tag = info(2)
        p.BackColor = Color.White

        Controls.Add(p)

        ReorderPanel()
    End Sub
    Private Sub ReorderPanel()
        Dim BoxWidth As Integer = 183
        Dim BoxHeit As Integer = 89
        Dim ColNum As Integer = Width / BoxWidth

        Dim CNum As Integer = 0
        Dim a As Control
        For Each a In Controls
            If TypeOf a Is Panel Then
                a.Location = New Point((BoxWidth + 5) * (CNum Mod ColNum), Fix(CNum / ColNum) * BoxHeit)
                CNum += 1
            End If
        Next
    End Sub
#End Region

#Region "Close connection / remove Panel"
    Private Sub ConnClose(sender As Object, e As ConnectionEventArgs)
        RemovePanel(e.Host.HostName)
    End Sub
    Private Sub RemovePanel(IP As String)
        Dim a As Control
        For Each a In Controls
            If TypeOf a Is Panel Then
                If a.Tag = IP Then
                    Invoke(Sub()
                               Clients.Remove(IP)

                               a.Dispose()
                           End Sub)
                End If
            End If
        Next

        Invoke(Sub()
                   ReorderPanel()
               End Sub)
    End Sub
#End Region

#Region "Send life signal"
    Private Sub SendMSG(Client As HostInfo)
        Invoke(Sub()
                   MServer.SendData(Client, StrToByteArray("Test"))
               End Sub)
    End Sub
    Private Shared Function StrToByteArray(str As String) As Byte()
        Dim encoding As New UTF8Encoding()
        Return encoding.GetBytes(str)
    End Function
#End Region

    Private Sub ConnAccept(sender As Object, e As ConnectionEventArgs)
        Clients.Add(e.Host.HostName)
    End Sub

    Private Sub PCMonServer_FormClosing(sender As Object, e As FormClosingEventArgs) Handles Me.FormClosing
        MServer.Dispose()
    End Sub

    Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click
        'For Each h As HostInfo In MServer.Hosts
        '    MsgBox(h.HostName)
        'Next
        'MsgBox(MServer.Hosts.Count)
        Dim h As New HostInfo("192.168.1.130", 64555, False)
        SendMSG(h)
    End Sub
End Class

And this is the code for the Client:

Imports System.Net
Imports System.Text
Imports System.Threading

Public Class PCMonClient
    Dim MClient As ACS.Messaging.MessageClient
    Private Sub PCMonClient_Load(sender As Object, e As EventArgs) Handles MyBase.Load
        Connect()
    End Sub

    Private Sub Connect()
        MClient = New ACS.Messaging.MessageClient("192.168.1.121", 64555, False)

        AddHandler MClient.MessageReceived, AddressOf MSGIn

        MClient.Connect()
        Thread.Sleep(100)

        Sendinfo()
    End Sub

#Region "Send info to server"
    Private Sub Sendinfo()
        Dim CName As String = Environment.MachineName
        Dim UName As String = Environment.UserName
        Dim IP As String = GetIP()

        Dim str As String = "0|" & CName & "|" & IP & "|" & UName & "|" '---   TODO: get disconnected users (I should have the code somewhere)

        MClient.SendData(StrToByteArray(str))
    End Sub
    Private Shared Function StrToByteArray(str As String) As Byte()
        Dim encoding As New UTF8Encoding()
        Return encoding.GetBytes(str)
    End Function
#End Region

#Region "Receive heartbeat"
    Private Sub MSGIn(sender As Object, e As MessageReceivedEventArgs)
        MsgBox(ByteArrayToStr(e.Data))
    End Sub
    Private Shared Function ByteArrayToStr(byteArray As Byte()) As String
        Dim str As String
        Dim enc As New UTF8Encoding()
        str = enc.GetString(byteArray)
        Return str
    End Function
#End Region

    Private Shared Function GetIP()
        Dim IP As String = Nothing
        Dim hostName = Dns.GetHostName()
        For Each hostAdr In Dns.GetHostEntry(hostName).AddressList()
            If hostAdr.ToString().StartsWith("192.168.1.") Then IP = hostAdr.ToString() : Exit For
        Next

        Return IP
    End Function

    Private Sub PCMonClient_FormClosing(sender As Object, e As FormClosingEventArgs) Handles Me.FormClosing
        MClient.Dispose()
    End Sub





    'Buttons 1 and 2 are tests to see if the server knows that the client was disconnected
    Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click
        Connect()
    End Sub
    Private Sub Button2_Click(sender As Object, e As EventArgs) Handles Button2.Click
        'If Client.Client.Connected = True Then
        '    Client.Client.Client.Close()
        'End If
        MClient.Dispose()
    End Sub
End Class

Thanks for the help

[jmcilhinney]

The specific issue is that you are calling First on a sequence with no items. If there's a chance that the sequence is empty then you need to call FirstOrDefault and then check whether the result is Nothing. Consider this:

Dim words = {"if", "and", "but"}
Dim word = words.Where(s => s.Length > 3).First()

As you can see, there are no Strings in the array that have a Length greater than 3 so Where will return an empty sequence and First will throw the same exception. If the original data and the filter are valid then you need to allow for the fact that there may not be a matching item. You can use FirstOrDefault like this:

Dim words = {"if", "and", "but"}
Dim word = words.Where(s => s.Length > 3).FirstOrDefault()
 
If word Is Nothing Then
    Console.WriteLine("No matches found")
Else
    Console.WriteLine("First match: " & word)
End If

[jmcilhinney]

I would also suggest that you could simplify your code considerably. This:

TcpClient client = clients.Select(c => new { client = c, host = c.Value }).Where(x => x.host.Equals(host)).Select(x => x.client.Key).First();

could be simplified to this:

TcpClient client = clients.Where(c => c.Value.Equals(host)).Select(c => c.Key).First();

which could be simplified to this:

TcpClient client = clients.First(c => c.Value.Equals(host)).Key;

If you need to use FirstOrDefault, you can just use null propagation with it:

TcpClient client = clients.FirstOrDefault(c => c.Value.Equals(host))?.Key;

If FirstOrDefault returns a value then its Key property will be assigned to the variable and if FirstOrDefault returns Nothing then the whole expression evaluates to Nothing and the variable will be set to Nothing.

[threeeye]

The specific issue is that you are calling First on a sequence with no items. If there's a chance that the sequence is empty then you need to call FirstOrDefault and then check whether the result is Nothing. Consider this:

Dim words = {"if", "and", "but"}
Dim word = words.Where(s => s.Length > 3).First()

As you can see, there are no Strings in the array that have a Length greater than 3 so Where will return an empty sequence and First will throw the same exception. If the original data and the filter are valid then you need to allow for the fact that there may not be a matching item. You can use FirstOrDefault like this:

Dim words = {"if", "and", "but"}
Dim word = words.Where(s => s.Length > 3).FirstOrDefault()
 
If word Is Nothing Then
    Console.WriteLine("No matches found")
Else
    Console.WriteLine("First match: " & word)
End If

I think I get what you are saying...

I would also suggest that you could simplify your code considerably. This:

TcpClient client = clients.Select(c => new { client = c, host = c.Value }).Where(x => x.host.Equals(host)).Select(x => x.client.Key).First();

could be simplified to this:

TcpClient client = clients.Where(c => c.Value.Equals(host)).Select(c => c.Key).First();

which could be simplified to this:

TcpClient client = clients.First(c => c.Value.Equals(host)).Key;

If you need to use FirstOrDefault, you can just use null propagation with it:

TcpClient client = clients.FirstOrDefault(c => c.Value.Equals(host))?.Key;

If FirstOrDefault returns a value then its Key property will be assigned to the variable and if FirstOrDefault returns Nothing then the whole expression evaluates to Nothing and the variable will be set to Nothing.

Well, I didn't write this, it's on the DLL from the GitHub, but I changed it to this:

TcpClient client = clients.FirstOrDefault(c => c.Value.Equals(host))?.Key;

as you suggested.

But I still get an error, just now it's on this line:

MServer.SendData(Client, StrToByteArray("Test"))

(BTW, I tried removing the line from the Invoke, and also putting it in a Try Catch...)
But it works when I remove the "Client" (sending data to ALL the client), the server is having issues when I specify a client...

I don't really need to send a message to the Clients to get a heartbeat, right?
Is there a way to handle a client disconnecting unexpectedly (Client app/pc/network crashed)?
If so, I don't need that part...

O, and thanks again...

[jmcilhinney]

But I still get an error, just now it's on this line:

MServer.SendData(Client, StrToByteArray("Test"))

Different error for a different reason. The issue you originally asked about has been resolved so you should mark this thread Resolved, using the Thread Tools menu, and create a new thread for the new issue.

[threeeye]

Different error for a different reason. The issue you originally asked about has been resolved so you should mark this thread Resolved, using the Thread Tools menu, and create a new thread for the new issue.

Sorry for the late reply (I was on vacation...)
It is the same error, just on different line (pointing to the same sub).
The issue on both lines is when trying to send a message from the Server to a specific Client, but sending to ALL clients works...

[jmcilhinney]

Think about what you're doing here. First will throw an exception if the sequence contains no items. FirstOrDefault will not throw an exception but return Nothing. Using FirstOrDefault won't magically add items to the sequence so you can't magically use the result of FirstOrDefault as though it did. If there were no TcpClient objects in clients before then there still aren't, so why would you expect to be able to get one and use it there? Your root problem is that there are no TcpClients in your list so you need to go back and look at where you expect one or more to be added and work out why it isn't. That's what you have a debugger for. Reading code can only do so much. You need to watch it as it executes to see what parts get executed when and what data they are using at the time. That's what VS has a debugger for.

[threeeye]

Think about what you're doing here. First will throw an exception if the sequence contains no items. FirstOrDefault will not throw an exception but return Nothing. Using FirstOrDefault won't magically add items to the sequence so you can't magically use the result of FirstOrDefault as though it did. If there were no TcpClient objects in clients before then there still aren't, so why would you expect to be able to get one and use it there? Your root problem is that there are no TcpClients in your list so you need to go back and look at where you expect one or more to be added and work out why it isn't. That's what you have a debugger for. Reading code can only do so much. You need to watch it as it executes to see what parts get executed when and what data they are using at the time. That's what VS has a debugger for.

That's the thing, there is a Client connected.
I put this as a test in the button:

        MsgBox(MServer.Hosts.Count)

        For Each c In MServer.Hosts
            MsgBox(c.HostName)
        Next

Results:
before I connect the Client: first MsgBox gives me 0, as expected, and I don't get a 2nd MsgBox (also expected)
after I connect the Client: first MsgBox gives me 1, as expected, and the 2nd MsgBox gives me the IP address of the Client (in my case 192.168.1.130 (and yes, I did verify it))

And if I put this:

        For Each h As HostInfo In MServer.Hosts
            MServer.SendData(h.HostName, h.Port, StrToByteArray("Test"))
        Next

the Client dos get the "Test" message...

But TBH, if there is a way do handle an unexpected disconnection, I would not need to send a message, but I'll open a new thread for that...

[jmcilhinney]

That's the thing, there is a Client connected.

It doesn't matter whether there's a client connected. It matters whether there's a client in the clients list. There obviously isn't, or First wouldn't have told you that there are no items in the sequence. You mustn't be adding anything to that list, or else you're clearing some time after you do add items. You need to make sure that you are adding that connected to client to that list. Where do you think that's happening? If you don't know then that's the problem. If you think you do know then place a breakpoint there and debug your code to make sure that it's being executed.