How to avoid throw clause in finally block

Question

I am using SonarQube for code quality. I got one issue related to exception handling, which says remove throw clause from finally block.

} catch(Exception e) {
            throw new MyException("request failed : ", e);
        } finally {
            try {
                httpClient.close();
            } catch (IOException e) {               
                throw new MyException("failed to close server conn: ", e);
            }
        }

Based on my understanding above code looks good. If I remove throw clause and suppress exception in finally then caller of this method will not be able to know server's status. I am not sure how we can achieve same functionality without having throw clause.

Noenoel

Did you try adding a finally block in that inner try-catch block and see if it still warns? – Aghast

If you're already using throw clauses to indicate other errors in the same function, perhaps it would be unwise to use a different method for this particular case? – Blackdamp

Oh, I think you're worried because even if the connection is never successfully made, your finally block still throws your MyException, am I right? – Blackdamp

@MarkoTopolnik, fancy providing a link that describes what a "try-with-resources block" is? – Blackdamp

@SebastianTroy yes you are right. – Noenoel

@Noenoel then surely the easiest thing to do would be to put an if statement before the throw clause, so that it only throws the exception if the connection was ever made in the first place? – Blackdamp

Answer 1

Your best shot is to use the Automatic Resource Management feature of Java, available since Java 7. If that is for some reason not available to you, then the next best thing is to replicate what that syntactic sugar expands into:

public static void runWithoutMasking() throws MyException {
   AutoClose autoClose = new AutoClose();
   MyException myException = null;
   try {
       autoClose.work();
   } catch (MyException e) {
       myException = e;
       throw e;
   } finally {
       if (myException != null) {
           try {
               autoClose.close();
           } catch (Throwable t) {
               myException.addSuppressed(t);
           }
       } else {
           autoClose.close();
       }
   }
}

Things to note:

• your code swallows the original exception from the try block in case closing the resource fails. The original exception is surely more important for diagnostic;
• in the ARM idiom above, closing the resource is done differently depending on whether there already was an exception in the try-block. If try completed normally, then the resource is closed outside any try-catch block, naturally propagating any exception.

Grimalkin

Answer 2

Generally, methods in the finally block are 'cleanup' codes (Closing the Connection, etc) which the user does not necessarily need to know.

What I do for these exceptions is to absorb the exception, but log the details.

finally{
   try{
      connection.close();
   }catch(SQLException e){
      // do nothing and just log the error
      LOG.error("Something happened while closing connection. Cause: " + e.getMessage());
   }
}

Gran

Answer 3

You're getting a warning because this code could potentially throw an exception while dealing with a thrown exception. You can use the try with resource syntax to close the resource automatically. Read more here.

In the case that the "request failed : " exception is thrown and you fail to close the httpclient, the second exception is the one that would bubble up.

Weakness

Answer 4

I am not sure how we can achieve same functionality without having throw clause.

You could nest the two try blocks differently to achieve the same result:

HttpClient httpClient = null; // initialize
try {
    try {
        // do something with httpClient
    } catch(Exception e) {
        throw new MyException("request failed : ", e);
    } finally {
        httpClient.close();
    }
} catch (IOException e) {               
    throw new MyException("failed to close server conn: ", e);
}

Serica