Catching specific vs. generic exceptions in c#

Question

This question comes from a code analysis run against an object I've created. The analysis says that I should catch a more specific exception type than just the basic Exception.

Do you find yourself using just catching the generic Exception or attempting to catch a specific Exception and defaulting to a generic Exception using multiple catch blocks?

One of the code chunks in question is below:

internal static bool ClearFlags(string connectionString, Guid ID)
{
    bool returnValue = false;
    SqlConnection dbEngine = new SqlConnection(connectionString);
    SqlCommand dbCmd = new SqlCommand("ClearFlags", dbEngine);
    SqlDataAdapter dataAdapter = new SqlDataAdapter(dbCmd);

    dbCmd.CommandType = CommandType.StoredProcedure;
    try
    {
        dbCmd.Parameters.AddWithValue("@ID", ID.ToString());

        dbEngine.Open();
        dbCmd.ExecuteNonQuery();
        dbEngine.Close();

        returnValue = true;
    }
    catch (Exception ex)
    { ErrorHandler(ex); }

    return returnValue;
}

Thank you for your advice

EDIT: Here is the warning from the code analysis

Warning 351 CA1031 : Microsoft.Design : Modify 'ClearFlags(string, Guid)' to catch a more specific exception than 'Exception' or rethrow the exception

Answer 1

You should almost never catch the top level Exception.

In most cases you should catch and handle the most specific exception possible and only if there is something useful you can do with it.

The exception (haha) to this is if you are catching for logging and re-throw the exception, then it is sometimes OK to catch a top level Exception, log it and rethrow it.

You should almost never catch a top level Exception and swallow it. This is because if you are catching a top level exception you don't really know what you are handling; absolutly anything could have caused it so you will almost certainly not be able to do anything that will handle every single failure case correctly. There probably are some failures that you may just want to silently handle and swallow, but by just swallowing top level Exceptions you'll also be swallowing a whole bunch that really should have been thrown upwards for your code to handle higher up. In your code example what you probably want to do is handle a SQLException and log+swallow that; and then for an Exception, log and rethrow it. This covers yourself. You're still logging all exception types, but your only swallowing the fairly predictable SQLException which indicates problems with your SQL/database.

A common practise is to only every handle exceptions that you can actually resolve at that point, if you can't resolve it at that point in code then you allow it to bubble upwards. If you can't resolve it at the next level up, allow it to continue up. If it reaches the top unhandled then display a polite appology to the user (perhaps attempt a quick autosave) and close the app. It's generally considered worse to allow an app to continue running after an unhandled exception because you can't predict the state of the application as something exceptional has occured. It's better just to shutdown and restart the app to get back to an expected state.

Answer 2

Have a look at this article by Krzysztof Cwalina, which I've found very helpful in understanding when to catch or ignore exceptions:

How to Design Exception Hierarchies

All the principles it describes about designing exception hierarchies are also applicable when deciding when to catch, throw, or ignore exceptions. He divides exceptions into three groups:

• Usage errors, such as DivideByZeroException, which indicate errors in code; you shouldn't handle these because they can be avoided by changing your code.
• Logical errors, such as FileNotFoundException, which you need to handle because you can't guarantee they won't happen. (Even if you check for the file's existence, it could still be deleted in that split-second before you read from it.)
• System failures, such as OutOfMemoryException, which you can't avoid or handle.

Answer 3

You should read a general paper or google "Structured Exception Handling" and get a better big picture of what this topic is all about, but in general, catching every exception is considered bad practice because you have no idea what the exception was (Memory fault, out of memory error, Disk failure, etc.).

And for many unknown/unexpected exceptions, you should not be allowing the application to continue. In general, you "catch" and handle only those exceptions toy have determined, as a result of an analysis of the method you are coding the catch clause for, that method can in fact create, and that you can do something about. The only time you should catch all exceptions (catch Exception x) is to do something like logging it, in which case you should immediately rethrow the same exception (whatever it was) so that it can bubble up the stack to some general "Unhandled Exception Handler" which can display an appropriate message to the user and then cause the application to terminate.

Answer 4

Yes,

You should catch from the most specific exception down to the least, so you can deal with things in an appropriate manner.

For example, if you were making a web request, you should catch things like TimeOuts and 404s first, then you can inform the end user they should retry (timeout) and/or check they URL they entered.

Then you could catch something less general, in case something a bit more wacky goes wrong, then fall right back to just catching an Exception in the case that something ridiculous happens.

Answer 5

As a best practice, you should avoid catching Exception and using flags as return values.

Instead, you should design custom exceptions for expected exceptions and catch those directly. Anything else should bubble up as an unexpected exception.

In your example above, you may want to rethrow a more business specific Exception.

Answer 6

I agree that, in general, you should only catch exceptions you're expecting and understand how to handle. A few cases where I often don't do this:

1. As mentioned above, if I'm capturing some sort of useful information to log and then rethrow.

2. If I'm performing an asynchronous operation, such as handling queued messages or jobs in a worker thread, and I want to catch the exception for rethrowing in a different context. I also often use an ugly hack here that tricks the CLR into appending stack trace information, so that it's not lost when rethrowing in the new context.

3. If I'm working with an isolated task or operation and I can handle the exception by shutting down the task, without shutting down the whole application. I often wish here that there were a top-level exception for truly fatal exceptions (like OutOfMemoryException), as I've been ignoring these. The proper way to handle this would be to run the isolated task in its own AppDomain, but I haven't had the available schedule time to implement this on a project yet.

Answer 7

I agree with the code analysis tool. My exception to the rule is that I catch the general exception in my event handlers and the user is given the option to report the error or to ignore it.

In the example you provide, I think the code analysis got it right. If you can't handle a specific exception right there, you shouldn't be catching anything at all and let it bubble up to the highest level. That way you'll have a much easier time recreating the issue when you try to fix it.

You could make your example better by adding the connection string and the ID value to the exception's Data property and be sure it is logged as well. That way you give yourself a fighting chance at reproducing the error.