What do you think of the "Don't catch unexpected Exceptions" best practice?

Question

I've read a lot of times that one should never blindly catch exceptions. Some people say it's ok to wrap your Main() into a catch block to display errors instead of just exiting (see this SO post for example), but there seems to be a consensus that you should never let your program running if something unexpected occurred, since it's in an unknown state, and may act in unexpected ways.

While I agree on the fact that hiding bugs rather than fixing 'em is definitely a wrong idea, consider the following case :

You've got a huge server. Million of lines of code.

When it starts, it loads all the Customer into its local cache.

To, me, it makes a lot of sense to write this :

           foreach (string CustomerID in Customers)
                try
                {
                    LoadCustomer(CustomerID);
                }
                catch (Exception ex) // blind catch of all exceptions
                {
                    // log the exception, and investigate later.

                }

Without the blind catch, failing to load a single Customer would just crash all the server.

I definitely prefer having my server up with a little unknown side effect on one customer rather than my whole server down.

Of course, if I ever run my catch code, the first thing I'll do is fix the bug in my code.

Is there something I'm overlooking here? Are there known best practices (other than the 'never catch unexpected exception' strategy'?)

Is it better to catch the exception in the LoadCustomer() method, to rethrow a 'CustomerLoadException' from there, and to catch CustomerLoadException instead of all Exception in the calling code?

Answer 1

It's a case-by-case thing. If you can guarantee that the loading failure doesn't have any effects on the rest of the customers, then logging and skipping that particular customer seems reasonable. Not all exceptions work that way, though. Especially for error conditions, e.g., VirtualMachineError, it's often the case that the problem has compromised the entire program run, and you should abort.

Answer 2

To address exceptions which would really compromise my program stability, maybe it's a good idea to write this kind of code :

      foreach (string CustomerID in Customers)
            try
            {
                LoadCustomer(CustomerID);
            }
            catch (Exception ex) // blind catch of all exceptions
            {
                if (ex is OutOfMemoryException || ex is StackOverflowException ||...)
                     throw;
                // log the exception, and investigate later.

            }

Answer 3

I definitely prefer having my server up with a little unknown side effect on one customer rather than my whole server down.

How do you know that the bug that caused the exception within LoadCustomer() hasn't hosed anything else? On the whole, I think I'd prefer a "log exception, re-throw the exception to a higher level" and there, probably, exit.

In this specific case, I could probably argue for both ways of dealing with it, though, but in general, handling errors that you don't have a clear way of dealing with is not ideal.

Answer 4

I think the thing you're missing is that the "best practices" for a desktop application aren't necessarily the same as those for a server, which aren't the same as those for a web page.

If a web page throws an unhandled exception, you think "meh" and hit refresh. If a desktop app throws one, you might lose some data and be pretty annoyed. If a server throws one, your whole business could be down until it's fixed.

It also depends on how easy it is to patch - if it's easy for you to push a patch to all installs (such as your own server or an in-house app) it might be better to leave the exception uncaught. If you're writing something that can't be patched (a game for an older console or an embedded device application) then you might be better swallowing the exception.

Of course, in all circumstances it would be better to have your tests catch the problem so you can deal with it properly, but we all know that bugs happen...

Answer 5

I am not very experienced in this area. However, it appears to me that: 1. There are some exceptions to almost any rule. Knowing when to break a rule is important. 2. In the case of exception handling, it is almost never a good idea to catch exceptions blindly. This is because you may be catching some really unexpected errors.

For instance, in Python (2.5.2 at least), catching blindly will allow me to catch the ctrl+c signal (Linux). Which means that I cannot terminate the application in an emergency.

1. For your webserver code, you may want to do one of the following
   +Use a custom exception that is thrown if a customer fails to load and log/correct that.
   +Use exception handling in a deeper level of the code and handle it there.

Answer 6

Ok let me put it this way.. say the mysterious Customer failed to load because it doesn't exist, say because someone has figured out how to hack your system's front layers. Now imagine the hacker does all sorts of malicious stuff with the power of not existing until he happens to trigger some function that tries to read from an unknown value of the customer, now the system will crash anyways, but the hacker would've been able to do all sort of disruptions with the non-valid customer.

Is that really better?

Answer 7

"Is there something I'm overlooking here?"

Yes.

"Are there known best practices (other than the 'never catch unexpected exception' strategy'?)"

