The Semi-Swallowed Exception

I am often brought in to help a team track down super-hard-to-find bugs. One situation that results in the super-hard-to-find bug is what I call the semi-swallowed-exception.

Most java developers know that it's a very bad idea (99% of the time) to completely swallow an exception like this:

try{
    ...
    ...
}catch(Exception e){}

That is, you very rarely want an empty catch block.

But almost as troublesome is this:

try{
    ...
    ...
}catch(Exception e){
    log.error(e);
}

The problem is, that the exception probably left the application in an invalid state. For example, some variable wasn't set correctly. Then, sometime later, you get a null pointer exception or worse, some strange hard to explain bug.

Based on the projects I have been involved with, I would say that 90% of time when you find yourself wanting to do this:

try{
    ...
    ...
}catch(Exception e){
    log.error(e);
}
...

Instead, you probably should do this:


try{
    ...
    ...
}catch(Exception e){
    log.error(e);
    throw new IllegalStateException("bla bla",e);
}

Also, if you are just catching an exception from a lower layer, there is not much value in wrapping it in a do nothing exception class. For example, I came across this the other day:


try{
    ...
    ...
}catch(Exception ex{
    log.error(ex);
    throw new MyCompanyNameException("bla bla");
}


Two things to note about this snippet:

1. The MyCompanyNameException class adds no value in this case. This seems to be a popular thing to do. But I can see no point to it.
    
2. This exception handler actually swallows information.The MyCompanyNameException includes a string message ("bla bla"). But it swallows the cause exception. So I would replace the above exception handler with this:

try{
    ...
    ...
}catch(Exception ex){
    log.error(ex);
    throw new RuntimeException("bla bla",ex);
}