Where to trap failed connection on WCF calling class?

Question

I'm trying to write a class that encapsulates WCF calls (the client is Silverlight, if it matters). It all works swimmingly, but I'm not sure how to trap a connection failure, as if the server won't respond. It would appear that bit of work happens somewhere in the resulting code from ChannelFactory, but I'm not sure. General code review is welcome also. :)

Bottom line, surrounding the creation of the channel, or the begin or async result delegate in try/catch doesn't trap a failed connection. I'd like to have that catch run the ServiceCallError event.

public class ServiceCaller : IDisposable
{
    private IFeedService _channel;

    public ServiceCaller()
    {
        var elements = new List<BindingElement>();
        elements.Add(new BinaryMessageEncodingBindingElement());
        elements.Add(new HttpTransportBindingElement());
        var binding = new CustomBinding(elements);
        var endpointAddress = new EndpointAddress(App.GetRootUrl() + "Feed.svc");
        _channel = new ChannelFactory<IFeedService>(binding, endpointAddress).CreateChannel();
    }

    public void MakeCall(DateTime lastTime, Dictionary<string, string> context)
    {
        AsyncCallback asyncCallBack = delegate(IAsyncResult result)
        {
            var items = ((IFeedService)result.AsyncState).EndGet(result);
            if (ItemsRetrieved != null)
                ItemsRetrieved(this, new ServiceCallerEventArgs(items));
        };
        _channel.BeginGet(lastTime, context, asyncCallBack, _channel);
    }

    public event ItemsRetrievedEventHandler ItemsRetrieved;
    public event ServiceCallErrorHandler ServiceCallError;

    public delegate void ItemsRetrievedEventHandler(object sender, ServiceCallerEventArgs e);

    public delegate void ServiceCallErrorHandler(object sender, ServiceCallErrorEventArgs e);

    public void Dispose()
    {
        _channel.Close();
        _channel.Dispose();
    }
}

Here's the stack trace, for those who are curious:

 An AsyncCallback threw an exception.
at System.ServiceModel.AsyncResult.Complete(Boolean completedSynchronously)
   at System.ServiceModel.AsyncResult.Complete(Boolean completedSynchronously, Exception exception)
   at System.ServiceModel.Channels.HttpChannelFactory.HttpRequestChannel.HttpChannelAsyncRequest.OnGetResponse(IAsyncResult result)
   at System.Net.Browser.BrowserHttpWebRequest.<>c__DisplayClassd.<InvokeGetResponseCallback>b__b(Object state2)
   at System.Threading._ThreadPoolWaitCallback.WaitCallback_Context(Object state)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading._ThreadPoolWaitCallback.PerformWaitCallbackInternal(_ThreadPoolWaitCallback tpWaitCallBack)
   at System.Threading._ThreadPoolWaitCallback.PerformWaitCallback(Object state)

To make this happen, I fire up the app in the browser, then I kill the Web server process from Visual Studio. In a test environment, I get the same thing by killing the network connection for the client system.

Here's the FULL exception's ToString():

