BrownRecluse
BrownRecluse

Reputation: 1670

Catching throwable and handling specific exceptions

Ok I know catching throwable is not a good idea:

    try {
         // Some code
    } catch(Throwable e) { // Not cool!
        // handle the exception
    }

But recently I was reading through an open sourced code and I saw this interesting (at least to me) piece of code:

    try {
        // Some Code
    } catch (Throwable ex){
        response = handleException(ex, resource);
    }

    private handleException(Throwable t, String resource) {
        if (t instanceof SQLEXception) {
               // Some code
        } else if (t instanceof IllegalArgumentException) {
               //some code
        } //so on and so forth
    }

This doesn't seem to be that bad? What is wrong with this approach?

Upvotes: 25

Views: 15435

Answers (9)

Navankur Chauhan
Navankur Chauhan

Reputation: 417

You can always catch different type of exceptions and perform some operations based on the type of the exception you got.

Here is an example

          try{

              //do something that could throw an exception

             }catch (ConnectException e) {
                //do something related to connection


            } catch (InvalidAttributeValueException e) {
                // do anything related to invalid attribute exception

            } catch (NullPointerException e) {

                // do something if a null if obtained
            }

            catch (Exception e) {
            // any other exception that is not handled can be catch here, handle it here

            }
            finally{
          //perform the final operatin like closing the connections etc.
             }

Upvotes: 0

Chetan Kinger
Chetan Kinger

Reputation: 15212

You are right when you say that catching Throwable is not a good idea. However, the code that you present in your question is not catching Throwable in an evil way but let's talk about that later. For now, the code that you present in your question has several advantages :

1. Readability

If you look at the code carefully, you will notice that even though the catch block is catching a Throwable, the handleException method is checking the type of exception thrown and possibly taking different actions based on the exception type.

The code presented in your question is synonymous to saying:

try {
    doSomething();
} catch (SQLEXception ex){
    response = handleException(resource);
} catch(IllegalArgumentException ex) {
    response = handleException(resource);
} catch(Throwable ex) {
    response = handleException(resource);
}

Even if you have to catch 10+ exceptions only, this code can easily take up a lot of lines of code and the multi-catch construct is not going to make the code any cleaner. The code that you present in your question is simply delegating the catch to another method to make the actual method that does the work more readable.

2. Reusability

The code for the handleRequest method could easily be modified and placed in a utility class and accessed throughout your application to handle both Exceptions and Errors. You could even extract the method into two private methods; One that handles Exception and one that handles Error and have the handleException method that takes a Throwable further delegate the calls to these methods.

3. Maintainibility

If you decide that you want to change the way you log an SQLExceptions in your application, you have to make this change in a single place rather than visiting every method in every class that throws an SQLException.

So is catching Throwable a bad idea?

The code that you present in your question is not really the same as catching Throwable alone. The following piece of code is a big no-no:

try {
   doSomething();
} catch(Throwable e) {
    //log, rethrow or take some action
}

You should catch Throwable or Exception as far away in the catch chain as possible.

Last but not the least, remember that the code you present in your question is framework's code and there are certain errors that the framework can still recover from. See When to catch java.lang.Error for a better explanation.

Upvotes: 19

OldCurmudgeon
OldCurmudgeon

Reputation: 65793

Just to provide balance - there is one place where I will always catch (Throwable):

public static void main(String args[]) {
    try {
        new Test().test();
    } catch (Throwable t) {
        t.printStackTrace(System.err);
    }
}

At least something shows somewhere that something went wrong.

Upvotes: 2

Deduplicator
Deduplicator

Reputation: 45654

There are exactly two valid uses for using a huge net:

  • If you will handle everything uniformly, like a top-level catch for logging/reporting, possibly followed by an immediate exit.

  • To reduce duplication, by exporting all the handling into its own method.
    Catch the most derived common ancestor there is to avoid extra-work and increase clarity.
    DRY is an important design principle.

