Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I typically agree with most code analysis warnings, and I try to adhere to them. However, I'm having a harder time with this one:

CA1031: Do not catch general exception types

I understand the rationale for this rule. But, in practice, if I want to take the same action regardless of the exception thrown, why would I handle each one specifically? Furthermore, if I handle specific exceptions, what if the code I'm calling changes to throw a new exception in the future? Now I have to change my code to handle that new exception. Whereas if I simply caught Exception my code doesn't have to change.

For example, if Foo calls Bar, and Foo needs to stop processing regardless of the type of exception thrown by Bar, is there any advantage in being specific about the type of exception I'm catching?

Maybe a better example:

public void Foo()
{
    // Some logic here.
    LogUtility.Log("some message");
}

public static void Log()
{
    try
    {
        // Actual logging here.
    }
    catch (Exception ex)
    {
        // Eat it. Logging failures shouldn't stop us from processing.
    }
}

If you don't catch a general exception here, then you have to catch every type of exception possible. Patrick has a good point that OutOfMemoryException shouldn't be dealt with this way. So what if I want to ignore every exception but OutOfMemoryException?

share|improve this question
9  
What about OutOfMemoryException? Same handling code as everything else? –  Patrick Sep 9 '12 at 15:23
    
@Patrick Good point. I added a new example in my question to cover your question. –  Bob Horn Sep 9 '12 at 16:23
1  
Catching general exceptions is about as bad as making general statements about what you everyone should do. There is generally no one-size-fits-all approach to everything. –  Phong Sep 13 '12 at 17:40

9 Answers 9

up vote 18 down vote accepted

These rules are generally a good idea and thus should be followed.

But remember these are generic rules. They don't cover all situations. They cover the most common situations. If you have a specific situation and you can make the argument that your technique is better (and you should be able to write a comment in the code to articulate your argument for doing so) then do so (and then get it peer reviewed).

On the counter side of the argument.

I don't see your example above as a good situation for doing so. If the logging system is failing (presumably logging some other exception) then I probably do not want the application to continue. Exit and print the exception to the output so the user can see what happened.

share|improve this answer
    
Concur. At the very least, now that regular logging is offline make some kind of provision that'll give some kind of debugging output, to STDERR if nothing else. –  Shadur Sep 10 '12 at 4:20

Yes, catching general exceptions is a bad thing. An exception usually means that the program cannot do what you asked it to do.

There are a few types of exceptions that you could handle:

  • Fatal exceptions: out of memory, stack overflow, etc. Some supernatural force just messed up your universe and the process is already dying. You cannot make the situation better so just give up
  • Exception thrown because bugs in the code that you are using: Don't try to handle them but rather fix the source of the problem. Don't use exceptions for flow control
  • Exceptional situations: Only handle exception in these cases. Here we can include: network cable unplugged, internet connection stopped working, missing permissions, etc.

Oh, and as a general rule: if you don't know what to do with an exception if you catch it, it is better to just fail fast (pass the exception to caller and let it handle it)

share|improve this answer
1  
What about the specific case in the question? If there is a bug in the logging code, it's probably okay to ignore the exception, because the code that actually matters shouldn't be affected by that. –  svick Sep 9 '12 at 17:52
3  
Why not fix the bug in the logging code instead? –  Victor Hurdugaci Sep 9 '12 at 21:30
2  
Victor, it's probably not a bug. If the disk the log lives on runs out of space, is that a bug in the logging code? –  Carson63000 Sep 10 '12 at 0:05
3  
@Carson63000 - well, yes. The logs aren't being written, therefore: bug. Implement fixed size rotating logs. –  detly Sep 10 '12 at 0:36
2  
This is the best answer imo, but it is missing one crucial aspect: security. There are some exceptions that occur as a result of the bad guys trying to f-up your program. If an exception gets thrown that you are unable to handle, then you cannot necessarily trust that code anymore. Nobody likes to hear that they wrote the code that let the bad guys in. –  Stargazer712 Sep 10 '12 at 2:04