System.Exception: An AsyncCallback threw an exception. ---> System.Exception: An AsyncCallback threw an exception. ---> System.ServiceModel.CommunicationException: The remote server returned an error: NotFound. ---> System.Net.WebException: The remote server returned an error: NotFound. ---> System.Net.WebException: The remote server returned an error: NotFound.
   at System.Net.Browser.BrowserHttpWebRequest.InternalEndGetResponse(IAsyncResult asyncResult)
   at System.Net.Browser.BrowserHttpWebRequest.<>c__DisplayClass5.<EndGetResponse>b__4(Object sendState)
   at System.Net.Browser.AsyncHelper.<>c__DisplayClass2.<BeginOnUI>b__0(Object sendState)
   --- End of inner exception stack trace ---
   at System.Net.Browser.AsyncHelper.BeginOnUI(SendOrPostCallback beginMethod, Object state)
   at System.Net.Browser.BrowserHttpWebRequest.EndGetResponse(IAsyncResult asyncResult)
   at System.ServiceModel.Channels.HttpChannelFactory.HttpRequestChannel.HttpChannelAsyncRequest.CompleteGetResponse(IAsyncResult result)
   --- End of inner exception stack trace ---
   at System.ServiceModel.Channels.Remoting.RealProxy.Invoke(Object[] args)
   at proxy_2.EndGet(IAsyncResult )
   at CoasterBuzz.Feed.Client.ServiceCaller.<MakeCall>b__0(IAsyncResult result)
   at System.ServiceModel.AsyncResult.Complete(Boolean completedSynchronously)
   --- End of inner exception stack trace ---
   at System.ServiceModel.AsyncResult.Complete(Boolean completedSynchronously)
   at System.ServiceModel.AsyncResult.Complete(Boolean completedSynchronously, Exception exception)
   at System.ServiceModel.Channels.ServiceChannel.SendAsyncResult.CallComplete(Boolean completedSynchronously, Exception exception)
   at System.ServiceModel.Channels.ServiceChannel.SendAsyncResult.FinishSend(IAsyncResult result, Boolean completedSynchronously)
   at System.ServiceModel.Channels.ServiceChannel.SendAsyncResult.SendCallback(IAsyncResult result)
   at System.ServiceModel.AsyncResult.Complete(Boolean completedSynchronously)
   --- End of inner exception stack trace ---
   at System.ServiceModel.AsyncResult.Complete(Boolean completedSynchronously)
   at System.ServiceModel.AsyncResult.Complete(Boolean completedSynchronously, Exception exception)
   at System.ServiceModel.Channels.HttpChannelFactory.HttpRequestChannel.HttpChannelAsyncRequest.OnGetResponse(IAsyncResult result)
   at System.Net.Browser.BrowserHttpWebRequest.<>c__DisplayClassd.<InvokeGetResponseCallback>b__b(Object state2)
   at System.Threading._ThreadPoolWaitCallback.WaitCallback_Context(Object state)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading._ThreadPoolWaitCallback.PerformWaitCallbackInternal(_ThreadPoolWaitCallback tpWaitCallBack)
   at System.Threading._ThreadPoolWaitCallback.PerformWaitCallback(Object state)

By the way, the only way I can catch this at all is to use app level UnhandledException event in the SL client.

Answer 1

To catch communications errors with the server, I'd suggest putting the call to .CreateChannel() into a try...catch and handle things like CommunicationException and TimeoutExceptions there.

Also, one nuggest of general advice: creating the ChannelFactory is quite an expensive operation, so you might want to do this separately and store that reference to the ChannelFactory object somewhere.

If you need to create and possibly re-create the channel from the ChannelFactory, you can then use that stored / cached instance of the ChannelFactory over and over again without having to create that all the time.

But other than that, I would think you're doing quite fine here!

Marc

Answer 2

Jeff, I'm not entirely certain from your question how you are simulating a connection failure. I am assuming that "as if the server won't respond" means you are looking for some kind of timeout error. You are probably simulating a non-responsive server by forcing a slow response from your service call using Thread.Sleep() on the server side. Right? Close enough?

I had a similar project, so I tested this out and was able to trap a timeout and other exceptions successfully. I think perhaps you weren't seeing these exceptions because your custom binding had a very long default timeout and you may not have waited long enough.

To explain, WCF timeouts are controlled by the binding configuration. Looking at your code, you are creating your own custom binding like this:

var elements = new List<BindingElement>();
elements.Add(new BinaryMessageEncodingBindingElement());
elements.Add(new HttpTransportBindingElement());
var binding = new CustomBinding(elements);

If you set a breakpoint there and inspect the custom binding you created, you will see it has a one minute SendTimeout by default. I suspect you were either not waiting long enough or not setting your service's simulated timeout long enough, so you were not able to catch a timeout exception.

Here are some code changes to demonstrate. First, to set the timeout on the binding:

var binding = new CustomBinding(elements);
//Set the timeout to something reasonable for your service
//This will fail very quickly
binding.SendTimeout = TimeSpan.FromSeconds(1);

