On Apr 6, 2010, at 12:51 AM, Scott Gray wrote:

> On 5/04/2010, at 11:38 PM, Adrian Crum wrote:
> 
>> --- On Mon, 4/5/10, Adam Heath <[email protected]> wrote:
>>>> 
>>> 
>>> I just fixed the first.
>>> 
>>> The others should have their logging fixed, don't change
>>> the log level
>>> for run-tests.
>>> 
>>> If low-level code logs an error, and rethrows, then just
>>> stop logging
>>> the error.
>> 
>> Another pattern that bugs me:
>> 
>> try {
>>   ...
>> } catch (Exception e) {
>>   Debug.logError(e, "Something went wrong: " + e.getMessage(), module);
>> }
>> 
>> The e.getMessage isn't necessary - the message will be printed in the stack 
>> trace.
>> 
>> -Adrian
> 
> That's nothing, check out the 80-odd occurrences of this:
> try {
>    ...
> } catch (Exception e) {
>    ServiceUtil.returnError(e.getMessage);
> }
> 
> Not really logging related but wow, 80 occurrences?

This is a good example of error hiding, kind of the opposite of logging an 
exception multiple times... and much worse as with this pattern there's a good 
chance you'll never find the source of the error without making a code change 
to fix the error hiding.

-David

Reply via email to