Click here to Skip to main content
15,889,096 members
Articles / Programming Languages / C#
Article

Exception Handling in C# with the "Do Not Catch Exceptions That You Cannot Handle" rule in mind

Rate me:
Please Sign up or sign in to vote.
4.38/5 (44 votes)
5 Jan 20064 min read 299.3K   74   33
Exception handling in C# with the "Do Not Catch Exceptions That You Cannot Handle" rule in mind.

Introduction

Microsoft advices in several documents never to do try { ... } catch (Exception ex) { ... } if you don't rethrow the same exception. This document explains how you can attempt to comply to this .NET Framework design guidelines rule: Do Not Catch Exceptions That You Cannot Handle.

Background

After working with Visual Basic 6 error-handling for several years, .NET exception handling comes as a relief. Although it is a major improvement compared to on error goto statements, it does have some problems. Since there are already plenty of resources out there that explain exception handling, I won't do that in this document. Instead, I'll explain how I attempt to comply to one of the .NET Framework design guidelines rules: Do Not Catch Exceptions That You Cannot Handle.

Or in other words: You should never catch System.Exception or System.SystemException in a catch block because you could inadvertently hide run-time problems like Out Of Memory. Refer to the Improving .NET Application Performance and Scalability document on Patterns & Practices website for more information on this rule. An example of something you should not do:

C#
try {
  intNumber = int.Parse(strNumber);
} catch (Exception ex) {
  Console.WriteLine("Can't convert the string to " + 
                             "a number: " + ex.Message);
}

I agree that most of the times you should let the exception propagate up the call stack. However, this doesn't always work for me. For example, suppose I have an unattended application that copies files from one directory to another (it's a stupid example, I know, I just want to prove my point). I want this application to continue working regardless of what goes wrong during the file-copy operation because I'm going to send an e-mail in the end informing the administrator of all the failures. However, I don't want the application to continue if it throws an OutOfMemoryException or a BadImageFormatException for example. This is the C# code of my little application:

C#
static void Main(string[] args)
{
  string[] strFiles = 
      System.IO.Directory.GetFiles(@"C:\Temp");
  System.Collections.ArrayList objSuccesses = 
               new System.Collections.ArrayList();
  System.Collections.ArrayList objFailures = 
               new System.Collections.ArrayList();
  foreach (string strFile in strFiles) 
  {
    try 
    {
      // We're simply copying a file here. 
      // In a real application, 
      //something more complicated would happen...
      System.IO.File.Copy(strFile, strFile + ".new");
      objSuccesses.Add(strFile);
    } 
    catch (Exception ex) 
    {
      Console.WriteLine("Failed to copy the file: " + 
                                             ex.Message);
      objFailures.Add(strFile);
    }
  }
  // We could send an e-mail here informing an 
  // administrator of all failures...
}

If we follow Microsoft's guidelines then we have to catch only those exceptions that could be triggered by the File.Copy method. If you look in the .NET Framework's Class Library, you'll see that this is UnauthorizedAccessException, ArgumentException, ArgumentNullException, PathTooLongException, DirectoryNotFoundException, FileNotFoundException, IOException and NotSupportedException. If you ask me, catching all these exceptions individually is not doable. All I wanted to know is whether or not a file copy failed or not. It would be sufficient to only capture IOException, ArgumentException and NotSupportedException (because all the other exceptions derive from these three exceptions) but to know this I have to read the documentation again. I really don't want to go through that much trouble to simply copy a file.

The guideline assumes that we know all the exceptions that can be thrown by the task executed. Copying a file is a very simple operation that is properly documented by Microsoft. But what if we're using some third-party tool? The stability of our unattended tool depends on the quality of the documentation of this tool. If this third-party forgets to mention one of the exceptions that their tool throws in the documentation, then our unattended application could stop prematurely.

Also, suppose that we know all the exceptions that can be thrown by this tool and suppose that we write our code to handle all these exceptions individually. What happens if we start using a new version of this tool one day? This new version could have the exact same API, but could be throwing more detailed or other exceptions. We therefore need to compare all the exceptions thrown by this tool with the ones that we capture in our code. Again, not doable...