The topmost outer loop should have one of these to print all it can, and then die a horrific, violent and NOISY death (as this shouldn't happen and someone needs to hear).

Otherwise you should generally be very careful as you most likely have not anticipated everything that could happen at this location, and hence will most likely not treat it correctly. Be as specific as possible so you only catch those you know will happen, and let those not seen before bubble up to the above mentioned noisy death.

share|improve this answer
    
I agree with this in theory. But what about my logging example above? What if you simple want to ignore logging exceptions and continue? Should you need to catch every exception that could be thrown and handle each of those while letting the more serious exception bubble-up? –  Bob Horn Sep 9 '12 at 16:45
5  
@BobHorn: Two ways of looking at that. Yes, you can say that logging isn't particularly important and if it fails then it shouldn't bring your app to a halt. And that's a fair enough point. But, what if your application fails to log for five months and you had no idea? I've seen this happen. And then we needed the logs. Disaster. I would suggest doing everything you can think of to stop logging failing is better than ignoring it when it does. (But you're not going to get much of a consensus on that.) –  pdr Sep 9 '12 at 17:12
    
@pdr In my logging example, I would typically send an email alert. Or, we have systems in place to monitor logs, and if a log isn't being actively populated, our support team would get an alert. So we'd become aware of the logging issue in other ways. –  Bob Horn Sep 9 '12 at 17:23
    
@BobHorn your logging example is rather contrived - most logging frameworks I know of, go to great length to not throw any exceptions and would not be invoked like that (as it would make any "log statement happened at line X in file Y" useless) –  user1249 Sep 9 '12 at 17:44
    
@pdr I've raised the question at the logback list (Java) on how to make the logging framework halt the application if logging configuration fails, simply because it is SO important we have the logs, and any silent failure is unacceptable. A good solution is still pending. –  user1249 Sep 9 '12 at 17:46

It's not that it's bad, it's just that specific catches is better. When you're specific, it means that you actually understand, more concretely, what your application is doing, and have more control over it. In general, if you come upon a situation where you just catch an Exception, log it and continue, there's probably some bad things that are going on anyway. If you're specifically catching the exceptions that you know a code block or method can throw, then there's a higher likelihood you can actually recover instead of just logging and hoping for the best.

share|improve this answer

The two possibilities are not mutually exclusive.

In an ideal situation, you would catch all possible types of exception your method could generate, handle them on a per-exception basis, and in the end add a general catch clause to catch any future or unknown exceptions. This way you get the best of both worlds.

try
{
    this.Foo();
}
catch (BarException ex)
{
    Console.WriteLine("Foo has barred!");
}
catch (BazException ex)
{
    Console.WriteLine("Foo has bazzed!");
}
catch (Exception ex)
{
    Console.WriteLine("Unknown exception on Foo!");
    throw;
}

Keep in mind that in order to catch the more specific exceptions, you must put them first.

Edit: For reasons stated in the comments, added a rethrow in the last catch.

share|improve this answer
2  
Wait. Why would you ever want to log an unknown exception to console and carry on? If it's an exception that you didn't even know could be thrown by the code, you're probably talking about a system failure. Carrying on in this scenario is probably a really bad idea. –  pdr Sep 9 '12 at 15:43
    
@pdr Surely you realize it's a contrived example. The point was to demostrate catching of both specific and general exceptions, not how to handle them. –  Rotem Sep 9 '12 at 15:48
    
@Rotem I think you have a good answer here. But pdr has a point. The code, as is, makes it look like catching and continuing is fine. Some beginner, seeing this example, might think that's a good approach. –  Bob Horn Sep 9 '12 at 16:14
    
Contrived or not, it makes your answer wrong. As soon as you start catching exceptions like Out Of Memory and Disk Full and just swallowing them, you're proving why catching general exceptions is in fact bad. –  pdr Sep 9 '12 at 16:16
    
If you catch a general exception only to log it, then you should rethrow it after logging. –  Victor Hurdugaci Sep 9 '12 at 16:20

Pokemon exception handling (gotta catch em all!) is certainly not always bad. When your exposing a method to a client, especially an end user its often better to catch anything and everything rather than have your application crash and burn.

Generally though they should be avoided where you can. Unless you can take specific action based on the type of the exception its better not to handle it and allow the exception to bubble up rather then swallow the exception or handle it incorrectly.

Have a look at this SO answer for more reading.

share|improve this answer
    
This situation (when a developer must be a Pokemon Trainer) appears when you are making an entire software yourself: you made the layers, database connections and user's GUI. Your app must recover from situations like connectivity lost, user´s folder deleted, data corrupted, etc. End Users don't like apps that crash and burn. They like apps that can work on a burning exploding crashing computer! –  Broken_Window Mar 3 at 16:00
    
I hadn't thought of this until now, but in Pokemon, it's generally assumed that by 'catch them all' you want exactly one of each, and have to handle catching it specifically, which is the opposite of the common use of this phrase among programmers. –  Magus Apr 3 at 19:45
    
@Magus: With a method like LoadDocument(), it's essentially impossible to identify all the things that might go wrong, but 99% of exceptions that could be thrown will simply mean "It was not possible to interpret the contents of a file with the given name as a document; deal with it." If someone tries to open something that isn't a valid document file, that shouldn't crash the application and kill any other open documents. Pokemon error handling in such cases is ugly, but I don't know any good alternatives. –  supercat Jul 9 at 20:53
    
@supercat: I was just making a linguistic point. However, I don't think invalid file contents sound like something that should throw more than one kind of exception. –  Magus Jul 9 at 22:56
    
@Magus: It can throw all sorts of exceptions--that's the problem. It's often very difficult to anticipate all the kinds of exceptions that might get thrown as a consequence of invalid data in a file. If one doesn't use PokeMon handling, one risks having an application die because e.g. the file one was loading contained an exceptionally-long string of digits in a place which required a decimal-formatted 32-bit integer, and a numerical overflow occurred in the parsing logic. –  supercat Jul 9 at 23:02

Catching general exception is bad because it leaves your program in an undefined state. You don't know where stuff went wrong so you don't know what your program has actually done or hasn't done.

Where I would allow catching all is when closing a program. As long as you can clean it up alright. Nothing as annoying as a program you close which just throws an error dialog that does nothing but sit there, not going away and preventing your computer from closing down.

In a distributed environment your log method could backfire: catching a general exception could mean your program still holds a lock on the log-file preventing other users from making logs.

share|improve this answer

You should catch general exceptions at the top level of every process, and handle it by reporting the bug as well as possible and then terminating the process.

You should not catch general exceptions and try to continue execution.

share|improve this answer

I understand the rationale for this rule. But, in practice, if I want to take the same action regardless of the exception thrown, why would I handle each one specifically?

As others have said, it's really difficult (if not impossible) to imagine some action that you'd want to take regardless of the exception thrown. Just one example are situations where the program's state has been corrupted and any further processing can lead to problems (this is the rationale behind Environment.FailFast).

Furthermore, if I handle specific exceptions, what if the code I'm calling changes to throw a new exception in the future? Now I have to change my code to handle that new exception. Whereas if I simply caught Exception my code doesn't have to change.

For hobby code it's just fine to catch Exception, but for professional-grade code introducing a new exception type should be treated with the same respect as a change in the method signature, i.e. be considered a breaking change. If you subscribe to this point of view then it's immediately obvious that going back to change (or verify) the client code is the only correct course of action.

For example, if Foo calls Bar, and Foo needs to stop processing regardless of the type of exception thrown by Bar, is there any advantage in being specific about the type of exception I'm catching?

Sure, because you will not be catching just exceptions thrown by Bar. There will also be exceptions that Bar's clients or even the runtime might throw during the time that Bar is on the call stack. A well-written Bar should define its own exception type if necessary so that callers can specifically catch the exceptions emitted by itself.

So what if I want to ignore every exception but OutOfMemoryException?

IMHO this is the wrong way to think about exception handling. You should be operating on whitelists (catch exception types A and B), not on blacklists (catch all exceptions apart from X).

share|improve this answer

Your Answer

 
discard

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.