Finally, to catch the exceptions and raise the correct events, you can update your AsyncCallback delegate as follows. The exception is thrown when EndGet() is called.

AsyncCallback asyncCallBack = delegate(IAsyncResult result)
{
    IFeedService service = ((IFeedService)result.AsyncState);
    try
    {
        var items = service.EndGet(result);
        ItemsRetrieved(this, EventArgs.Empty);
    }
    catch (System.TimeoutException ex)
    {
        //Handles timeout
        ServiceCallError(this, EventArgs.Empty);
    }
    catch (System.ServiceModel.CommunicationException ex)
    {
        //Handles a number of failures:
        //  Lack of cross-domain policy on target site
        //  Exception thrown by a service
        ServiceCallError(this, EventArgs.Empty);
    }
    catch (System.Exception ex)
    {
        //Handles any other errors here
        ServiceCallError(this, EventArgs.Empty);
    }
};

As far as a general code review, I would recommend that you avoid hard-coding your binding configuration. You can instead declare your endpoint configuration (including reasonable timeouts) in your ServiceReferences.ClientConfig file and new up your channel factory with the endpoint name like this:

new ChannelFactory<IFeedService>("feedServiceEndpoint");

That should make the app more maintainable over time.

I hope this helps.

Jerry

Updated:

Jeff,

Please take a look at this code and the comments within for more explanation of what is happening here. The MakeCall() method is creating a function (delegate) that is being passed to the BeginGet() method. That function is later executed when the service responds.

Please make the changes suggested and set breakpoints at the lines starting with "AsyncCallback asyncCallback" and "var items". You will see the first pass through the debugger merely declares the code (function/delegate) to execute when the service responds and the second pass is the actual processing of that response, starting at the inside of your delegate declaration. This means the outer try/catch will not be in scope when the response of the service call is processed.

public void MakeCall(DateTime lastTime, Dictionary<string, string> context)
{
    try
    {
        AsyncCallback asyncCallBack = delegate(IAsyncResult result)
        {
            try
            {
                var items = ((IFeedService)result.AsyncState).EndGet(result);
                if (ItemsRetrieved != null)
                    ItemsRetrieved(this, new ServiceCallerEventArgs(items));
            }
            catch (Exception ex)
            { 
                //This will catch errors from the service call
            }
        };
        _channel.BeginGet(lastTime, context, asyncCallBack, _channel);
    }
    catch(Exception ex)
    {
        //This will not catch an error coming back from the service.  It will 
        //catch only errors in the act of calling the service asynchronously.

        //The communication with the service and response is handled on a different
        //thread, so this try/catch will be out of scope when that executes.

        //So, at this point in the execution, you have declared a delegate 
        //(actually an anonymous delegate the compiler will turn into a hidden class) 
        //which describes some code that will be executed when the service responds.

        //You can see this in action by setting a breakpoint just inside the delegate
        //at the line starting with "var items".  It will not be hit until the service
        // responds (or doesn't respond in a timeout situation).
    }
}

Jerry

Answer 3

Your ChannelFactory instance is going to be in the "Aborted" state when this timeout occurs. You'll find that the exception is being thrown not when the exception happens in the call, but when you call _channel.Dispose().

You'll need to guard against this exception somehow. The simplest way would be to put a try/catch around the contents of your dispose method, but it would be better if you checked the state of the _channel instance before trying to close/dispose.

This is why you are still getting the exception - it's not being handled here.

Update:

Based on your stack trace, it looks like the framework is trying to execute your callback delegate and is getting an exception there. You responded elsewhere that try/catching the contents of your delegate doesn't solve the issue... what about inner exceptions? What is the full output of TheException.ToString()?

