Why not catch general Exceptions

Question

My VS just told me;

Warning 2 CA1031 : Microsoft.Design : Modify 'Program.Main(string[])' to catch a more specific exception than 'Exception' or rethrow the exception.

Why should I do that? If I do so, and don't catch all exceptions to handle them, my program crashes with the all-popular report-screen. I don't want my users to get such error-crap!

Why should I not catch all exceptions at once to display a nice warning to the user saying: "Something went wrong, don't care about it, I will handle it, just be patient"?

Edit: Just saw I have a dupe here, sorry for that Dupe

Edit2: To clarify things; I do exit the program after any exception has been catched! I just don't want my user to see that "report to microsoft" dialog that show up when an unhandled exception is raised in a console-application!

Ashlan

Answer 1

Swallowing exceptions is a dangerous practice because:

• It can cause the user to think something succeeded when it actually failed.
• It can put your application into states that you didn't plan for.
• It complicates debugging, since it's much harder to find out where the failure happened when you're dealing with bizarre/broken behavior instead of a stack trace.

As you can probably imagine, some of these outcomes can be extremely catastrophic, so doing this right is an important habbit.

Best Practice

First off, code defensively so that exceptions don't occur any more than necessary. They're computationally expensive.

Handle the expected exceptions at a granular level (for example: FileNotFoundException) when possible.

For unexpected exceptions, you can do one of two things:

• Let them bubble up normally and cause a crash
• Catch them and fail gracefully

Fail Gracefully?

Let's say you're working in ASP.Net and you don't want to show the yellow screen of death to your users, but you also don't want problems to be hidden from the dev team.

In our applications, we usually catch unhandled exceptions in global.asax and then do logging and send out notification emails. We also show a more friendly error page, which can be configured in web.config using the customErrors tag.

That's our last line of defense, and if we end up getting an email we jump on it right away.

That type of pattern is not the same as just swallowing exceptions, where you have an empty Catch block that only exists to "pretend" that the exception did not occur.

Other Notes

In VS2010, there's something called intellitrace coming that will allow you to actually email the application state back home and step through code, examine variable values at the time of the exception, and so on. That's going to be extremely useful.

Chant

Not exactly, but good point though. I work in console based applications and I hate it like hell if a program crashes - so I caught all exceptions and printed them out (with stacktrace) in the console whilst logging them in the background. – Ashlan

Gotcha. Well, it's still a similiar situation -- you could display a friendly message to the user, do some logging/reporting, and exit gracefully (or restart/return to a safe state). – Chant

See the answer of James Sheck, that's exactly what I did - didn't point that out though in my question... – Ashlan

Sounds like you're all over it then! :) – Chant

This comment begins by saying "Swallowing exceptions is a dangerous practice because..." I found this disconcerting since the original post explicitly described handling generic exceptions (in a particular, maybe suboptimal) way, not swallowing them. (I assume "swallowing exceptions" means "not doing anything to handle them.") I hope people reading this (old) discussion notice that disconnect. Swallowing exceptions is always bad. For the situation proposed by the original post, the answer should be more of "it depends on...". – Eusporangiate

@Eusporangiate You are correct... I think this odd answer was caused by the discussion on another response, and then there were updates afterwards that further refined his meaning. This needs more updating than you mentioned actually. At the end, I marvel at the amazing new features coming in VS2010. :) To be clear to anyone who reads this: swallowing exceptions is the practice of catching exceptions and then ignoring them entirely, as though they didn't happen. Which can be problematic and is only sort of what the OP was talking about. – Chant

Just one important thing: ASP.NET is written with the violation of CA1031. And at the end it brings same troubles as mentioned by @BrianMacKay . ASP.NET actually is written with a lot of such violations and some of them has even empty catch blocks. It does not matter if you have console application, or web or any other, theory of exceptions works same way everywhere. The modern approach is to kill the container with your broken instance of web app and to bring fresh one (which exists in well known state) up. – Astatine

Answer 2

Because programs that swallow (catch) exceptions indiscriminately, (and then continue), cannot be relied upon to do what it is they are expected to do. This is because you have no idea what kind of exception was "ignored". What if there was an overflow or memory access error that causes the wrong amount to be debited from a financial account? What if it steers the ship into the iceberg instead of away from it ? Unexpected failures should always cause the application to terminate. That forces the development process to identify and correct the exceptions it finds, (crashes during demos are a wonderful motivator), and, in production, allows appropriately designed backup systems to react when the software experiences an "unexpected" inability to do what it was designed to do.

EDIT: To clarify distinctions between UI components, and service or middleware componentrs.

