is catch and re-throw a good idea?

[Andy Fish]

Hi,

I have a scenario where I need to "undo" some processing when an exception
occurs but I don't want to otherwise interfere with the handling of the
exception. one way seems to be catching and re-throwing it thus:

try {
action 1;
action 2;
} catch (Exception ex) {
if (action 1 completed) {
undo action 1
}
throw ex
}

another way would be using the finally clause. This seems structurally
better because I'm not interfering with the exception itself. However,
because it gets executed even if there hasn't been an exception, I need to
use an extra local variable to figure out whether there was actually an
exception at all. maybe I am abusing the finally block to do exception
handling?

try {
action 1;
action 2;
finished=true;
} finally {
if (!finished) {
if (action 1 completed) {
undo action 1
}
}
}

anyone have any thoughts on which is best? is there a common idiom for this?

Andy

 

[Niels Dybdahl]

I have a scenario where I need to "undo" some processing when an exception

occurs but I don't want to otherwise interfere with the handling of the
exception. one way seems to be catching and re-throwing it thus:

try {
action 1;
action 2;
} catch (Exception ex) {
if (action 1 completed) {
undo action 1
}
throw ex
}

I would write it like this:

try {
action1;
}
catch (Exception ex) {
undoAction1;
throw ex;
}
action2;

Then there is no need to keep track of whether action1 has completed or not.
You could make a separate handling of action2.

Niels Dybdahl

 

[Andy Fish]

I would write it like this:

try {
action1;
}
catch (Exception ex) {
undoAction1;
throw ex;
}
action2;

Then there is no need to keep track of whether action1 has completed or not.
You could make a separate handling of action2.

maybe I wasn't clear enough. I have assumed that both action 1 and 2 are
atomic. If action 2 throws an exception I want to undo action 1. In general
there may be many actions

In my case, I accept the need to keep track of which actions have completed
(and therefore need to be undone in the event of an error); I was more
concerned with whether it is better to re-throw an exception or to do the
undoing in a finally block

 

[Niels Dybdahl]

maybe I wasn't clear enough. I have assumed that both action 1 and 2 are

atomic. If action 2 throws an exception I want to undo action 1. In general
there may be many actions

In that case your solution is ok.

In my case, I accept the need to keep track of which actions have completed
(and therefore need to be undone in the event of an error); I was more
concerned with whether it is better to re-throw an exception or to do the
undoing in a finally block

It is more clear, if you do it in a catch block than in a finally block as
the cleanup is only done on errors.
I have not done any performance tests on the two solutions, but throws
usually happen seldom, so handling them should not be a performance issue.

Niels Dybdahl

 

[antroy]

Hi,

I have a scenario where I need to "undo" some processing when an exception
occurs but I don't want to otherwise interfere with the handling of the
exception. one way seems to be catching and re-throwing it thus:

try {
action 1;
action 2;
} catch (Exception ex) {
if (action 1 completed) {
undo action 1
}
throw ex
}

In my opinion this is the better way. 'finally' is designed as a way to
get a bit of code to run regardless of whether an exception was thrown
or not.

Your code only needs to be run if an exception occurs, so catch is the
right place to do it. The fact that you need to rethrow the exception
doesn't alter this.

 

[Bent C Dalager]

It is more clear, if you do it in a catch block than in a finally block as
the cleanup is only done on errors.
I have not done any performance tests on the two solutions, but throws
usually happen seldom, so handling them should not be a performance issue.

As I understand it, neither "throw" nor "catch" cost much. What does
cost is "new Exception()" since that has to create a stacktrace.

Cheers
Bent D

 

[Andy Fish]

In my opinion this is the better way. 'finally' is designed as a way to
get a bit of code to run regardless of whether an exception was thrown
or not.

Your code only needs to be run if an exception occurs, so catch is the
right place to do it. The fact that you need to rethrow the exception
doesn't alter this.

hmmm, I tried this method but I got stuck and currently have it implemented
using 'finally'.

the problem with the 'catch' method is that I have to catch every possible
type of exception, even unchecked ones, so I have to catch Throwable.
Obviously I don't want to declare my method as 'throws Exception' so in
order to re-throw it I have to say:

