Scott Oaks wrote:
> 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...
In my case I was never going to expose original exception to the caller.
The caller wants to know that the connection failed, not that there was
a programming error in the connection provider that caused it to throw
NullPointerException.