Handling exceptions in asynchronous socket callbacks

Question

I've been trying to get my head wrapped around socket communications and I've put together a small Windows Forms application as a test. This is basically a client that will connect to a server, send some bytes and disconnect. Ultimately, I'll receive a response from the server, too, but I've mostly stripped that out for now.

As best I can tell, this code works properly but since the sending of the data occurs as a result of a button click in my UI, it needs to be asynchronous. I have a method called SendDataToServer(byte[] bytesToSend) that connects to the server and transmits the data. In my button click event handler, I created a backgroundworker which will call this method.

If my server isn't up and running, I'll obviously get a socket exception and certainly there are other reasons that an exception could get thrown during the process of attempting to connect and transfer data. With the backgroundworker and the async socket callbacks (ClientConnectCallback() and ClientSendCallback()), what's the best way to make sure that any exceptions get bubbled up and displayed properly in my UI?

Right now I have MessageBoxes inside the catch blocks within the async callbacks themselves but I'm wondering if this is really the place to be displaying the messages or if they should be passed back up?

Here's what my code looks like:

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Diagnostics;
using System.Net;
using System.Net.Sockets;
using System.Threading;
using System.Windows.Forms;

namespace ClientTest
{
    public partial class MyForm : Form
    {
        private string ServerIpAddress = "x.x.x.x";
        private int ServerPort = 59176;

        public Socket ClientSocket;

        private static ManualResetEvent connectDone = new ManualResetEvent(false);
        private static ManualResetEvent sendDone = new ManualResetEvent(false);

        // state object for reading client data asynchronously
        public class StateObject
        {
            // client socket
            public Socket socket = null;
            // size of receive buffer
            public const int BufferSize = 1024;
            // receive buffer
            public byte[] buffer = new byte[BufferSize];
            // all bytes received get added to this
            public List<byte> bytes = new List<byte>();
        }

        public MyForm()
        {
            InitializeComponent();
        }

        private void ClientConnectCallback(IAsyncResult asyncResult)
        {
            try
            {
                // retrieve the socket from the state object
                Socket client = (Socket)asyncResult.AsyncState;

                // complete the connection
                client.EndConnect(asyncResult);

                // signal that the connection has been made
                connectDone.Set();
            }
            catch (SocketException sockEx)
            {
                // if the server isn't running, we'll get a socket exception here
                MessageBox.Show(sockEx.Message, Application.ProductName, MessageBoxButtons.OK, MessageBoxIcon.Error);
            }
            catch (ObjectDisposedException)
            {
            }
            catch (Exception ex)
            {
                MessageBox.Show(ex.Message, Application.ProductName, MessageBoxButtons.OK, MessageBoxIcon.Error);
            }
        }

        private void ClientSendCallback(IAsyncResult asyncResult)
        {
            try
            {
                // retrieve the socket from the state object
                Socket client = (Socket)asyncResult.AsyncState;

                // complete sending the data to the server
                int bytesSent = client.EndSend(asyncResult);

                // signal that all bytes have been sent
                sendDone.Set();
            }
            catch (ObjectDisposedException objDispEx)
            {
                Debug.WriteLine(objDispEx.Message);
            }
            catch (Exception ex)
            {
                MessageBox.Show(ex.Message, Application.ProductName, MessageBoxButtons.OK, MessageBoxIcon.Error);
            }
        }

        public void SendDataToServer(byte[] bytesToSend)
        {
            try
            {
                connectDone.Reset();
                sendDone.Reset();

                ClientSocket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);

                IPAddress ipAddress = IPAddress.Parse(ServerIpAddress);
                IPEndPoint remoteEndPoint = new IPEndPoint(ipAddress, ServerPort);

                ClientSocket.BeginConnect(remoteEndPoint, new AsyncCallback(ClientConnectCallback), ClientSocket);
                connectDone.WaitOne();

                ClientSocket.BeginSend(bytesToSend, 0, bytesToSend.Length, 0, new AsyncCallback(ClientSendCallback), ClientSocket);
                sendDone.WaitOne();
            }
            catch (ObjectDisposedException objDispEx)
            {
                Debug.WriteLine(objDispEx.Message);
            }
            catch (Exception ex)
            {
                MessageBox.Show(ex.Message, Application.ProductName, MessageBoxButtons.OK, MessageBoxIcon.Error);
            }
            finally
            {
                ClientSocket.Shutdown(SocketShutdown.Both);
                ClientSocket.Close();
            }
        }

