On 05/14/09 13:40, Bill Shannon wrote:
> 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.
I was just going to bite my tongue, but I will argue that the finally
clause is far, far better. Exception chaining is a frustrating thing; it
makes root cause analysis very difficult. I definitely agree that in
most cases it is a mistake...
What caused my to put my $.02 after all? I'm just looking right now at
some appserver code where something threw a NullPointerException, which
was wrapped in a JDOFatalInternalException, which was wrapped in an
EJBException, which was wrapped in a TransactionRolledBackException,
which was wrapped into another EJBException. It should be trivial to
track down the NPE, but almost all information about it is lost (except
for the location where it was caught, which is something I guess...).
-Scott