Catching base Exception class in .NET

Question

Hello,

I keep hearing that

catch (Exception ex)

Is bad practise, however, I often use it in event handlers where an operation may for example go to network, allowing the possibility of many different types of failure. In this case, I catch all exceptions and display the error message to the user in a message box.

Is this considered bad practise? There's nothing more I can do with the exception: I don't want it to halt the application, the user needs to know what happened, and I'm at the top level of my code. What else should I be doing?

EDIT:

People are saying that I should look through the stack of calls and handle errors specifically, because for example a StackOverflow exception cannot be handled meaningfully. However, halting the process is the worst outcome, I want to prevent that at all costs. If I can't handle a StackOverflow, so be it - the outcome will be no worse than not catching exceptions at all, and in 99% of cases, informing the user is the least bad option as far as I'm concerned.

Also, despite my best efforts to work out all of the possible exceptions that can be thrown, in a large code-base it's likely that I would miss some. And for most of them the best defense is still to inform the user.

Answer 1

It's perfectly okay if you re-raise exceptions you can't handle properly. If you just catch the exceptions you could hide bugs in the code you don't expect. If you catch exceptions to display them (and bypass the die-and-print-traceback-to-stderr behavior) that's perfectly acceptable.

Answer 2

It is bad practice in the sense that you shouldn't do it everywhere.

In this case, I would consider it the only reasonable solution as your exception could be truly anything. The only possible improvement would be to add extra handlers before your catch everything for specific error cases where you could do something about the exception.

Answer 3

No; in that case if you don't want to halt the program there's nothing else you can do and at the top level is the right place to do it, as long as you're logging properly and not hiding it away in hope grin

Answer 4

The bad practice is

catch (Exception ex){}

and variants:

catch (Exception ex){ return false; }

etc.

Catching all exceptions on the top-level and passing them on to the user (by either logging them or displaying them in a message-box, depending on whether you are writing a server- or a client-application), is exactly the right thing to do.

Answer 5

It makes complete sense to catch the exception at the highest level in your code. Catching the base Exception type is fine as long as you don't need to do any different logic based on the exception's type.

Also, make sure you're displaying a friendly, general error message and not showing the actual exception's message. That may lead to security vulnerabilities.

Answer 6

The important thing is to understand the path of exceptions through your application, and not just throw or catch them arbitrarily. For example, what if the exception you catch is Out-Of-Memory? Are you sure that your dialog box is going to display in that case? But it is certainly fine to define a last-ditch exception point and say that you never want errors to propagate past that point.

Answer 7

When I see

catch (Exception ex)

my hand starts to groping for a hammer. There are almost no excuses to catch base Exception. Only valid cases that come to my mind are:
1) 3rd party component throws Exception (be damned it's author)
2) Very top level exceptions handling (as a last resort) (for example handle "unhandled" exceptions in WinForms app)

If you find a case where many different types of exceptions can happen it's a good sign of bad design.

I would disagree with Armin Ronacher. How would you behave if StackOverflow exception raised? Trying to perform additional actions can lead to even worse consequences. Catch exception only if you can handle it in meaningful and safe way. Catching System.Exception to cover range of possible exceptions is terribly wrong. Even when you are re-throwing it.

Answer 8

You should catch the exceptions related to what you are doing. If you look at the methods you call, you will see what exceptions they throw, and you want to stay more specific to those. You should have access to know what exceptions may be thrown by the methods you call, and handle them appropriately.

And... better than having one big try catch, do your try and catch where you need the catch.

try {
   myThing.DoStuff();
}
catch (StuffGoneWrongException ex) {
   //figure out what you need to do or bail
}

Maybe not quite this closely packed, but it depends on what you are doing. Remember, the job isn't just to compile it and put it on someones desktop, you want to know what breaks if something did and how to fix it. (Insert rant about tracing here)

Answer 9

I think the poster is referring to exception handling like this:

try {something} catch (SqlException) {do stuff} catch (Exception) {do other stuff}

The idea here is that you want to catch the more specific errors (like SqlException) first and handle them appropriately, rather than always relying on the catch-all general Exception.

The conventional wisdom says that this is the proper way to do exception handling (and that a solo Catch (Exception ex) is bad). In practice this approach doesn't always work, especially when you're working with components and libraries written by someone else.

These components will often throw a different type of exception in production than the one your code was expecting based on how the component behaved in your development environment, even though the underlying problem is the same in both environments. This is an amazingly common problem in ASP.NET, and has often led me to use a naked Catch (Exception ex) block, which doesn't care what type of exception is thrown.

Structured exception handling is a great idea in theory. In practice, it can still be a great idea within the code domain that you control. Once you introduce third party stuff, it sometimes doesn't work very well.

Answer 10

We use Catch ex as Exception (VB.Net variant) quite a bit. We log it, and examine our logs regularly. Track down the causes, and resolve.

I think Catch ex as Exception is completely acceptabile once you are dealing with production code, AND you have a general way to handle unknown exceptions gracefully. Personally I don't put the generic catch in until I've completed a module / new functionality and put in specialized handling for any exceptions I found in testing. That seems to be the best of both worlds.