if (ex instanceof RuntimeException) {
throw (RuntimeException) ex;
} else if (ex instanceof Error) {
throw (Error) ex;
} else if (ex instanceof myExceptionClass) {
throw (myExceptionClass) ex;
}

which is quite messy.

It almost seems like we need a new language construct, something that is a
kind of event notification that an exception occured. 'Catch' carries with
it the inference that I'm going to actually handle the exception (which I'm
not)

Andy

 

[Timo Kinnunen]

As I understand it, neither "throw" nor "catch" cost much. What does
cost is "new Exception()" since that has to create a stacktrace.

The stacktrace is created when throwing, because you can create a new
exception, store it and throw it later.

 

[Suresh]

hmmm, I tried this method but I got stuck and currently have it implemented
using 'finally'.

the problem with the 'catch' method is that I have to catch every possible
type of exception, even unchecked ones, so I have to catch Throwable.
Obviously I don't want to declare my method as 'throws Exception' so in
order to re-throw it I have to say:

if (ex instanceof RuntimeException) {
throw (RuntimeException) ex;
} else if (ex instanceof Error) {
throw (Error) ex;
} else if (ex instanceof myExceptionClass) {
throw (myExceptionClass) ex;
}

which is quite messy.

It almost seems like we need a new language construct, something that is a
kind of event notification that an exception occured. 'Catch' carries with
it the inference that I'm going to actually handle the exception (which I'm
not)

Andy

Thats not the way to handle the exceptions ...

What book are you reading ?

try{
// Statements ...

}catch (RuntimeException ex) {
// Write logs
// Clean up
throw ex;
}catch(Error ex) {
// Write logs
// Clean up
throw ex;
}catch(myExceptionClass ex) {
// Write logs
// Clean up
throw ex;
}

 

[antroy]

Andy Fish wrote:
....

the problem with the 'catch' method is that I have to catch every possible
type of exception, even unchecked ones, so I have to catch Throwable.
Obviously I don't want to declare my method as 'throws Exception' so in
order to re-throw it I have to say:

if (ex instanceof RuntimeException) {
throw (RuntimeException) ex;
} else if (ex instanceof Error) {
throw (Error) ex;
} else if (ex instanceof myExceptionClass) {
throw (myExceptionClass) ex;
}

Have you checked to see that this is necessary? Assuming your action1
and action2 throw a MyException, all you need to declare is that the
method throws a MyException, and catch and re-throw the Throwable and
polymorphism should take care of the rest; declaring that you are
catching a Throwable doesn't stop the exception from being a MyException
nor make RuntimeExceptions or Errors checked:

void method1() throws MyException {

try {
action 1;
action 2;
} catch (Throwable ex) {
if (action 1 completed) {
undo action 1
}
throw ex
}
}

 

[Bent C Dalager]

The stacktrace is created when throwing, because you can create a new
exception, store it and throw it later.

The following program:

public class Test
{
static Exception m_ex;

public static void main(String[] args)
throws Exception
{
makeException();
throwException();
}

static void makeException()
{
m_ex = new Exception();
}

static void throwException()
throws Exception
{
throw m_ex;
}
}

Produces the following output:

Exception in thread "main" java.lang.Exception
at Test.makeException(Test.java:14)
at Test.main(Test.java:8)

which seems to indicate that the stacktrace is determined upon
creation of the exception, not at the point where it is thrown.

Cheers
Bent D

 

[Bent C Dalager]

Have you checked to see that this is necessary? Assuming your action1
and action2 throw a MyException, all you need to declare is that the
method throws a MyException, and catch and re-throw the Throwable and
polymorphism should take care of the rest; declaring that you are
catching a Throwable doesn't stop the exception from being a MyException
nor make RuntimeExceptions or Errors checked:

The below code doesn't compile because

Test.java:19: unreported exception java.lang.Throwable; must be caught or declared to be thrown
throw th;
^
1 error

If, however, I change the "catch (Throwable th)" to
"catch (IOException th)", it compiles.