In Service or Middleware components, where there is no user interacting with the code component from within the same process space that the code is running in, the component needs to "pass On" the exception to whatever client component imnitiated the call it is currently processing. No matter the exception, it should make every possible attempt to do this. It is still the case, however, tjhat in cases where an unexpected, or unanticipated exception occurs, the component should finally terminate the process it is running in. For anticipated or expected exceptions, a velopment analysis should be done to determine whether or not, for that specific exception, the component and it's host process can continue to operate (handling future requests), or whether it should be terminated.

Gittel

I actually had an instructor who insisted that you should catch all exceptions and I couldn't convince him that it was a bad idea. Needless to say, I don't do that in my normal programming. – Tonsillotomy

"Unexpected failures should always cause the application to terminate." This might be ok in interactive applications, but there are many situations where throwing up your hands and exiting is inappropriate. You imply that with "appropriately designed backup systems" but it was said without qualification and with always. You might want to qualify/clarify. – Adoration

@NVRAM, yes you're right in that the statement I made implies a volume of detail, but there's only so much you can put into an answer here. Nevertheless, at the risk of being absolute, I'd kinda tend (sounds tentative I guess) to stick to my "always" characterization. If an app cannot do what is expected of it, (That's my definition of an exception) then - Either you can handle the exception, (Then do so) or the user can handle it (Then ask him/her) or the application should terminate. – Gittel

Some answers like this one seem to me to be missing a lot of "it depends". Always bad to catch general exceptions? No, it depends. Consider a simple example: A long-running server application that launches numerous transactions. Code to handle a single transaction has a series of specific exception-type catches, followed by a final catch(Exception) block that logs all available exception information, declares that single transaction has failed, and then allows the server to start the next transaction. This seems like a fine idea, yet violates the "it's always always always wrong" meme. – Eusporangiate

No it doesn't... Your comment contains the clue. "... declares that single transaction has failed ..." is the place where you are "propagating" the exception to the calling routine. What if, in your example, inside the routine for that single transaction, where the exception actually occurred, the code caught and swallowed it. Then your code that "... declares that single transaction has failed ..." would never get to run, right? – Gittel

"What if it steers the ship into the iceberg instead of away from it ?" and what if not swallowing the exception causes the iceberg detection program to crash and not allow the ship to turn away from the iceberg? – Wrack

Answer 3

You should handle the exact exceptions you are capable of handling and let all others bubble up. If it displays a message to the user that means you don't quite know what you can handle.

Ardrey

If his program is main, isn't there no where else to bubble up to? – Proportional

@Kathy - correct. In this case the user will see the undesired dialog box. The thing is, the program should identify the things it can actually handle and do so, but there are others that may leave the program in a unstable state and it should be shut down immediately. – Brainard

Answer 4

Having worked on equipment used by emergency responders, I would rather the user see an ugly error message than to accidently swallow an exception that misleads the user into believing everything is "ok". Depending on your application, the consequence could be anything from nothing to a lost sale to a catastrophic loss of life.

If a person were going to catch all exception, show a better error dialog, and then quit the application, that's ok.. but if they are going to continue running after swallowing an unknown exception, I would fire a person for that. It's not ok. Ever.

Good coding is about practices that assume humans make mistakes. Assuming all "critical" exceptions have been caught and handled is a bad idea.

Hendiadys

No, the plan was to show a nice error message and quit the program in critical state (Environment.Exit(2)) – Ashlan

Exactly, Florian. You clearly stated in your original post that you were thinking of HANDLING the exception, just as you describe in this comment. You never brought up this topic of "swallowing" (silently?) exceptions...the other commenters did. – Eusporangiate

Answer 5

Simple answer: you are supposed to fix your bug. Find the place that throws the exception and unless it is beyond your control - fix it. Also catching (without rethrowing) all kinds of exception violates exception neutrality. In general you do not want to do this (although catching exceptions in main does look like special case)

Isotropic

Apo Y2k: question is about best practices. As an author you are free to do whatever you want of course, but then why asking question in first place? In the bug free program user will not see "error-messages" as exception whet it thrown is caught and dealt with in appropriate place. – Isotropic

Answer 6

Since your warning message shows that this is in Main(), I'll assume that in lower levels, you do catch only more specific Exceptions.

For Main(), I'd consider two cases:

1. Your own (debugging) build, where you want all the exception information you can get: Do not catch any Exceptions here, so the debugger breaks and you have your call stack,
2. Your public releases, where you want the application to behave normally: Catch Exception and display a nice message. This is always better (for the average user) than the 'send report' window.