Also, suppose I wanted to use for example the Save method of the System.Drawing.Image class. This method doesn't even list any exceptions; so how can I possibly handle an exception thrown by this method according to the guideline? This method is supposed to save an image to disk; there are dozens of things that can go wrong.

The feature that is missing in C# according to me is the ability to catch all the exceptions except the critical ones. To me Critical exceptions are for example, Out Of Memory, or AppDomain Is Unloading. That's why I wrote a small static method that tests if the exception thrown is a critical one. If it is, the exception is rethrown (the application could also simply stop gracefully). If it isn't a critical exception, it is logged, swallowed and the application continues. This is the code of my IsCritical method:

C#
public static bool IsCritical(Exception ex) 
{
  if (ex is OutOfMemoryException) return true;
  if (ex is AppDomainUnloadedException) return true;
  if (ex is BadImageFormatException) return true;
  if (ex is CannotUnloadAppDomainException) return true;
  if (ex is ExecutionEngineException) return true;
  if (ex is InvalidProgramException) return true;
  if (ex is System.Threading.ThreadAbortException) 
      return true;
  return false;
}

This is the code of the application while using the IsCritical method:

C#
static void Main(string[] args)
{
  string[] strFiles = 
      System.IO.Directory.GetFiles(@"C:\Temp");
  System.Collections.ArrayList objSuccesses = 
                      new System.Collections.ArrayList();
  System.Collections.ArrayList objFailures = 
                      new System.Collections.ArrayList();
  foreach (string strFile in strFiles) 
  {
    try 
    {
      // We're simply copying a file here. 
      // In a real application, 
      // something more complicated would happen...
      System.IO.File.Copy(strFile, strFile + ".new");
      objSuccesses.Add(strFile);
    } 
    catch (Exception ex) 
    {
      if (IsCritical(ex)) throw;
      Console.WriteLine("Failed to copy the file: " + 
                                           ex.Message);
      objFailures.Add(strFile);
    }
 }
 // We could send an e-mail here informing 
 // an administrator of all failures...
}

If you think that using this IsCritical method is a good idea, please let me know. If you know a reason why it's a bad idea, please do let me know.

License

This article has no explicit license attached to it but may contain usage terms in the article text or the download files themselves. If in doubt please contact the author via the discussion board below.

A list of licenses authors might use can be found here


Written By
Team Leader
Belgium Belgium
I am a developer spending most of my time in C#, .NET 2.0 and Sql Server 2005. I am working for a Belgium company called Adam Software developing Asset Management Software. More information about my company and our software can be found at http://www.adamsoftware.net

Comments and Discussions

 
GeneralMy opinion Pin
popsickle7-Jul-04 12:34
popsickle7-Jul-04 12:34 
GeneralRe: My opinion Pin
Michael Vanhoutte8-Jul-04 10:59
Michael Vanhoutte8-Jul-04 10:59 
GeneralRe: My opinion Pin
popsickle11-Jul-04 8:41
popsickle11-Jul-04 8:41 
GeneralSome feedback Pin
jfos7-Jul-04 5:25
jfos7-Jul-04 5:25 
GeneralRe: Some feedback Pin
Michael Vanhoutte7-Jul-04 9:45
Michael Vanhoutte7-Jul-04 9:45 
GeneralRe: Some feedback Pin
jfos7-Jul-04 17:24
jfos7-Jul-04 17:24 
GeneralRe: Some feedback Pin
Super Lloyd8-Jul-04 1:27
Super Lloyd8-Jul-04 1:27 
GeneralRe: Some feedback Pin
Michael Vanhoutte8-Jul-04 10:57
Michael Vanhoutte8-Jul-04 10:57 
I don't think that I fully understand your application-domain-solution. I understand that having your main loop in one application domain and having your maintenance tasks in separate application domains solves some problems: If you maintenance task crashes, than it will only be that application domain that crashes and not the main loop. But how does that solve the exception-dilema? Not all 'critical' exceptions are so bad that they cause your application domain to crash. Some (like 'out of memory') are also very important but will not (to my knowledge, I might be wrong) stop your application domain from running. So how you deal with that?