import java.io.*;
public class Test
{
public static void main(String[] args)
throws Exception
{
throwIt();
}

static void throwIt()
throws IOException
{
try
{
new FileInputStream("nonexist");
}
catch (Throwable th)
{
throw th;
}
}
}

Cheers
Bent D

 

[Andy Fish]

Thats not the way to handle the exceptions ...

What book are you reading ?

try{
// Statements ...

}catch (RuntimeException ex) {
// Write logs
// Clean up
throw ex;
}catch(Error ex) {
// Write logs
// Clean up
throw ex;
}catch(myExceptionClass ex) {
// Write logs
// Clean up
throw ex;
}

that is another way of doing the same thing, but is even more verbose becaue
I have to either repeat the clean-up code several times or refactor it out
into a procedure

the thing I did with the casting is a shortcut because it means you can get
away with only one catch block

 

[Roedy Green]

The stacktrace is created when throwing, because you can create a new
exception, store it and throw it later.

You can also make it take a stacktrace snapshot any time you want.
Try this "I am here" code inserted anywhere into a program:


Throwable t = new Throwable() ;
StackTraceElement[] es = t.getStackTrace() ;
for ( int i=0 ; i<es.length ; i++ )
{
StackTraceElement e = es[i ];
System.out.println (
" in class:" + e.getClassName()
+ " in source file:" + e.getFileName()
+ " in method:" + e.getMethodName()
+ " at line:" + e.getLineNumber()
+ " " + ( e.isNativeMethod() ? "native" : "" ) );
}

 

[Timo Kinnunen]

The stacktrace is created when throwing, because you can create a new
exception, store it and throw it later.

The following program:
[snip]

Produces the following output:

Exception in thread "main" java.lang.Exception
at Test.makeException(Test.java:14)
at Test.main(Test.java:8)

which seems to indicate that the stacktrace is determined upon
creation of the exception, not at the point where it is thrown.

You're right, but I'm partly right too, AFAICT. In J2SE, Throwable's
constructors call fillInStackTrace(), but J2ME's (CLDC 1.0's) Throwable
doesn't have that method. What I'm guessing is happening is that the
throw-statement fills in the stacktrace if it hasn't been filled already.
In J2ME, the following

new Exception().printStackTrace();

doesn't print any stacktrace, so I have to do

try {
throw new Exception();
} catch(Exception e) {
e.printStackTrace();
}

to get it filled in. My intuition says that throw should fill in the
stacktrace regardsless.

 

[Suresh]

Have you checked to see that this is necessary? Assuming your action1
and action2 throw a MyException, all you need to declare is that the
method throws a MyException, and catch and re-throw the Throwable and
polymorphism should take care of the rest; declaring that you are
catching a Throwable doesn't stop the exception from being a MyException
nor make RuntimeExceptions or Errors checked:

The below code doesn't compile because

Test.java:19: unreported exception java.lang.Throwable; must be caught or declared to be thrown
throw th;
^
1 error

If, however, I change the "catch (Throwable th)" to
"catch (IOException th)", it compiles.


import java.io.*;
public class Test
{
public static void main(String[] args)
throws Exception
{
throwIt();
}

static void throwIt()
throws IOException
{
try
{
new FileInputStream("nonexist");
}
catch (Throwable th)
{
throw th;
}
}
}

Cheers
Bent D

import java.io.*;
public class Test
{
public static void main(String[] args)
throws Throwable
{
throwIt();
}

static void throwIt()
throws Throwable
{
try
{
new FileInputStream("nonexist");
}
catch (Throwable th)
{
throw th;
}
}
}



Should work now ....

 

[Bent C Dalager]

Should work now ....

However, the OP didn't want to throw "Throwable", for understandable
reasons

Cheers
Bent D

 

[Roedy Green]

public static void main(String[] args)
throws Exception

this is a bit of a silly thing to bother saying since every method can
potentially throw some exception, e.g. NullPointerException.
You only have to declare the "unusual" exceptions in your throws
clause -- i.e. checked exceptions in Java parlance.

see http://mindprod.com/jgloss/exception.html

static void throwIt()
throws IOException

this is a "lie". The method as a whole can never throw an IOException
because you catch it and throw a Throwable instead

ml