You might be swallowing exceptions if:
- Your method returns a bool indicating success or failure
- Your method returns an integer code of some sort
- Your method has a try/catch block
- Your method has an out parameter
- Your method has a ref parameter
Overview
This post is my view of best practices for error handling in Microsoft’s .NET.
What is Swallowing an Exception?
Swallowing an exception is when you catch an exception that is thrown rather than letting it bubble up the stack when you don’t really know how to correct the situation.
What Pain Comes from Swallowed Exceptions?
The biggest pain that comes from swallowing exceptions it that it makes finding bugs VERY hard to find. Last summer, as a result of a swallowed exception it took me over one week to track down a serious bug! (My objective on my own code is 15 minutes from the time I’m told about a bug until it’s fixed… if it goes beyond one hour I start to get upset.)
What can happen is that you catch an exception and take what you think is a corrective action. However, down the line some other code was expecting it to work differently to the exception gets thrown in that code rather than in the code that originally had the exception.
Here is another example. A user reported a bug for electronic delivery software. Evey time they would try to use it they would get a message saying, “You are not connected to the internet.” But they were… So lets look at some code… here was the original code:
- public bool IsConnectedToInternetOriginal
- {
- get
- {
- try
- {
- // Makes a call but PingOriginal return nothing…
- PingService.PingOriginal();
- return true;
- }
- catch (Exception) { return false; }
- }
- }
So this violated some of the guidance you’ll see below “DO NOT use exceptions for the normal flow of control, if possible.“ So this is an excellent example of swallowing an exception.
Here is the modified code:
- public bool IsConnectedToInternetRefactored
- {
- // Ping simply returns true. No try/catch here.
- get { return PingService.Ping(); }
- }
Now in this code the real exception was thrown, caught at the very top level of the program, and it was easier to track down that the IT organization at the user’s company was redirecting the service call to a web page that would ask for credentials. But the user WAS connected to the internet.
Conversion to Errors
When you catch an exception but then convert it to an Error so that it can be displayed to the user you are not really swallowing an exception. So a try/catch bock for the purposes of converting to Errors (EntityErrors in our code base) is not swallowing an exception… UNLESS the code above yours does not properly handle those errors and in effect prevent further execution until the problem is corrected.
The Strategy of High Quality Code
Do not let the user make mistakes.
I think its really that simple. Your code should not allow users to do things that are wrong. To allow your code to converge quickly, you will likely have many throw statements. These let you find the problems quickly by finding them right when the problem first occurs. These exceptions are for the developers to find the problems – they are not really for the user to see in most cases.
I say all this because a number of years ago I was in a conversation with a Microsoft consultant about try/catch blocks. After reviewing a large number of code bases I noticed a pattern – “Code I considered high quality had very few try’/catch bocks.” The consultant agreed, but could not describe why – neither could I.
That was 2005. Today I understand with great clarity why that is the case. If you have a try/catch block, then you let the user make a mistake (There are some exceptions to this statement… namely you are using someone else’s code and something outside your control has happened). The simple example of preventing the mistakes that we are all familiar with is showing error messages at the bottom of a form and disabling the OK button until the user corrects those mistakes. If you do that, then you don’t need to try/catch. Instead you have a property – example: IsValid – that is true and will enable the button when all the information is correct. No try/catch needed.
As a result, now I view try/catch blocks as a form of “Code Smell”.
Error Codes are Out, Exceptions are In
Many, many years ago the prevailing error handling pattern was to return an integer error code. Everyone needed to know and have access to the list of error codes so they could then figure out what to do.
While the exception approach was not new, Microsoft did embrace the exception handling pattern in .NET. This approach allowed you to use the return value for returning the resulting work of your method. Exceptions were now handled on a different channel, so to speak.
A bool or an integer are pretty meaningless. I need a translator so I need access to your error codes with the old style.
- public static bool BadDivide(int a, int b, ref int c)
- {
- try { c = a / b; }
- catch (DivideByZeroException) { return false; }
- return true;
- }
With the new style, I will get an exception – and usually in my own language.
- public static double GoodDivide(int a, int b)
- {
- return a / b;
- }
An Example
The Story
A key feature was the ability to import data. In order for this to happen we need to parse the file.
Original State
The original code would try to parse the file and if it ran into an issue, it would popup an error message (in the middle of the read method). The user would press Ok, and then be presented with an empty import dialog box.
try/catch Removed
I didn’t like the MessageBox.Show(…) in the middle of a read method, so I changed it to remove the try/catch block and thus the message dialog. So now the application crashes because there is an exception that’s not caught.
Move try/catch Block?
So one option is to move the try/catch block up one level. Now we’ll get the exception in the presentation tier and show the message there. But let’s look at what happens there. The user gets a message stating, “We don’t know how to read your file.” If I’m the user, now I’m wondering, “Why? What’s in my file?” We couldn’t read it, so we don’t show them anything… If I’m a user, now I have to go to another application such as NotePad and open up my file to see what the problem might be – Do I need tab delimiters? Did I copy the wrong file? etc.
Final State
What if I showed the user the file as well as my attempt to parse it. If my parsing was unsuccessful, disable the Ok button and show an error message, “We are unable to parse your file because…”. Now the user does not need to open another application – they can see, for example, that we have tried to open a binary file and not a text file. So now I could get by with two properties, IsParsable and ParsingErrorMessage. I might have a try/catch block in the IsParsable to catch for issues with permissions in reading the file, etc. But I don’t need a dialog box to show the user.
Framework Design Guidelines Excerpts
Here are some of the statements from the book, Framework Design Guidelines by Krzysztof Cwalina and Brad Abrams. They use the conventions of Do, Consider, Avoid, and Do Not.
- DO NOT return error codes.
- DO report execution failures by throwing exceptions.
- DO NOT use exceptions for the normal flow of control, if possible.
- DO document all exceptions thrown by publicly callable members because of a violation of the member contract (rather than a system failure) and treat them as part of your contract.
- DO NOT have public members that can either throw or not based on some option.
- DO NOT have public members that return exceptions as a return value or an out parameter.
- DO NOT throw exceptions from exception filter (catch) blocks.
- AVOID explicitly throwing exceptions from finally blocks. Implicitly thrown messages resulting from calling methods that throw are acceptable.
- DO NOT create new exception types to communicate usage errors. Throw one of the existing Framework exceptions instead.
- DO NOT create and throw new exceptions just to have your team’s exception.
- DO throw the most specific (the most derived) exception that makes sense. (For example, throw ArumentNullException and not its base type, ArgumentException, if a null argument is passed.
- DO provide a rich and meaningful message text targeted at the developer when throwing an exception.
- DO ensure that exception messages are grammatically correct.
- DO ensure that each sentence of the message text ends with a period.
- AVOID question marks and exclamation points in exception messages.
- DO NOT disclose security-sensitive information in exception messages without demanding appropriate permissions.
- CONSIDER localizing the exception messages thrown by your components if you expect your components to be used by developers speaking different languages.
- DO NOT swallow errors by catching nonspecific exceptions such as System.Exception…
- CONSIDER catching a specific exception when you understand why it was thrown in a given context and can respond to the failure programmatically.
- DO NOT overcatch. Exceptions should often be allowed to propagate up the call stack.
- DO use try-finally and avoid using try-catch for cleanup code.
- DO prefer using an empty throw when catching and rethrowing an exception. This is the best way to preserve the exception call stack.
The list goes on… I have the book in my office if anyone would like to read this section, or hopefully there is a copy of this book in other locations. Chapter 7 Exceptions starts on page 211 and ends on page 243.
Summary
So in that example, I believe we can prevent the user from making any mistakes and can be far more transparent about what is going on.
Leave a Reply