Answer 11

I find the arguments that generic catches are always bad to be overly dogmatic. They, like everything else, have a place.

That place is not your library code, nor the classes you custom-develop for your app. That place is, as many have mentioned, the very top level of the app, where if any exception is raised, it is most likely unexpected.

Here's my general rule (and like all rules, it's designed to be broken when appropriate):

I use classes and custom-built libraries for the majority of the lifting in an app. This is basic app architecture -- really basic, mind you. These guys try to handle as many exceptions as possible, and if they really can't continue, throw the most specific kind available back up to the UI.

At the UI, I tend to always catch all from event handlers. If there is a reasonable expectation of catching a specific exception, and I can do something about it, then I catch the specific exception and handle it gracefully. This must come before the catch all, however, as .NET will only use the very first exception handler which matches your exception. (Always order from most specific to most generic!)

If I can't do anything about the exception other than error out (say, the database is offline), or if the exception truly is unexpected, catch all will take it, log it, and fail safe quickly, with a general error message displayed to the user before dying. (Of course, there are certain classes of errors which will almost always fail ungracefully -- OutOfMemory, StackOverflow, etc. I'm fortunate enough to have not had to deal with those in prod-level code ... so far!)

Catch all has its place. That place is not to hide the exception, that place is not to try and recover (because if you don't know what you caught, how can you possibly recover), that place is not to prevent errors from showing to the user while allowing your app to continue executing in an unknown and bad state.

Catch all's place is to be a last resort, a trap to ensure that if anything makes it through your well-designed and well-guarded defenses, that at a minimum it's logged appropriately and a clean exit can be made. It is bad practice if you don't have well-designed and well-guarded defenses in place at lower levels, and it is very bad practice at lower levels, but done as a last resort it is (in my mind) not only acceptable, but often the right thing to do.

Answer 12

Yes, it is fine to catch the base Execption at the top level of the application, which is what you are doing.

The strong reactions you are getting is probably because at any other level, its almost always wrong to catch the Base exception. Specifically in an a library it would be very bad practice.

Answer 13

Sometimes letting the application die is better. The application could get in to a state where it lets the user think their they're accomplishing something (viewing accurate data, properly saving data, etc) but they're not.

Answer 14

a lot of times exception are catched to free resources, it' s not important if exception is (re)thrown. in these cases you can avoid try catch:

1) for Disposable object you can use "using" keyword: using(SqlConnection conn = new SqlConnection(connStr)) { //code } once you are out of the using scope (normally or by a return statement or by exception), Dispsose method is automatically called on object. in other word, it' s like try/finally construct.

2) in asp.net, you can intercept Error or UnLoad event of Page object to free your resource.

i hope i help you!

Answer 15

I'm responding to "However, halting the process is the worst outcome..."

If you can handle an exception by running different code (using try/catch as control flow), retrying, waiting and retrying, retrying with an different but equivalent technique (ie fallback method) then by all means do so.

It is also nice to do error message replacement and logging, unless it is that pseudo-polite-passive-aggressive "contact your administrator" (when you know there is no administrator and if there was the administrator can't do anything about it!) But after you do that, the application should end, i.e. same behavior you get with an unhandled exception.

On the other hand, if you intend to handle the exception by returning the user to a code thread that has potentially trashed its state, I'd say that is worse than ending the application and letting the user start over. Is it better for the user to have to restart at the beginning or better to let the user destroy data?

If I get an unexpected exception in the module that determines which accounts I can withdraw money from, do I really want to log and report an Exception and return the user to the withdraw money screen? For all we know we just granted him the right to withdraw money from all accounts!

Answer 16

This is all good of catching exceptions that you can handled. But sometimes it also happens that due to unstable environment or users just do the process correctly, the application runs into unexpected exception. Which you haven't been listed or handled in code. Is there a way that the unhandled exception is captured from app.config file and displays a common error?

Also puts that details exception message in a log file.

Answer 17

I've been working a fair bit with exceptions, and here's the implementation structure I'm currently following:

1. Dim everything to Nothing / String.Empty / 0 etc. outside of Try / Catch.
2. Initialise everything inside Try / Catch to desired values.
3. Catch the most specific exceptions first, e.g. FormatException but leave in base Exception handling as a last resort (you can have multiple catch blocks, remember)
4. Almost always Throw exceptions
5. Let Application_Error sub in global.asax handle errors gracefully, e.g. call a custom function to log the details of the error to a file and redirect to some error page
6. Kill all objects you Dim'd in a Finally block

One example where I thought it was acceptable to not process an exception 'properly' recently was working with a GUID string (strGuid) passed via HTTP GET to a page. I could have implemented a function to check the GUID string for validity before calling New Guid(strGuid), but it seemed fairly reasonable to:

Dim myGuid As Guid = Nothing

Try

myGuid = New Guid(strGuid)

'Some processing here...

Catch ex As FormatException lblError.Text = "Invalid ID" Catch ex As Exception Throw Finally If myGuid IsNot Nothing Then myGuid = Nothing End If End Try