Why the enclosing "finally" block should not be exited with a "throw" statement?

Question

I am doing code review using Jtest. JTest report states "The enclosing "finally" block should not be exited with a "throw" statement". Whats the reason? & how can I fix it? I am wrapping exception in a custom exception & the method has throws.

Codes looks like:

public HashMap methodName(Connection conn, HashMap hMap) throws MyCustomException {
 try
 {}
 catch(SQLException)
 {}
 catch(Exception)
 {}
 finally
 {
   try
   {}
   catch(SQLException e)
   {
     mLog.fatal("Error Msg", e);
     throw new MyCustomException("msg", e);
   }
 }
}

What do you think will happen if the closing `finally` throws? — Boris the Spider, Jan 08 '18 at 10:13
Have a look at this question : https://softwareengineering.stackexchange.com/questions/188858/throwing-an-exception-inside-finally — Arnaud, Jan 08 '18 at 10:15

score 0 · Answer 1 · answered Jan 08 '18 at 10:16

0

This is because throwing an exception in a finally block will cause exception to be propagated further(beyond the try-catch block). This in some sense defeats the purpose of the try catch block. However, there might be cases where it might actually help.

answered Jan 08 '18 at 10:16

MozenRath

9,652
13
61
104

1

"_there might be cases where it might actually help_" [citation needed] – Boris the Spider Jan 08 '18 at 10:47
if you need to generate a new kind of error after catching the original error and this kind of error could come irrespective of an exception in the try block, then it will help. But that is a very niche use case that can probably be avoided. – MozenRath Jan 08 '18 at 12:19

score 0 · Answer 2 · answered Jan 08 '18 at 11:09

It will hide the original exception.

As one method call can only throw one exception, the exception from the finally block will completely replace and hide an exception from the main part of your method.

Do you want to throw the finally exception at all?

Not doing so makes things much easier and often is a correct decision.

Typically, finally blocks are meant to close some resource. So, if the method body was able to read the result asked for, I'd regard that a success, even if the method wasn't able to close some resource, so I'd log that as a warning and continue (and then the report will be happy as well).

But maybe you decide the other way: if something goes wrong in the finally block, you see that as such a failure of the methodName() call that you have to signal that as a failure to your caller. Then you can end up with two competing exceptions and you have to decide which one to throw to your caller. And you should log the other one, so it isn't completely lost.

The main-body one typically is the more interesting one, but isn't easily accessible: you have to introduce an Exception variable outside the try-catch-finally block, initialized to null, where you store any main-body exception, so you can throw that one from the finally block. The resulting code will be weird...

So I'd stay with your approach (if you decide an exception in the finally block to be a failure of the complete method), just make sure the original exception is logged. This will lead to double log entries, which should be avoided, but better logged twice than never.

Why the enclosing "finally" block should not be exited with a "throw" statement?

2 Answers2