In both cases, unless you expected that exception and handled it completely, rethrow.

Upvotes: 3

James_pic
James_pic

Reputation: 3307

You posted a link to Jongo, which demonstrates one possible use for this technique: re-using error handling code.

Let's say you've got a large block of error handling code that naturally repeats itself in various places in your code - for example Jongo produces standard responses for some standard classes of errors. It may be a good idea to extract that error handling code into a method, so you can re-use it from all the places it's needed.

However, that's not to say that there's nothing wrong with Jongo's code.

Catching Throwable (rather than using multicatch) is still suspicious, as you're likely to catch Errors that you're not really in a position to handle (are you sure you meant to catch ThreadDeath?). In this situation, if you absolutely have to catch Throwable, it'd be better to "catch and release" (i.e, rethrow anything that you didn't mean to catch). Jongo doesn't do this.

Upvotes: 3

biziclop
biziclop

Reputation: 49724

Catching Throwables out of laziness is a bad idea.

This was particularly tempting before try-multi-catch was introduced.

try {
   ...
} catch (SomeException e) {
   //do something
} catch (OtherException e) {
   //do the same thing
} ...

Repeating catch blocks is tedious and verbose, so some people decided to just catch Exception or Throwable and be done with it. This is what should be avoided because:

  1. It makes it difficult to follow what you're trying to do.
  2. You may end up catching a lot of stuff you can't deal with.
  3. You deserve bonus punishment if you completely swallow the Throwable in the catch block. (And we've all seen code that does that...:))

But catching Throwables when it is absolutely necessary is fine.

When is it necessary? Very rarely. In framework-style code there are various scenarios (dynamically loading an external class is the most obvious one), in a standalone application a typical example is to attempt to display/log some kind of error message before exiting. (Bearing in mind that the attempt may fail, so you don't want to put anything critical there.)

As a rule of thumb, if there's nothing you can do about an exception/error, you shouldn't catch it at all.

Upvotes: 9

insidesin
insidesin

Reputation: 743

Java 7 solves a bit of the tedium that is multi-catching of similar exceptions with similar handling. You definitely should not be doing what the person has done here. Just catch the appropriate exceptions as needed, it may look ugly but then that's what throws is for, pass it to the method that should catch it and you shouldn't be wasting too much code space.

Check out this link for more information.

Upvotes: 2

crazzle
crazzle

Reputation: 271

First of all, catching Throwable makes your application rather intransparent. You should be as explicit as possible on catching exceptions to enable good traceability in exceptional cases.

Let's have a look at the method handleException(...) and see some of the problems that occur by this approach:

  • you catch Throwable but you only handle Exceptions, what happens if an e.g. OutOfMemoryError of type Error is thrown? - I see bad things to happen...
  • Regarding good object oriented programming using instanceof breaks the Open-Closed-Principle and makes code changes (e.g. adding new exceptions) really messy.

From my point of view, catch-blocks are exactly made for the functionality that are tried to cover in handleExceptions(...), so use them.

Upvotes: 2

Florian Schaetz
Florian Schaetz

Reputation: 10652

There are various reasons why you should not catch a Throwable. First of all is, that Throwable includes Errors - and there's normally not much an application can do if one of these appears. Also Throwable reduces your chances of finding out, WHAT has happened. All you get is "something bad has happened" - which might be a catastrophe or just a nuisance.

The other aproach is better but of course I still would not catch Throwable, but try to catch more specific Exceptions, if possible at all. Otherwise you are catching everything and then try to sort out which kind of bad thing happened. Your example could be written as...

try {
    ...
} catch (SQLEXception ex){
    response = ... ;
} catch (IllegalArgumentException ex){
    response = ...;
}

...which would reduce the amount of if ( ... instanceof ... ) blocks (which are only needed because the author first decided to catch everything in one big bucket). It something actually throws Throwable, then you don't have much choice, of course.

Upvotes: 21

Related Questions