Exception Handling in a try/finally block
However I recently came across a problem I put into a system. I did not notice the problem until I was going through the exception logs and discovered that they were not as useful as they could have been.
I had written some code that was a client to a WCF service and if there was a problem with the service then the intention was to log the error for the main program to carry on – in other words the WCF service implemented something that was nice to have rather than must have. However it was important that we record the correct error.
The code in question looked a bit like this
public DataObject GetData(InputObject param) { DataObject result = null; using (ServiceClient client = new ServiceClient()) { try { client.Open(); result = client.GetDataFromService(param); } finally { client.Close(); } } return result; }
If an error occurred in the GetDataFromService() method and it throws an exception then we want to record that exception, however because the Close() method is called from the finally block if it throws an exception then it will mask the exception from GetDataFromService(). This is exactly what did happen and my log was full of CommunicationException exceptions with the message,
“The communication object, System.ServiceModel.Channels.ServiceChannel, cannot be used for communication because it is in the Faulted state.”
It is interesting to note that I would have got that error message even if I had not explicitly coded the close() call in the finally block, as the using statement itself internally does exactly the same thing. It appears to have tripped a number of people up. Microsoft have even produced an example project to show how to deal with this problem. The solution is to not put the ServiceClient in a using statement looks a bit like this
try { ... double result = client.Add(value1, value2); ... client.Close(); } catch (TimeoutException exception) { Console.WriteLine("Got {0}", exception.GetType()); client.Abort(); } catch (CommunicationException exception) { Console.WriteLine("Got {0}", exception.GetType()); client.Abort(); }
Now I know that this will work but I' am a bit unhappy about it because
- I think that everything that implements IDisposable should go in a using block, and making exceptions to that rule just makes me feel uneasy.
- It requires knowledge about which exceptions are “expected” and which are “unexpected”
- Its specific to calling WCF services and does not address the wider issues of things that are called from a Dispose() method in a using statement throwing an exception masking other exceptions. This could happen with any kind of IDisposable object not just WCF.
he method I used was as follows. It is verbose but it does not require any specific knowledge of “expected” exceptions, the using statement is retained and it would work for any IDisposable object.
public DataObject GetData(InputObject param) { DataObject result = null; using (ServiceClient client = new ServiceClient()) { // this is the excpetion we will throw System.Exception exception = null; try { client.Open(); result = client.GetDataFromService(param); } catch (System.Exception ex) { exception = ex; } finally { try { client.Close(); } catch (System.Exception ex) { // only record the exception in the log // if there’s no exception in the try block if (exception == null) { exception = ex; } } } if (exception != null) { throw exception; } } return result; }