Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am new to WCF and need to work with another programmer's code.

I am unsure of the way the WCF service client is used here :

private void barButtonDocuments_ItemClick(object sender, ItemClickEventArgs e)
{
    try
    {
        MyServiceClient myServiceClient = new MyServiceClient();

        try
        {
            documents = myServiceClient.GetDocuments();
            // More code that isn't useful including here ...
        }
        finally
        {
            try
            {
                myServiceClient.Close();
            }
            catch
            {
            }
        }
    }
    catch (FaultException<ServiceErrorDetails> error)
    {
        MessageBox.Show(error.Detail.Message, "Erreur", MessageBoxButtons.OK, MessageBoxIcon.Error);
    }
}

What do you think of that code ?

Is it good practice to close the proxy in a finally block, and doing so in an additionnal try, catch block ?

I personnally don't like the empty catch block because it's like hiding potentially useful exceptions when trying to close the service client.

Any better way to handle a WCF service client ?

share|improve this question
    
You may be interested in this WCF approach I asked about here: codereview.stackexchange.com/q/41692/1714 –  makerofthings7 Feb 14 at 21:41
    
Any final solution with full source code sample application ? IMHO, better samples for minimize learning curve are real applications with full source code and good patterns –  Kiquenet Sep 25 at 8:23

3 Answers 3

up vote 8 down vote accepted

Although paritosh's answer is very helpful, there is additional useful information to take into account when using a service client, as found in this MSDN article :

It is recommended that a calling application open the channel, use it, and close the channel inside one try block.

Let's look at the original try/catch block that was used for the solely purpose of closing the channel :

    try
    {
        myServiceClient.Close();
    }
    catch
    {
    }

This is useless, as stated in the article :

Datagram channels never fault even if exceptions occur when they are closed.

About handling exceptions, MSDN suggests that no unexpected exceptions (like OutOfMemoryException, ArgumentNullException or InvalidOperationException) should be caught when using a service client, as stated in this MSDN article :

Exceptions that are thrown from communication methods on a Windows Communication Foundation (WCF) client are either expected or unexpected. Unexpected exceptions include catastrophic failures like OutOfMemoryException and programming errors like ArgumentNullException or InvalidOperationException. Typically there is no useful way to handle unexpected errors, so typically you should not catch them when calling a WCF client communication method.

Recommended exceptions to catch while using a service client are TimeoutException and any other exception that derives from CommunicationException.

As the article states :

One way to handle such errors is to abort the client and report the communication failure.

In addition, as this particular project handles it's own FaultException (of type ServiceErrorDetails), the recommended method to catch exceptions while using this service client would be to first catch any TimeoutException, then any FaultException and finally any CommunicationException.

The rewritten code would then look like this :

MyServiceClient myServiceClient = new MyServiceClient();

try
{
    documents = myServiceClient.GetDocuments();
    // More code that isn't useful including here ...
    myServiceClient.Close();
}
catch (TimeoutException exception)
{
    MessageBox.Show(exception.Message, "Timeout error", MessageBoxButtons.OK, MessageBoxIcon.Error);
    myServiceClient.Abort();
}
catch (FaultException<ServiceErrorDetails> error)
{
    MessageBox.Show(error.Detail.Message, "Service error", MessageBoxButtons.OK, MessageBoxIcon.Error);
    myServiceClient.Abort();
}
catch (CommunicationException exception)
{
    MessageBox.Show(exception.Message, "Communication error", MessageBoxButtons.OK, MessageBoxIcon.Error);
    myServiceClient.Abort();
}
share|improve this answer

Use Close for always for clean path of execution of your code.Use Abort in case of faulted channel or say you code throws an error. As you might know if you call close on faulted channel it will throw another error, close do a graceful shutdown of channel whereas abort do instant shut down of the channel . here is an snippet of the same:

MyServiceClient myServiceClient = new MyServiceClient();

try
{
    documents = myServiceClient.GetDocuments();
    // More code that isn't useful including here ...
    myServiceClient.Close();
}
catch(Exception ex)
{
    myServiceClient.Abort();
}
share|improve this answer
1  
Thanks for your answer. I didn't know about Abort(), very useful in case something's wrong with the service code. Now, I can be sure that the channel is shut down. –  user19024 Nov 7 '12 at 16:57

Here's a gizmo I wrote that handles WCF errors gracefully for individual calls that can be called as such:

return new MyServiceClient().Execute(client => client.DoSomething(parameters));

The extension method:

/// <summary>
/// Executes a method on the specified WCF client.
/// </summary>
/// <typeparam name="T">The type of the WCF client.</typeparam>
/// <typeparam name="TU">The return type of the method.</typeparam>
/// <param name="client">The WCF client.</param>
/// <param name="action">The method to execute.</param>
/// <returns>A value from the executed method.</returns>
/// <exception cref="CommunicationException">A WCF communication exception occurred.</exception>
/// <exception cref="TimeoutException">A WCF timeout exception occurred.</exception>
/// <exception cref="Exception">Another exception type occurred.</exception>
public static TU Execute<T, TU>(this T client, Func<T, TU> action) where T : class, ICommunicationObject
{
    if ((client == null) || (action == null))
    {
        return default(TU);
    }

    try
    {
        return action(client);
    }
    catch (CommunicationException)
    {
        client.Abort();
        throw;
    }
    catch (TimeoutException)
    {
        client.Abort();
        throw;
    }
    catch
    {
        if (client.State == CommunicationState.Faulted)
        {
            client.Abort();
        }

        throw;
    }
    finally
    {
        try
        {
            if (client.State != CommunicationState.Faulted)
            {
                client.Close();
            }
        }
        catch
        {
            client.Abort();
        }
    }
}
share|improve this answer
1  
I've been looking for a generic handler like this!!! Thanks! –  Slick86 Jun 23 at 23:23

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.