views:

2056

answers:

5

I am implementing an asynchronous command pattern for the "client" class in a client/server application. I have done some socket coding in the past and I like the new Async pattern that they used in the Socket / SocketAsyncEventArgs classes.

My async method looks like this: public bool ExecuteAsync(Command cmd); It returns true if the execution is pending and false if it completed synchronously. My question is: Should I always call the callback (cmd.OnCompleted), even in the event of an exception? Or should I throw exceptions right from ExecuteAsync?

Here are some more details if you need them. This is similar to using SocketAsyncEventArgs, but instead of SocketAsyncEventArgs my class is called SomeCmd.

SomeCmd cmd = new SomeCmd(23, 14, 10, "hike!");
cmd.OnCompleted += this.SomeCmd_OnCompleted;
this.ConnectionToServer.ExecuteAsync(cmd);

As with the Socket class, if you need to coordinate with your callback method (SomeCmd_OnCompleted in this case), you can use the return value of ExecuteAsync to know if the operation is pending (true) or if the operation completed synchronously.

SomeCmd cmd = new SomeCmd(23, 14, 10, "hike!");
cmd.OnCompleted += this.SomeCmd_OnCompleted;
if( this.ConnectionToServer.ExecuteAsync(cmd) )
{
    Monitor.Wait( this.WillBePulsedBy_SomeCmd_OnCompleted );
}

Here is a greatly simplified version of my base classes, but you can see how it works:

class Connection
{
    public bool ExecuteAsync(Command cmd)
    {
     /// CONSIDER: If you don't catch every exception here
     /// then every caller of this method must have 2 sets of
                /// exception handling:
     /// One in the handler of Command.OnCompleted and one where ExecuteAsync
     /// is called.
     try
     {
     /// Some possible exceptions here:
     /// 1) remote is disposed. happens when the other side disconnects (WCF).
     /// 2) I do something wrong in TrackCommand (a bug that I want to fix!)
      this.TrackCommand(cmd);
      remote.ServerExecuteAsync( cmd.GetRequest() );
      return true;
     }
     catch(Exception ex)
     {
      /// Command completing synchronously.
      cmd.Completed(ex, true);
      return false;
     }
    }
    /// <summary>This is what gets called by some magic when the server returns a response.</summary>
    internal CommandExecuteReturn(CommandResponse response)
    {
     Command cmd = this.GetTrackedCommand(response.RequestId);
     /// Command completing asynchronously.
     cmd.Completed(response, false);
    }

    private IServer remote;
}

abstract class Command: EventArgs
{
    internal void Completed(Exception ex, bool synchronously)
    {
     this.Exception = ex;

     this.CompletedSynchronously = synchronously;

     if( this.OnCompleted != null )
     {
      this.OnCompleted(this);
     }
    }

    internal void Completed(CommandResponse response, bool synchronously)
    {
     this.Response = response;
     this.Completed(response.ExceptionFromServer, synchronously)
    }

    public bool CompletedSynchronously{ get; private set; }

    public event EventHandler<Command> OnCompleted;

    public Exception Exception{ get; private set; }

    internal protected abstract CommandRequest GetRequest();
}
A: 

I would throw a custom exception and not call the completed callback. After all, the command was not completed if an exception occurred.

CStick
+2  A: 

I would not throw an exception in the ExecuteAsync, and instead set the exception condition for the callback. This will create a consistent way of programming against the asynchronous logic and cut down on repetitive code. The client can call this class and expect one way to handle exceptions. This will provide less buggy, less brittle code.

Jason Jackson
+1  A: 

throwing an exception from the dispatch point may or may not be useful

calling the callback passing an exception argument requires the completion callback to do 2 distinct things

a second callback for exception reporting might make sense instead

Steven A. Lowe
Now, why didn't I think of that? The funny thing is that I've actually done that before for a class that runs a series of queries.
wizlb
This is the correct answer because basically you said "it depends".
wizlb
A: 

Handling exceptions in one spot is a lot easier. I'd use the following distinction: for exceptions that should be handled, throw them in the callback. It makes it simpler to use the class. For exceptions that should not be caught (e.g., ArgumentException) throw in the ExecuteAsync. We want unhandled exceptions to blow up ASAP.

dpp
A: 

One general pattern for asynchronous operations in .NET (at least for the BackgroundWorker and the BeginInvoke()/EndInvoke() method pairs is to have a result object that separates the callback from the actual return value or any exceptions that occurred. It's the responsibility of the callback to handle the exception.

Some C#-like pseudocode:


private delegate int CommandDelegate(string number);

private void ExecuteCommandAsync()
{
    CommandDelegate del = new CommandDelegate(BeginExecuteCommand);
    del.BeginInvoke("four", new AsyncCallback(EndExecuteCommand), null);
}

private int BeginExecuteCommand(string number)
{
   if (number == "five")
   {
      return 5;
   }
   else
   {
      throw new InvalidOperationException("I only understand the number five!");
   }
}

private void EndExecuteCommand(IAsyncResult result)
{
    CommandDelegate del;
    int retVal;

    del = (CommandDelegate)((AsyncResult)result).AsyncDelegate;

    try
    {
        // Here's where we get the return value
        retVal = del.EndInvoke(result);
    }
    catch (InvalidOperationException e)
    {
        // See, we had EndExecuteCommand called, but the exception
        // from the Begin method got tossed here
    }
}

So if you call ExecuteCommandAsync(), it returns immediately. The BeginExecuteCommand() is launched in a separate thread. If it tosses an exception, that exception won't get thrown until you call EndInvoke() on the IAsyncResult (which you can cast to AsyncResult, which is documented but you could pass it in the state if the cast makes you uncomfortable. This way, the exception handling code is "naturally placed" around where you would be interacting with the return value of the method.

For more information, checkout more information on the IAsyncResult pattern on MSDN.

Hope this helps.

Nicholas Piasecki
Note that the exception will only be thrown on the EndXYZ cannot be guaranteed, even in .NET Framework code. For example, calling BeginXYZ on a WCF client service proxy pointing to an endpoint that doesn't exist. The exception is thrown immediately.
Alan Christensen