views:

290

answers:

5

In java, I want to log the exception being thrown from a piece of code:

try
{
    doRiskyThing();
}
catch(Throwable t)
{
    log.warn("too bad");
    throw t;
}

The problem is that now my method must declare it throws throwable.

On the other hand, if I log the exception in a finally block, how do I know what exception is being thrown (and if any exception is being thrown at all).

A: 

This seems to be the wrong place to log the exception. Either log it within doRiskyThing() when throwing it, or log it where it is supposed to be captured.

Zed
A: 

This should work:

try
{
    doRiskyThing();
}
catch(RuntimeException e)
{
    log.warn("too bad");
    throw e;
}
catch(Exception e)
{
    log.warn("too bad");
    throw new RuntimeException(e);
}
catch(Error e)
{
    log.warn("too bad");
    throw e;
}

But it's ugly and I'd rather have a single catch block that catches everything and logs it at the top level of the app, with no re-throwing. The stack trace lets you pinpoint the problem anyway.

Michael Borgwardt
A: 

You can log the Throwable, and throw a RuntimeException.

In Java, you have to declare any exception that your method might itself throw, because it's considered part of the method's signature.... that is, apart from RuntimeException, which avoids that rule. The RuntimeException can give the message of the Throwable as part of its message, if you like.

try
{
    doRiskyThing();
}
catch(Throwable t)
{
    log.warn("too bad, exception thrown: " + t.getMessage());
    throw new RuntimeException("Throwable exception was thrown, message: " + t.getMessage());
}

Beware however that this may be considered bad practice, and for more long-term code it's recommended that you throw a non-runtime exception and define it in the method's signature.

Jez
Regarding "bad practice", actually, the trend is away from checked exceptions in favor of unchecked ("runtime") exceptions, and you can still define runtime exceptions in the methods signature.
SingleShot
@SS one reason that this is a really bad idea is that wrapping in a RuntimeException is CHANGING the exception that is propagated. This makes it difficult for something lower down the stack to deal with the exception.
Stephen C
+3  A: 

You could at least write a catch for unchecked and n catch blocks for checked exceptions.

try{
    ..
}catch(RuntimeException e){
   log(e);
   throw e;
}catch(ExceptionException ..){
   log(e);
   throw e;
}

This will not change the signature of your method. Catching Errors is a bit of a smell. Well, logging and throwing exception is a smell, too. This will duplicate error messages.

Advance exception handling is a feature that was removed from project coin in JDK 7.

Thomas Jung
A: 

Catching Throwable in itself is already bad practice (and flagged as such by many code quality checking tools). Wrapping it in a RuntimeException and rethrowing it? Terrible! The whole point of having an exception hierarchy is undermined by this.

The 'trend' away from checked exceptions is actually a bit more subtle. Between architectural layers (or for example at a framework API boundary) it is common practice to define one or more meaningful exception types and wrap any implementation-specific errors in them (take a look at Spring or Hibernate for example).

Making exceptions checked or not is a matter of style. If there is a good chance that the caller will be willing and able to handle the error condition then I make it checked (e.g. concurrent update detected), otherwise unchecked (e.g. database connection error). Unchecked exceptions should 'bubble' up through your application code (i.e. not be caught) and handled at a high level by displaying some sort of error page/screen.

Adriaan Koster