One of the common practices that you might notice in production code – yes, production code – is what is sometimes known as “swallowing exceptions”. Swallowing an exception is the act of providing a catch clause for a certain exception type and then doing nothing with the exception caught. The following is an example I have seen us ship as part of our production code:
1: public static void TryDeleteDirectory(string folderPath)
2: {
3: if (Directory.Exists(folderPath) == false)
4: {
5: // Nothing to delete
6: return;
7: }
8:
9: try
10: {
11: DirectoryInfo directory = new DirectoryInfo(folderPath);
12: directory.Delete(true);
13: }
14: catch (Exception)
15: {
16: // Do Nothing
17: return;
18: }
19: }
In the code shown above, notice how the catch statement on line 15 is catching the base exception type System.Exception and then doing nothing with it. The method simply returns when it fails due to an “exception”. It does not rethrow since that is not the intention. It is supposed to be called if the caller intends to delete a folder but does not care if the operation fails.
However, there are two fundamental issues with the code shown above:
- The exception is caught and never logged. This can make troubleshooting failures in production very difficult as the code leaves no traces of the exception caught. Very bad.
- The catch statement attempts to catch the more generic base exception, System.Exception, rather than a more specific exception type like System.IO.FileNotFoundException.
Now let’s analyze those two issues in a bit more detail:
- Logging exceptions: As a rule, any catch block should always attempt to log an exception if it is not going to rethrow it. This can be very helpful to anyone trying to troubleshoot a failure at runtime. Swallowing the exception and always returning can potentially mask existing problems that might need to be corrected.
- Catching more specific exceptions: Why should we catch a specific exception if we can catch the more generic System.Exception? One of the reasons you should avoid catching System.Exception is because you might not be ready to deal with the System.Exception-derived type that you might end up catching. For example, if you happen to catch a System.OutOfMemoryException (OOM) exception, treating it the same way as you would treat a System.IO.FileNotFoundException is probably not the smartest thing to do. Doing so will not solve the OOM problem that your application might be experiencing. Ignoring the problem (by swallowing the exception) might be more dangerous. You might be masking an critical issue that can cause your application to become unstable. Even worse, you might be even causing data corruption inside your application if you chose not to deal with the critical problem and simply ignore it. If you can’t deal with the exception, rather than subdue it, it is better to actually rethrow it.
Unfortunately, this bad practice is not only so common, but it seems to be deeply ingrained in the minds of many developers. I had a discussion about this a while back with some developers on a team that we work with. Some of the statements I heard as part of the discussion were: “We must always catch System.Exception on a UI boundary” and “there is no harm in catching System.Exception and ignoring it in the TryXXX functions as we don’t care if it fails no matter what the specific exception is, even if it is OutOfMemoryException”. The interesting part was that the two statements came from a dev manager who claims he has many years of experience coding. Unfortunately, I have witnessed many occurrences of crashes and intermittent issues that that manager has flagged as no-repro because there was no log evidence of the issues… It is sad to see that such ignorant folks still exist in the software development industry since they can produce some of the worst-designed and written code. I see the proof on an almost daily basis…
Swallowing exceptions, especially when done blindly and without putting more thought into why it is being done, is bad and has been written about a lot before. Just Google “swallowing exceptions” and see for yourself.
However, since I know this is still a common practice, I felt it will be helpful to re-emphasize the issue again. Hopefully this will help get better coding practices maintained and alert devs to this issue. I will be covering it again in future posts from a different angle, as well as what is being done in .NET Framework 4.0 to help mitigate this issue.
Finally, I wanted to mention a very helpful resource for getting more information on this issue (and others). The “Framework Design Guidelines: Conventions, Idioms, and Patterns for Reusable .NET Libraries (2nd Edition)” book has a great discussion of this topic with many useful suggestions. You might want to check out section 7.2.2, titled Exception Handling, in particular.
The following are two suggestions extracted from that section:
The first one is:
DO NOT swallow errors by catching nonspecific exceptions, such as System.Exception, System.SystemException, and so on in framework code.
1: try{
2: File.Open(...);
3: }
4: catch(Exception e){ } // swallow “all” exceptions - don’t do this!
A legitimate reason for catching a nonspecific exception is so that you can transfer that exception to another thread. This can happen, for example, when a GUI application transfers an operation to the UI thread, when doing asynchronous programming, using thread pool operations, and so on. Obviously if you are transferring an exception to another thread, you aren’t actually swallowing it.
The second one is:
AVOID swallowing errors by catching nonspecific exceptions, such as System.Exception, System.SystemException, and so on in application code.
There are cases when swallowing errors in applications is acceptable, but such cases are rare.
If you decide to swallow exceptions, you must realize that you don’t know exactly what went wrong, so you generally cannot predict what state might now be inconsistent. By swallowing exceptions, you are making a trade-off that the value of continuing to execute code in this application domain or process exceeds the risk of executing in the face of inconsistencies. Because a security attack might be able to exploit those inconsistencies, your decision here has deep consequences.
The topic of catching and dealing with exceptions is quite interesting, especially when you see the many patterns that are being used in production code. I am sure I will be spending more time writing about that in future posts.