To do this nicely, just check if DEBUG is defined (and define it, if VS doesn't do this automatically):

#if DEBUG
  yadda(); // Check only specific Exception types here
#else
  try
  {
    yadda();
  }
  catch (Exception e)
  {
     ShowMessage(e); // Show friendly message to user
  }
#endif

I'd disable the warning about catching general Exceptions, but only for your Main() function, catching Exception in any other method is unwise, as other posters have said already.

Ovine

That's a very nice idea, I always liked to split up development and productive parts of a project. Wonder why I didn't think of that... – Ashlan

Lennaert: Would it not be better to put the conditional directives specifically around the " try {" and "} catch {... }" blocks, to avoid duplicating code? (And introducing bugs, when one yadda() is changed, but not the other?) – Paradies

@Ian: That's what I do normally, but I thought that this would illustrate the idea a bit better. Ideally, there would be multiple catch blocks already, with only the last block (catch Exception) in the #if/#endif. – Ovine

Answer 7

There is a way to suppress certain messages from code analysis. I've used this for this exact reason (catching the general exception for logging purposes) and it's been pretty handy. When you add this attribute, it shows you've at least acknowledged that you are breaking the rule for a specific reason. You also still get your warning for catch blocks that are incorrect (catching the general exception for purposes other than logging).

MSDN SuppressMessageAttribute

Repair

Answer 8

I am all for catching specific known exceptions and handling state...but I use general catch exceptions to quickly localize problems and pass errors up to calling methods which handle state just fine. During development as those are caught, they have a place right next to the general exception and are handled once in release.

I believe one should attempt to remove these once the code goes into production, but to constantly be nagged during the initial code creation is a bit much.

Hence turn off (uncheck) the warning by the project settings as found in Microsoft.CodeQuality.Analyzers. That is found in the project settings under Code Analysis:

Feliks

Answer 9

All answers are good here. But I would mention one more option.

The intention of author to show some fancy message is understandable. Also, default Windows error message is really ugly. Besides, if application is not submitted to "Windows Excellence Program" the developer will not receive information about this problem. So what is the point to use default runtime handler if it does not help?

The thing here is that default exception handler of CLR host ( https://learn.microsoft.com/en-us/previous-versions/visualstudio/visual-studio-2008/9x0wh2z3(v=vs.90)?redirectedfrom=MSDN ) works in a very safe way. The purpose of it is clear: log the error, send it to developer, set the return code of your process and kill it. The general way of how to change that is to write your own host. In this case you can provide your own way of handling exceptions.

Still, there is an easy solution which satisfies CA1031 and still most of your needs.

When catching the exception, you can handle it your own way (log, show the message etc) and at the end you can set the process result code and do the exit (using the mix of Thread.Abort and "Exit" methods, for example). Still, at the end of your catch block you can just put "throw;" (which will never be called because of ThreadAbortedException, but will satisfy the rule). Still there are some cases, like StackOverflowException, which can't be handled like that and you will see that default message box, for fixing which you need to fallback to custom CLR host option.

Additionally, just for your information, you application can run several threads (besides that one which execute Main method). To receive exceptions from all of them you can use AppDomain.UnhandledException. This event does not allow you to "mark" the exception as handled, still you can freeze the thread using Thread.Join() and then do the job (log, msgbox, exit) using another (one more) thread.

I understand all this looks a little tricky and may be not right, but we have to deal with the implementation of AppDomain.UnhandledException, ThreadAbortException, CorruptedState exceptions and default CLR host. All of this eventually does not leave us much of choice.

Astatine

Answer 10

When you catch general exceptions, you get the side effect of potentially hiding run-time problems from the user which, in turn, can complicate debugging. Also, by catching general exception, you're ignoring a problem (which you're probably throwing elsewhere).

Polka

This leads to duplication of the catch-clause, especially in cases where I know that different exceptions are possible, but the effect for me is always the same - Retry or cancel? – Armenian

Answer 11

You can set up your try catch to catch multiple different behavior types and handle the exception based on the type. For most methods and properties in the framework, you can also see what exceptions they are capable of throwing. So unless you are catching an exception from an extremely small block of code, you should probably catch specific exceptions.

Mckale

Answer 12

In VS you can setup a custom error page to show your users when something goes wrong instead of catching it in a try-catch. I'm assuming since you're using VS that you're using ASP .NET. If so add this tag to your Web.Config under the System.Web tag:

<customErrors mode="RemoteOnly" defaultRedirect="~/CustomErrorPage.aspx" redirectMode="ResponseRewrite" />

You can also catch all uncaught exceptions in the Global.asax file (if you don't have it already: Right-click on web project, select Add Item, and search for it). There are a bunch of application wide event handlers in that file like "Application_Error" that catches every exception that isn't caught within your application so you don't have to use Try-Catch all the time. This is good to use to send yourself an email if an exception occurs and possibly redirect them to your homepage or something if you don't want to use the customErrors tag above.

But ultimately you don't want to wrap your entire application in a try-catch nor do you want to catch a general Exception. Try-catches generally slow down your application and a lot of times if you catch every general exception than it could be possible that you wouldn't know a bug exists until months or years later because the try-catch caused you to overlook it.

Interne