Saturday, March 20, 2010

How to properly close WCF connections

    Seems that reading InfoQ is really a good thing, even though the content related to .NET is pretty weak, it has really good content on Architecture and Languages.While doing the weekly swoop I stumbled across this article which triggered the alarm/sleepy memory in my head. The not so funny thing is that just last week I implemented the Dispose pattern for some WCF connections to a web service,  because I’m a programmer and programmers are lazy, and lazy means less code to do more, which kind of forces you to use the Dispose pattern and the “using" statement to automatically close your connections.
    If you don’t have the time to read the article and the references, the summary is that using the Dispose pattern is a bad practice for WCF connections, because well, someone at MSFT screwed up the design and ICommunicationObject.Close() throws a bunch of exceptions, which goes against the specifications for IDisposable which should NEVER throw an exception on Dispose(). So let’s see what code I have before:

   17 public partial class IFooServiceClient : IDisposable
   18 {
   19     public void Dispose()
   20     {
   21         Close();
   22     }
   23 }
    This is not good code, but not really bad either as long as Close doesn’t throw, which does, so slap in the face and let’s fix this. Someone proposed something like this which make things worse:
   25 public partial class IFooServiceClient : IDisposable
   26 {
   27     public void Dispose()
   28     {
   29         try
   30         {
   31             Close();
   32         }
   33         catch (Exception)
   34         {
   35             if (State == CommunicationState.Opened)
   36             {
   37                 Close();
   38             }
   39             else
   40             {
   41                 Abort();
   42             }
   43         }
   44     }
   45 }
    This is bad because you lose the exception context and this has the possibility to deadlock as well, checking the channel state is an anti pattern as well. This is the solution I came up with:
class IFooServiceClientHelper
{
    internal T CallServiceMethod<T>(ICommunicationObject service,    Func<T> serviceMethod)
    {
        T returnedInstance = default(T);
        try
        {
            returnedInstance = serviceMethod();
            service.Close();
        }
        catch (FaultException<IFooServiceFault> iof)
        {
            service.Abort();
            throw new IFooServiceException("Executing" + serviceMethod.Method.Name + " has failed.", iof);
        }
        catch (CommunicationException e)
        {
            service.Abort();
            throw new IFooServiceConnectionException("There was a communication problem. " + e.Message, e);
        }
        catch (TimeoutException e)
        {
            service.Abort();
            throw new IFooServiceConnectionException("The service operation timed out. " + e.Message, e);
        }
        catch (InvalidMessageContractException e)
        {
            service.Abort();
            throw new IFooServiceConnectionException(
                "The DataContract for the web service is not up-to-date. " + e.Message, e);
        }
        catch (Exception e)
        {
            service.Abort();
            throw new IFooServiceException("Executing" + serviceMethod.Method.Name + " has failed." + e.Message, e);
        }
        return returnedInstance;
    }
}
    This helper method has multiple overloads in order to call Func with parameters but the idea is the same. Calling this is pretty easy:
public Foo GetFoo()
{
    var service = new FooServiceClient();
    var serviceHelper = new FooServiceHelper();
    return serviceHelper.CallServiceMethod<Foo>(service, service.GetFoo);
}

This is better than we had before, but maybe I can improve this more. Any ideas? 
Develop with passion, 
Adrian