Should I catch exceptions thrown when closing java.sql.Connection

Question

Connection.close() may throw SqlException, but I have always assumed that it is safe to ignore any such exceptions (and I have never seen code that does not ignore them).

Normally I would write:

 try{
    connection.close();
 }catch(Exception e) {}

Or

 try{
    connection.close();
 }catch(Exception e) {
     logger.log(e.getMessage(), e); 
 }

The question is:

1. Is it bad practice (and has anyone had problems when ignoring such exceptions).
2. When Connection.close() does throw any exception.
3. If it is bad how should I handle the exception.

Comment:

I know that discarding exceptions is evil, but I'm reffering only to exceptions thrown when closing a connection (and as I've seen this is fairly common in this case).

Does anyone know when Connection.close() may throw anything?

Answer 1

Actually, what you're doing is (almost) best practice :-) here's what I saw in Spring's JdbcUtils.java. So, you might want to add another Catch block.

/**
 * Close the given  ResultSet and ignore any thrown exception.
 * This is useful for typical finally blocks in manual  code.
 * @param resultSet the  ResultSet to close
 * @see javax.resource.cci.ResultSet#close()
 */
private void closeResultSet(ResultSet resultSet) {
  if (resultSet != null) {
    try {
      resultSet.close();
    }
    catch (SQLException ex) {
      logger.debug("Could not close  ResultSet", ex);
    }
    catch (Throwable ex) {
      // We don't trust the  driver: It might throw RuntimeException or Error.
      logger.debug("Unexpected exception on closing  ResultSet", ex);
    }
  }
}

Answer 2

In general, I've had days wasted by people throwing away exceptions like that.

I recommend following a few basic rules with exceptions:

If you are ABSOLUTELY SURE you will NEVER cause a problem with a checked exception, catch JUST that exception and comment exactly why you don't need to handle it. (Sleep throws an InterruptedException that can always be ignored unless you actually are interested in it, but honestly this is the only case I usually ignore--even at that, if you never get it, what's the cost of logging it?)

If you are not sure, but you may get it occasionally, catch and log a stack trace just so that if it is causing a problem, it can be found. Again, catch only the exception you need to.

If you don't see any way the checked exception can be thrown, catch it and re-throw it as an unchecked exception.

If you know exactly what is causing the exception, catch it and log exactly why, you don't really need a stack trace in this case if you are very clear as to what's causing it (and you might mention the class that's logging it if you're not already using log4j or something.

It sounds like your problem would fall into the last category, and for this kind of a catch, never do what you wrote (Exception e), always do the specific exception just in case some unchecked exception is thrown (bad parameters, null pointer, ...)

Update: The main problem here is that Checked Exceptions are ungood. The only highly used language they exist in is Java. They are neat in theory, but in action they cause this behavior of catch and hide that you don't get with unchecked exceptions.

A lot of people have commented on the fact that I said that hiding them is okay sometimes. To be specific, the one case I can think of is:

try {
    Thread.sleep(1000);
catch (InterruptedException e) {
    // I really don't care if this sleep is interrupted!
}

I suppose the main reason I feel this use is okay is because this use of InterruptedException is an abuse of the checked exception pattern in the first place, it's communicating the result of a sleep more than indicating an exception condition.

It would have made much more sense to have:

boolean interrupted=Thread.sleep(1000);

But they were very proud of their new checked exception pattern when they first created Java (understandably so, it's really neat in concept--only fails in practice)

I can't imagine another case where this is acceptable, so perhaps I should have listed this as the single case where it might be valid to ignore an exception.