Why is the Catch(Exception) almost always a bad Idea?

Why is the catch(Exception) almost always a bad Idea?

36105 次浏览

Because when you catch exception you're supposed to handle it properly. And you cannot expect to handle all kind of exceptions in your code. Also when you catch all exceptions, you may get an exception that cannot deal with and prevent code that is upper in the stack to handle it properly.

The general principal is to catch the most specific type you can.

Because you don't really know why an exception happened, and several exceptions require very special car to be handled correctly (if possible at all), such as a OutOfMemoryException and similar low-level system exceptions.

Therefore, you should only catch exceptions:

  • which you know exactly how to deal with it (e.g. FileNotFoundException or so)
  • when you will re-raise them afterwards (for instance to perform post-fail cleanup)
  • when you need to transport the exception to another thread

It depends on what you need. If you need to handle different types of exceptions in different ways then you should use multiple catch blocks and catch as much specific exceptions as you can.

But sometimes you may need to handle all exceptions in the same way. In such cases catch(Exception) may be ok. For example:

    try
{
DoSomething();
}
catch (Exception e)
{
LogError(e);
ShowErrorMessage(e); // Show "unexpected error ocurred" error message for user.
}

You should only catch exceptions if you can properly handle them. As you cannot properly handle all possible exceptions you should not catch them :-)

Short story: it's called bug masking. If you have a piece of code which is not working well and throwing exceptions (or you pass malformed input to that piece of code) and you just blind your eyes by catching all possible exceptions, you will actually never uncover the bug and fix it.

This may be java specific:

Sometimes you will need to call methods that throw checked exceptions. If this is in your EJB / business logic layer you have 2 choices - catch them or re-throw them.

Catching specific exception classes means you will need to re-analyze your actions for which exceptions can be thrown when you look to see how this code handles exceptions. You will often get into a "what if..." situation and it can be a lot of effort just working out if exceptions are handled correctly.

Re-throwing means that code calling your EJBs will be littered with catching code that will typically not mean anything to the calling class. n.b. throwing checked exceptions from EJB methods will mean that you are responsible for manually rolling back any transactions.

Besides what yet answered by @anthares:

Because when you catch exception you're supposed to handle it properly. And you cannot expect to handle all kind of exceptions in your code. Also when you catch all exceptions, you may get an exception that cannot deal with and prevent code that is upper in the stack to handle it properly.

The general principal is to catch the most specific type you can.

catch(Exception) is a bad practice because it catches all RuntimeException (unchecked exception) too.

But sometimes it is OK! Like if you have a piece of code that does something 'extra', which you really don't care about, and you don't want it to blow up your application. For example, I worked on a large application recently where our business partners wanted a certain daily transaction to be summarized in a new log file. They explained that the log wasn't all that important to them, and that it did not qualify as a requirement. It was just something extra that would help them make sense of the data being processed. They did not need it, because they could get the information elsewhere. So that is a rare case where it is perfectly fine to catch and swallow exceptions.

I also worked at a company where all Throwables were caught, and then rethrown inside a custom RuntimeException. I would not recommend this approach, but just pointing out that it is done.

I find two acceptable uses of catch(Exception):

  • At the top level of the application (just before returning to the user). That way you can provide an adequate message.
  • Using it to mask low-level exceptions as business ones.

The first case is self-explanatory, but let me develop the second:

Doing:

try {
// xxxx
} catch(Exception e) {
logger.error("Error XXX",e)
}

is bug masking like @dimitarvp said.

But the below is different:

try {
// xxxx
} catch(Exception e) {
throw new BusinessException("Error doing operation XXX",e)
}

This way you aren't ignoring bugs and hiding them under the carpet. You are providing a high-level exception with a more explanatory message to higher application layers.

It's also always important to manage exceptions at the correct layer. If you escalate a low-level exception to a high business layer, it's practically impossible for the higher layer to manage it well.

In that case, I prefer to mask the low level exceptions with a business one that provides a better context and message and that also has the original exception to be able to go into the details.

Even so, if you can catch more concrete exceptions and provide better treatment for them you must do it.

If in a block of code you can get an SQLException and a NetworkException you must catch them and provide adequate messages and treatment for each of them. But if at the end of the try/catch block you have an Exception mapping it to a BusinessException it's ok for me.

In fact, I find it adequate when higher service layers only throw business exceptions (with details inside).

Isn't it another valid scenario to ensure that a thread keeps alive catching exception inside it?

Thread shouldRunWhenApplicationRuns = new Thread() {
@Override
public void run() {
try {
// do something that should never end
} catch (Exception ex) {
// log it
}
};
shouldRunWhenApplicationRuns.start();