Java – Top 5 Exception Handling Coding Practices to Avoid

This article represents top 5 coding practices related with Java exception handling that you may want to watch out for or better say, avoid, while doing coding for exception handling. Recently, I have been involved with code review of multiple Java projects and found following as most commonly seen coding instances across various Java projects. As a matter of fact, I recently ran sonar code analysis on Spring Core project (Spring Framework) and found the below mentioned instances related with exception handling. Please note that these are suggested to be avoided as a general coding practice and do not mean that they can not be used at all. There are still cases where one may end up using coding practice against following, but that should happen on case-to-case basis and not as a general practice. Please feel free to comment/suggest if I missed to mention one or more important points. Also, sorry for the typos.

Following are the key points described later in this article:

• Throwable and Error classes should not be caught
• Throwable.printStackTrace(…) should never be called
• Generic exceptions Error, RuntimeException, Throwable and Exception should never be thrown
• Exception handlers should preserve the original exception
• System.out or System.err should not be used to log exceptions

Throwable and Error classes should not be caught

One should try and avoid catching Throwable and Errors in their code. There is a very detailed article written on this topic on this page. Following is an example of non-compliant code taken from Spring-core code:

try {
	cl = Thread.currentThread().getContextClassLoader();	
}catch (Throwable ex) { //Non-compliant code
}

In nutshell, following are some of the reasons:

• Throwable is the superclass of all errors and exceptions in Java. Error is the superclass of all errors which are not meant to be caught by applications. Thus, catching Throwable would essentially mean that Errors such as system exceptions (e.g., OutOfMemoryError, StackOverFlowError or InternalError) would also get caught. And, the recommended approach is that application should not try and recover from Errors such as these. Thus, Throwable and Error classes should not be caught. Only Exception and its subclasses should be caught.

Above said, there are reasons why people still go for catching Throwable. The data errors such as encoding issues etc which are not known at programming time can be caught using this technique. However, catching Throwable such as InternelError or OutofMemoryError would not be of any help and should therefore be thrown. Thus, one should avoid writing code consisting of catching Throwable as general practice.

Throwable.printStackTrace(…) should never be called

Following is an example of code that represents the usage of invocation of printStackTrace on Throwable class.

try {
  /* ... */
} catch(Throwable t) {
  t.printStackTrace();        // Non-Compliant
}

Following are some of the reasons why one should avoid invoking printStackTrace method on Throwable/Exception classes and instead use Logger method using one of the frameworks such as LogBack or Log4J:

• Difficult to Retrieve Logs for Debugging:The logs written using printStackTrace is written to System.err which is hard to route or filter elsewhere. Instead, using Loggers, it is easy to retrieve logs for debugging purpose.
• Violation of Coding Best Practices:Generally, as per coding guidelines in production-ready applications, developers need to use Logger methods for logging different level of information. However, when it comes to exception handling, the instances of printStackTrace are commonly found in various places. This is, thus, a violation of coding practice and, thus, should be avoided.
• Following are some good pages explaining the reasons in detail:
  • Why is exception.printStackTrace() considered bad practice?
  • Avoid printStackTrace(); use a logger call instead

Generic exceptions such as Error, RuntimeException, Throwable and Exception should never be thrown

Following are some of the reasons why Generic Exceptions/Throwable should never be thrown:

• The primary reason why one should avoid throwing Generic Exceptions, Throwable, Error etc is that doing in this way prevents classes from catching the intended exceptions. Thus, a caller cannot examine the exception to determine why it was thrown and consequently cannot attempt recovery.
• Additionally, catching RuntimeException is considered as a bad practice. And, thus, throwing Generic Exceptions/Throwable would lead the developer to catch the exception at a later stage which would eventually lead to further code smells.

Following is the code sample that represents this code smell:

public void foo(String bar) throws Throwable { // Non-compliant
  throw new RuntimeException("My Message");    // Non-Compliant
}
// One other instance which displays throwing Exception
public void doSomething() throws Exception {...} // Non-compliant code

Instead, one would want to do something like following:

public void foo(String bar) { 
  throw new CustomRuntimeException("My Message");    // Compliant
}

Exception handlers should preserve the original exception

When writing code for doing exception handling, I have often seen code such as following which does some of the following:

• In the code sample below, the exception is lost.

  try { 
  	/* ... */ 
  } catch( Exception e ) {
  	SomeLogger.info( e.getMessage() ); // The exception is lost. Just that exception message is written; Also, context information is not logged.
  }
  

• In the code sample below, whole exception object is lost.

  try { 
  	/* ... */ 
  } catch( Exception e ) {
  	SomeLogger.info( "some context message" ); // The exception is lost
  }
  

• In the code sample below, no context message is provided.

  try { 
  	/* ... */ 
  } catch( Exception e ) {
          SomeLogger.info( e ); // No context message
  }
  

As a best practice, one would want to do something like following:

try { 
  /* ... */ 
} catch( Exception e ) {
  SomeLogger.info( "some context message", e ); // Context message is there. Also, exception object is present
}

In case of throwable exceptions, following should be done:

try {
  /* ... */
} catch (Exception e) {                                                   
  throw new CustomRuntimeException("context", e); // Context message is there. Also, exception object is present
}

System.out or System.err should not be used to log exceptions

The primary reason why one should avoid using System.out or System.err to log exception is the fact that one might simply loose the important error messages. Instead one should use Logging frameworks such as Log4J or LogBack etc to log the exceptions.