        private void buttonSendDataToServer_Click(object sender, EventArgs e)
        {
            BackgroundWorker bwSendDataToServer = new BackgroundWorker();
            bwSendDataToServer.DoWork += bwSendDataToServer_DoWork;
            bwSendDataToServer.RunWorkerCompleted += bwSendDataToServer_RunWorkerCompleted;
            bwSendDataToServer.RunWorkerAsync();
        }

        private void bwSendDataToServer_DoWork(object sender, DoWorkEventArgs e)
        {
            byte[] bytesToSend = new byte[100];
            SendDataToServer(bytesToSend);
        }

        private void bwSendDataToServer_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
        {
            Debug.WriteLine("BackgroundWorker has completed.");
        }
    }
}

Answer 1

"How do I report exceptions?" is a pretty broad question. It's hard to know for sure what specific advice would be best in your case, as there are many options for solving the problem.

That said, there are some general rules that IMHO you should always follow, and these can help direct your design:

1. Do not block I/O threads with UI.

This means that for sure, your code example should be improved, as you are delaying any I/O thread used to call your callbacks while waiting for the user to acknowledge the error message.

2. Observe the general OOP principle of "separation of concerns".

In other words, even if the thread that is presenting the error message isn't an I/O thread, it's still not a good idea to have user interaction logic inside a class that's main purpose is to handle network I/O.

There are of course other useful rules for programming, but I think the above two are most relevant considering your current code example. So, how to address these?

At a minimum, I would change your current implementation so that none of the I/O code was in the MyForm class. The MyForm class is for user-interface; it should not involve itself with the mechanisms of network I/O, but rather should delegate that work to some other class.

Now, that other class still needs to communicate with MyForm. But it should do so in a way that is independent of MyForm, i.e. which is not tied to (or "coupled") the class. In terms of reporting exceptions, one common way to accomplish this is to declare an event that is raised whenever an exception occurs.

That would address the second point, but the first point is still an issue. It can be resolved by using the Control.BeginInvoke() method to execute the event-handling code in the form object, so that that code is executed in the UI thread, and asynchronously relative to the I/O thread (i.e. the BeginInvoke() method doesn't wait for the invoked code to complete before returning).

Putting all that together, you might get something like this for example:

class ExceptionReportEventArgs : EventArgs
{
    public Exception Exception { get; private set; }

    public ExceptionEventArgs(Exception exception)
    {
        Exception = exception;
    }
}

class MyNetworkClient
{
    public event EventHandler<ExceptionReportEventArgs> ExceptionReport;

    private void OnExceptionReport(Exception exception)
    {
        EventHandler<ExceptionReportEventArgs> handler = ExceptionReport;

        if (handler != null)
        {
            handler(this, new ExceptionReportEventArgs(exception));
        }
    }

    // For example...
    private void ClientConnectCallback(IAsyncResult asyncResult)
    {
        try
        {
            /* code omitted */
        }
        catch (SocketException sockEx)
        {
            // if the server isn't running, we'll get a socket exception here
            OnExceptionReport(sockEx);
        }
        catch (ObjectDisposedException)
        {
        }
        catch (Exception ex)
        {
            OnExceptionReport(ex);
        }
    }
}

partial class MyForm : Form
{
    private MyNetworkClient _client;

    public MyForm()
    {
        InitializeComponent();

        _client = new MyNetworkClient();
        _client.ExceptionReport += (sender, e) =>
        {
            BeginInvoke((MethodInvoker)(() =>
                MessageBox.Show(e.Exception.Message, Application.ProductName,
                    MessageBoxButtons.OK, MessageBoxIcon.Error)));
        };
    }
}

Now, all that said: as I mentioned, there are lots of ways to address the basic issues. An alternative to the above would be to use the async/await pattern, which provides a mechanism to cleanly write asynchronous logic in an easy-to-read, imperative-style way, while still allowing for correct transitions between code that has to execute in the UI thread and code that does not.

I won't go into a lot of detail on that point — there are a lot of examples around, and it would require a much larger rework of the code you posted — but the basic idea would be to use e.g. NetworkStream, which has built-in async methods to handle the I/O, and to let exceptions "bubble up" from the I/O methods.

In this approach, you wouldn't need the ExceptionReport event. Instead, your MyNetworkClient class above would offer async methods that a controller-class (e.g. your MyForm object) can call to initiate I/O. If an exception occurs, each async method in the call stack can handle (if needed) the exception but then rethrow it (i.e. with a plain throw; statement), all the way back to the UI code, where the exception will be raised in the UI thread and can be presented to the user there, without blocking an I/O thread.

I think in general, the key is to just keep in mind the usual rules and stick with them. Ultimately, you will find the code is easier to write and understand, and will behave better in your chosen UI framework (e.g. Winforms).