dev@glassfish.java.net

Re: handling unexpected exceptions

From: Bill Shannon <bill.shannon_at_sun.com>
Date: Thu, 14 May 2009 10:40:15 -0700

Craig L Russell wrote:
>
> On May 14, 2009, at 9:06 AM, Bill Shannon wrote:
>
>> Craig L Russell wrote:
>>> On May 14, 2009, at 12:21 AM, Bill Shannon wrote:
>>>> Craig L Russell wrote:
>>>>> Hi Bill,
>>>>> On May 13, 2009, at 5:39 PM, Bill Shannon wrote:
>>>>>> I have a bunch of code in JavaMail that looks roughly like this:
>>>>>>
>>>>>> try {
>>>>>> // do something that might fail for unknown reasons
>>>>>> } catch (Exception ex) {
>>>>>> // clean up, close connections, etc.
>>>>>> throw MyException();
>>>>>> // or...
>>>>>> return null;
>>>>>> // or...
>>>>>> continue;
>>>>>> }
>>>>>>
>>>>>> The "do something" can fail for lots of reasons - NPE,
>>>>>> IOException, etc. -
>>>>>> usually several levels deep. No matter what goes wrong I want to
>>>>>> detect
>>>>>> the failure and clean things up, especially making sure not to
>>>>>> leave any
>>>>>> connections open.
>>>>>>
>>>>>> FindBugs complains about this because "do something" doesn't throw
>>>>>> Exception.
>>>>>>
>>>>> If do something doesn't throw Exception, you can catch
>>>>> RuntimeException or Throwable depending on whether you're prepared
>>>>> for just RuntimeException or Error conditions.
>>>>
>>>> Often "do something" throws some checked exception, so I need to handle
>>>> it *and* the possible RuntimeExceptions.
>>> This seems to be the crux of the FindBugs complaint.
>>> So IOException gets wrapped in RuntimeException which catch
>>> (RuntimeException) would catch and FindBugs would not complain.
>>
>> No, "do something" often is declared to throw a checked exception.
>
> If FindBugs complains that you are catching Exception instead of
> catching the specific checked exception, that's a bug in FindBugs IMHO.
> No one should be forced to catch every specific exception if they all
> have common treatment (clean up, wrap, and throw).

It's a medium priority error, so it's in the "usually correct" category.

But in most cases it *is* a mistake, hiding exceptions that should be
visible to higher layers.

In this case I'm asking not just because I'm trying to make FindBugs
shut up about this, but because the FindBugs complaint got me to thinking
whether there might be a better way to handle this common case. If the
consensus is that catching Exception is better than using finally in this
case, I'll just exclude this FindBugs complaint.