Is catching general exceptions really a bad thing?

Question

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?

Answer 1

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.

Answer 2

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)

Answer 3

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.

Answer 4

It's not that it's bad, it's just that specific catches are 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.

Answer 5

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.

Answer 6

I have been pondering the same recently, and my tentative conclusion is that the mere question arises because the .NET Exception hierarchy is severely messed up.

Take, for example, the lowly ArgumentNullException which might be a reasonable candidate for an exception you don't want to catch, because it tends to indicate a bug in the code rather than a legitimate runtime error. Ah yes. So does NullReferenceException, except that NullReferenceException does derive from SystemException directly, so there's no bucket you can place all "logical errors" into to catch (or not catch).

Then there is the IMNSHO major botch of having SEHException derive (via ExternalException) SystemException and thus make it a "normal" SystemException When you get an SEHException, you want to write a dump and terminate as fast as you can -- and starting with .NET 4 at least some SEH exceptions are considered Corrupted State Exceptions that will not be caught. A good thing, and a fact that makes the CA1031 rule even more useless, because now your lazy catch (Exception) will not catch these worst types anyway.

Then, it seems that the other Framework stuff rather inconsistently derives either Exception directly or via SystemException, making any attempt at grouping catch-clauses by something like severity moot.

There's a piece by Mr. Lippert of C# fame, called Vexing Exception, where he lays out some useful categorization of exceptions: You could argue you only want to catch "exogenous" ones, except ... C# the language and the design of the .NET framework exceptions make it impossible to "catch only exogenous ones" in any succinct manner. _{(and, e.g, an OutOfMemoryException may very well be a totally normal recoverable error for an API that has to allocate buffers that are somehow large)}

Bottom line for me is, that the way C# catch blocks work and the way the Framework exception hierarchy is designed, rule CA1031 is beyond totally useless. It pretends to help with the root issue of "do not swallow exceptions" but swallowing exceptions has zero to do with what you catch, but with what you then do:

There are at least 4 ways to legitimately handle a caught Exception, and CA1031 only seems to superficially handle one of them (namely the re-throw case).

---

As a side note, there's a C# 6 feature called Exception Filters that will make CA1031 slightly more valid again since then you will be able to properly, properly, properly filter the exceptions you want to catch, and there's less reason to write an unfiltered catch (Exception).

Answer 7

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.

Answer 8

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.

Answer 9

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).

Answer 10

Maybe. There are exceptions to every rule, and no rule should be followed uncritically. You example might conceivably be one of the instances where it makes sense to swallow all exceptions. For example if you want to add tracing to a critical production system, and you want to make absolutely sure that you changes don't disrupt the main task of the app.

However, you should think carefully about possible reasons for failure before you decide to silently ignore them all. For example, what if the reason for the exception is:

• a programming error in the logging method which causes it to always throw a particular exception
• an invalid path to the logging file in the configuration
• the disk is full

Don't you want to be notified immediately that this problem is present, so you can fix it? Swallowing the exception means you never know something went wrong.

Some problems (like the disk is full) might also cause other parts of the application to fail - but this failure is not logged now, so you never know!

Answer 11

I would like to address this from a logical perspective rather than a technical one,

Now I have to change my code to handle that new exception.

Well, someone would have to handle it. That is the idea. Writers of library code would be prudent about adding new exception types though because it is likely to break clients, you should not encounter this very often.

Your question is basically "What if I don't care what went wrong? Do I really have to go through the hassle of finding out what it was?"

Here's the beauty part: no you do not.

"So, can I just look the other way and have whatever nasty thing pops up swept under the carpet automatically and be done with it?"

No, that is not how it works either.

The point is, the collection of possible exceptions is always larger than the collection you expect and are interested in in the context of your local little problem. You handle those you anticipate and if something unexpected happens, you leave it to the higher level handlers rather than swallowing it. If you do not care about an exception you did not expect, you bet someone up the call stack will and it would be sabotage to kill an exception before it reaches its handler.

"But... but... then one of those other exceptions I do not care about could cause my task to fail!"

Yes. But they will always be more important then the locally handled ones. Like a fire alarm or the boss telling you to stop what you are doing and pick up a more urgent task that popped up.

Answer 12

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.