I trolled through the .NET Framework and it is indeed allowed to travel all the way back to the threadpool (at least in the stacktrace you have here) which would cause an unhandled exception and your thread to die. If worse comes to worse, you can always take over the threading... spin up your own thread and inside call the service synchronously, rather than using the builtin async pattern (unless this is a WCF Duplex type of communication, which I don't think it is).

I still think the full exception information (if there is anything more) might be useful.

Answer 4

Jeff, please show us your code when you try to put a try/catch block around the EndGet call. It's hard to believe that doesn't catch an exception, and seeing it will help me believe.

Also, as part of this experiment, get rid of the UnhandledException event handler. I think you're catching this exception as soon as the BrowserHttpRequest realizes there's a 404. I think that without the UnhandledException event, a try/catch around EndGet will catch the exception, if any. I think you're picking up an intermediate state of the exception handling process.

I'd also like to see you put a System.Debugger.Break or something immediately after the EndGet call.

Answer 5

Am I right in thinking that you have designed your ServiceCaller class so that you can call it in a using block?

If yes, then that's the problem. The WCF channel classes have been designed in such a way as to make it extremely difficult to write completely general disposing code that caters for all possible error conditions. If it were simple, then the channel classes themselves would be safe to dispose and you wouldn't need a wrapper class.

The only safe way I have found to date is to encapsulate not the dispose logic, but the entire call sequence, thus:

public static void Invoke<TContract>(ChannelFactory<TContract> factory, Action<TContract> action) where TContract : class {

    var proxy = (IClientChannel) factory.CreateChannel();
    bool success = false;
    try {
        action((TContract) proxy);
        proxy.Close();
        success = true;
    } finally {
        if(!success) {
            proxy.Abort();
        }
    }
}

I know that's a synchronous call, but the principle is the same. Instead of you trying to deduce whether you should call Close or Abort, you determine it in advance based on how far through the call you managed to get before it fell over.

Note - only the actual channel needs this special treatment. You can dispose of the factory any way you like AFAIK.

Answer 6

I've encountered a similar problem before. The main problem is with the way IDisposable is implemented on instances of your proxy/channel. The way I've solved it is shown in the code below, where IDirector is my service contract:

public class ProxyWrapper : IDisposable
{
 private IDirector proxy;
 private ChannelFactory<IDirector> factory;
 int callCount = 0;

 public ProxyWrapper()
 {
  factory = new ChannelFactory<IDirector>();

  proxy = factory.CreateChannel();
 }

 public IDirector Proxy
 {
  get
  {
   if (callCount > 0)
    throw new InvalidOperationException("The proxy can only be accessed once for every ProxyWrapper instance. You must create a new ProxyWrapper instance for each service request.");
   // only allow the proxy/channel to be used for one call.

   callCount++;
   return proxy;
  }
 }

 public void Dispose()
 {
  IClientChannel channel = (IClientChannel)proxy;

  try
  {
   if (channel.State != CommunicationState.Faulted)
   {
    channel.Close();
   }
   else
   {
    channel.Abort();
   }
  }
  catch (CommunicationException)
  {
   channel.Abort();
  }
  catch (TimeoutException)
  {
   channel.Abort();
  }
  catch (Exception)
  {
   channel.Abort();
   throw;
  }
  finally
  {
   channel = null;
   proxy = null;
  }
 }
}

The way to use the above class is as follows:

    public static void Login(string userName, string password)
    {
  using (ProxyWrapper wrapper = new ProxyWrapper())
  {
   currentSession = wrapper.Proxy.Login(userName, password);
  }
    }

Because the ProxyWrapper class implements IDisposable, if we use an instance of the ProxyWrapper class inside a using block, the Dispose() method is guaranteed to be called, even if an exception is thrown. The code in the Dispose() method will handle all cases and states of the proxy/channel. You can then add your error-handling / logging / event delegate code in this method.

Read the following blog entry for more info, and for a more generic version of the code above: http://bloggingabout.net/blogs/erwyn/archive/2006/12/09/WCF-Service-Proxy-Helper.aspx

Answer 7

I think that the problem is in the way you have designed service caller.

The channel is opened when service caller is created. This means that the channel can timeout and there is nothing in your code that can recover from the timeount.

I would move the creation and closing of the channel to the make call method.

You can do a try catch around the begin get and around the contents of the callback.