Search code examples
javaexceptiontry-catch

What is the problem with catching the base Exception class?


Many people claim that one should never catch the base class Exception (see Microsoft Guideline, SONAR guidelines: "Exception" should not be caught when not required by called methods )

Instead you should determine all of the non-base exception classes that are going to be thrown within the try block, and make a catch statement that lists all the specific subtypes. I don't understand why this is a good idea and this question presents a situation where one should catch the base class.

Consider this case of a writing a web service handler below. The try block contains all the normal processing and at the end of that a 200 response code is return along with the result of the calculation. There is a catch block which creates a 400 errro response with the exception encoded in the return message. Something like this:

handleWebRequest( request,  response ) {
    try {
        method1();
        method2();
        method3();
        response.writeResponse(200, responseObject);
    }
    catch (Exception e) {
        response.writeResponse(400, formErrorObject(e));
        logError(e);
    }
}

The guidelines say not to catch Exception class directly, but instead all the specific types that might come from methods 1, 2, and 3. For example, if method1 throws Exception1 and so on, you might have something like this: (RuntimeException needs to be included because those won't appear in the signature.)

handleWebRequest( request,  response ) {
    try {
        method1();
        method2();
        method3();
        response.writeResponse(200, responseObject);
    }
    catch (Exception1|Exception2|Exception3|RuntimeException e) {
        response.writeResponse(400, formErrorObject(e));
        logError(e);
    }
}

One can do that, but what if later method2 changes to throw Exception2 and Exception2a?? The coding standard says you should come back and modify the catch block for every occasion that method2 is used.

What if you forget to modify this catch condition? Then there will be exceptions that are not caught, and you will get no error response back from the server. There is no compiler warning that you forgot to include an exception type -- indeed you wouldn't want that because letting exceptions fly to the next level is common.

But in the case, it is absolutely important that all exceptions are caught. The smartest way to do this is to catch the base class Exception. It will automatically include any new exception types that method2 might obtain. We know for certain that we want an error response sent back from the service no matter what exception was thrown.

  • Can anyone tell me what harm is done by catching the base Exception class in this situation?

  • Can anyone list exceptions that it would be bad if caught in this situation?


Solution

  • Many people claim that one should never catch the base class Exception

    Many people think the moon is made of cheese. This is a logical fallacy known as appeal to the masses. It's quite simple, really: This is a load of lies. Not just "It is wrong" (that statement is wrong, very wrong), but even more perniciously: Most programmers do not believe this.

    The problem is, it's almost always true. In certain contexts it really is always true. But in the context you're thinking about it (web handler containers), it's not.

    The thing with rules of thumb

    Programming is hard. Really hard. And hard to get into. Rules, especially clearly stated iron fisted ones, actually help. If there are 1000 ways to do a thing and most of them are 'wrong' but you can't quite identify what 'wrong' really is, then if someone tells you a simple, easily understood and applied rule that eliminates 950 of the 1000 ways, that is helpful.

    Hence, rules of thumb stated without any caveats are very common.

    "You should never catch Exception".

    "Optimization is the root of all evil".

    "goto is bad"

    "Never repeat yourself"

    and the list goes on and on.

    The problem with this is that they are all factually wrong. Specifically, the word 'never'. That's the problem. The correct statement is almost never. However, newbies don't like this. "Never catch Exception" is a fantastic rule. Easy to apply. "Almost never catch Exception" is pretty much useless. If you run into a scenario where a newbie thinks: "Ooh, I think I can solve this with catching Exception", it might be correct. That rule says 'almost never' - maybe this is one of those cases where it's allowed?

    Nevertheless, all these style rules are rules of thumb. As in, exceptions exist. Always. (and this rule of 'all style guide rules have exceptions' itself has an exception, probably).

    Unfortunately, there are no easy answers here. Slavish adherence to style guides is the sign of a bad programmer. A programmer that thinks they are good because they are good at slavishly applying style guides and bashes others in the team over the head with them is downright toxic, even.

    Hence, we get to the thing with rules of thumb:

    You MUST understand the concept they are trying to convey, you CANNOT simply apply them as stated.

    Consider it this way: All rules of thumb include unstated caveats. If you don't know them, applying them is pointless.

    So, "Never catch exception", pithy and useful as it may sound, is useless advice. It's only useful if you consider it as shorthand for this much more robust, but extremely wordy advice:

    • Usually only endpoint containers (such as a web handler routing framework, the code in the JVM that invokes your main method, an app server, that sort of thing) should catch generic throwables: Exception or Throwable or RuntimeException or Error.

    • Catch-em-all denies handling specific issues to any code that is below you on the stack. For example, catching-all in a web handler means it cannot possibly get to the web framework's routing system. Therefore, if it has a configured error handler along with all sorts of useful stuff for dealing with errors (such as logging the IP and request headers and parameters along with the stack trace, which is extremely useful), you aren't using all that stuff, and that is bad.

    • catch blocks should actually deal with the error, and 'log it' is not dealing with an error. Generalized throwable types have an unclear meaning therefore it is not possible to actually deal with the problem in such a catch block. Therefore, trivially, any such catch block is bad, QED. Unless, of course, this doesn't apply and 'log it' really is the only thing left to do. Then it's fine..

    You see how that half-a-page doesn't come to mind nearly as easily as pithy oversimplified hackery such as "Never catch Exception!". Nevertheless, there are only two options:

    1. "Never catch Exception" is wrong, and anybody who says it is a bad programmer actively ruining your code base by erroneously treating rules of thumb as sacrosanct inviolable laws.

    2. You are to interpret that statement as a summary of that half a page I just wrote.

    Your specific example

    is totally fine.

    What you're doing in your example snippet is a case of 'endpoint runner': It's a transition from a framework in which 'applications' can be plugged (and what is 'this code responds to requests coming in to /path/on/webserver, but an application?) - it runs endpoints. catching all exceptions is correct.

    In fact, endpoints (such as main, or doGet in the servlet framework, and so on) SHOULD be allowed to declare throws Exception and most endpoints should do it. Managing checked exceptions is a thing libraries should do to inform the code that calls these libraries what alternative exit modes exist. Apps themselves shouldn't bother. However, any app larger than a person-week of work should be cut into modular parts, and each modular part should be written like a library. including having sane exception management: Expectable, handleable problems should be represented by checked exception types that accurately represent the problem.

    NB: Keep in mind that writing endpoint handlers is usually not a job for someone with little experience. Hence, if somebody is unlikely to ever write an endpoint runner, then we can boil all this down to 'do not ever catch Exception' more or less 'safely'. All the caveats that apply to that 'advice' aren't gonna come up yet. So, next time you see a programmer telling someone 'hey, just so you know, dont catch Exception', don't bite their head off just yet. They might simply be trying to keep the extremely complicated task of learning how to program manageable.