Yes. Encapsulation. Allocation of Responsibility. The S. O. L. I. D. principles.

Is it better to catch the exception in the LoadCustomer() method, to rethrow a 'CustomerLoadException' from there, and to catch CustomerLoadException instead of all Exception in the calling code ?

Yes. This encapsulates all the things that can go wrong in the class definition that provides the loadCustomer method.

Answer 8

I think it depends on the senario but as a rule I only catch exception I expect to occure through everyday use of the app and use some sort of unhandeled exception reporting/logging tool such as health monitoring for ASP.Net. However if it's a critical part of the app that simply cannot have an unhandeled exception I catch all exceptions and again report/log them.

Answer 9

This is a question of Robustness, that is continuing to operate even in the event of unexplainable errors, vs Correctness, prefering to fail completely rather than produce unreliable results.

Clearly if you are working on, for example, life-support systems, you don't want your system to simply shut down due to an unknown error condition. Cotninuing to operate, even if your state isn't well defined, is probably better than terminating.

On the other hand, if your program is a shopping cart, it's probably better to simply fail completely rather than potentially send the wrong item, or charge the wrong amount of money to the wrong individuals.

Everything in between is a gray area and it's a judgment call. In the main, it would seem that life-support systems programming is more rare than shopping-cart programming, so the broad advice is for people to do what's most common, which is fail in the event of an unexpected error. It's understood that if you are working on a case where that's not appropriate (such as your server example), you will know better.

Answer 10

You are fine catching alle Exceptions (not Errors though) when you know what objects and resources might be affected and discard all those objects.

So in your case you might be fine to handle the exception converting the whole process into a noop. But you must be sure that: - no other shared resources are affecte (e.g. a Hibernate Session which might be dead after an exception) - a complete (trans)action is canceled an not only half of it. This means that such an exception handling may only appear in special places, normaly directly 'under' the user interface. Example: the user presses a button which is supposed to load a customer, change its address and save it again. When any of those goes wrong you may catch the exception prevent all remaining steps to happen, throw away the customer object and present a message that says: sorry, didn't work.

But if you catch the exception (e.g. during modifying the address) and then go on working, the user things he has a customer object wiht changed adress under his control, while in reality he controls a customer object with a destroyed address. Not a good situation.

So the rule is: define the layer where you control transactions. This should be the layer that catches Exceptions. Could be SwingActions, could be Threads, could be public void main, could be inside a loop as in your example.

Answer 11

I've been caught out before by catching a general "Exception" rather than the individual type. It means you loose some information about what actually went wrong. You normally want to handle each type of exception differently.

Plus top-level exception handling should only be used to log the error and restart your app. If you don't know what type of exception cause the error your program could be in an invalid state, you could me out of memory or something else.

Answer 12

It depends on where you are standing. If you are on the low level, most exceptions you can foresee and you should explicitly catch only those, and leaving the rest bubble up. By the time you get to the GUI or top level logic, all known exception would have been caught and you would end up with only unexpected exceptions. Now you have 2 options:

1. Pessimistic view: show the exception and give up.
2. Optimistic view: you have confidence you already caught all critic exceptions, so you swalow up the exception and continue (ON ERROR RESUME NEXT, anyone?)

Either way you log the exception and make sure you check the logs on a regular basis.

On the end it's an "all depends" situation. What is worse? Continuing with bad data or exiting unexpectedly? Is it a critical system? Is it's output used by another system? etc, etc.

Anyway, all critical exception should have been caught on deeper levels of the application.

If thanks to the logs you find an unhandled exception, correct the code, test and redeploy.

Answer 13

It's perfectly fine to catch exceptions you know about. You might expect a NullObjectException in your example. Catching that is perfectly reasonable.

When you catch generic Exception, you're essentially saying that "I don't know what went wrong, or if the server's on fire, and I don't care." While your server may continue to run, there is no telling what the internal state may be, or if this exception will lead to a crash down the road.

Additionally, by catching arbitrary Exceptions at this time, if a crash occurs later, you're further from where the 'real' error happened - which will make it much harder to find the error.

Similarly, while not catching a generic Exception may cause an immediate crash, it's a crash that is more likely to be caught early in development cycle, and to be easily fixed (since you'll know where the error actually occurred).

There's really no upside to catching Exception.