But, let's assume for now that this application domain-solution solves my maintenance-task problem. I doesn't solve the Application.Login-problem that I described earlier.
I personally don't think that the solution Jim (jfos) suggested using pdb-files is feasible for us because we're going to obfuscate our library. I don't think that the pdb-files will still work.
Jim also stated in his latest response that I should let exceptions percolate through so that they developers using my library can decide what to do with it. I see three ways of doing this:

Solution 1
----------
Either my code does this:
public class Application {
  void Login(string username, string password) {
    ConnectToDatabase();
    VerifyLicenseKey();
    ConnectToDotNetRemotingServer();
    ...
  }
}


in which case a user might receive ApplicationExceptions that I trew because the license key was invalid for example. But he might also receive ADO.NET exceptions because I had problems with the database or he might get .NET Remoting Exceptions... The disadvantages of this according to me are:
1. Documenting all the exceptions that this method could throw is A LOT of work for me to do. I don't only have to document all exceptions thrown by Application.Login but also every exception that can possibly be thrown by any of the methods that Application.Login calls!
2. Trapping all these exceptions is also a lot of work for a user of my library. Most of the time, as a developer, they don't even want to know why it failed because they can't do much about it. They just want to know if the login was successful or not. I personally feel that I can't expect from my customers to catch 5 or even 10 different exceptions simply to check if a login was successful.
3. The advantage of using a business-layer (like my library is) is that you hide most of the complexity from users using your business-layer. I've decided to use .NET Remoting in this version, but if I run into problems with it later on, I might decide to use Memory Mapped Files in the next version of my library. Technically, this shouldn't have any impact on my users because they only call Application.Login; they don't even know that I use .NET Remoting. But the problem is that I used to let .NET
Remoting exceptions bubble all the way up so they had to catch these exceptions! If they would start using my newest version, their exception handler suddenly becomes useless.

Solution 2
----------
Another thing that I could do is this:
public class Application {
  void Login(string username, string password) {
    try {
      ConnectToDatabase();
      VerifyLicenseKey();
      ConnectToDotNetRemotingServer();
      ...
    } catch (Exception ex) {
      throw new MyApplicationException("Unable to login", ex);
    }
  }
}


This would be the easiest solution for users of my library. They just have to catch MyApplicationException to know if they've logged in or not. But this also has two disadvantages:

1. I hide some of the details of the exception. Using my previous code, a user might receive a 'can't connect to database'-exception which is cool because they immediately know what's wrong. They can also write your code to handle exceptions in a specific way.
2. The biggest danger with this way of working is that it hides important exceptions again. So, I can't do that.

Solution 3
----------
Another thing I could do is:
public class Application {
  void Login(string username, string password) {
    try {
      ConnectToDatabase();
      VerifyLicenseKey();
      ConnectToDotNetRemotingServer();
      ...
    } catch (Exception ex) {
      if IsCritical(ex) throw;
      throw new MyApplicationException("Unable to login", ex);
    }
  }
}


This has the advantage that any critical exception simply walks up the stack, but any other exception is wrapped in a MyApplicationException. Users of my library can now safely catch MyApplicationException not having to worry that they would catch an 'out of memory'-exception. But I agree, what do we do if a new critical exception appears?
GeneralRe: Some feedback Pin
Toughfella6-Jan-06 10:52
Toughfella6-Jan-06 10:52 

General General    News News    Suggestion Suggestion    Question Question    Bug Bug    Answer Answer    Joke Joke    Praise Praise    Rant Rant    Admin Admin   

Use Ctrl+Left/Right to switch messages, Ctrl+Up/Down to switch threads, Ctrl+Shift+Left/Right to switch pages.