jackrabbit-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Felix Meschberger <fmesc...@gmail.com>
Subject Re: Logging of exceptions
Date Thu, 09 Aug 2007 06:11:33 GMT
Hi,

I agree, that the cause must not be lost. But rather than logging the
cause inside loadBundle (in this case), I suggest the
hasNonVirtualItemState method should not ignore the exception and log
it.

IMHO, if an exception is thrown, the same message should not be logged
by the thrower, because the thrower already informs about the situation.
Rather the catcher of the exception should handle it by logging or
rethrowing.

The problem on the other hand is, that ItemStateException does not
always relate a problem: Sometime it is thrown IIRC if an item state is
just not availabel and sometimes - as here - it is thrown as a
consequence of a possibly real problem.... This distinction can probably
not be made by the catcher of the exception ....

Regards
Felix

Am Donnerstag, den 09.08.2007, 01:05 +0200 schrieb Christoph Kiehl:
> Hi,
> 
> while preparing the testcase for JCR-1039 I found a construct like this:
> 
> protected synchronized NodePropBundle loadBundle(NodeId id)
>          throws ItemStateException {
>      [...]
>      } catch (Exception e) {
>          String msg = "failed to read bundle: " + id + ": " + e;
>          log.error(msg);
>          throw new ItemStateException(msg, e);
>      } finally {
>      [...]
> }
> 
> In the calling method the exception is handled like this:
> 
> private boolean hasNonVirtualItemState(ItemId id) {
>      [...]
>      } catch (ItemStateException ise) {
>          return false;
>      }
>      [...]
> }
> 
> The result is that you will never find the root cause "e" in the log. 
> This makes it hard to diagnose bugs without using a debugger or 
> modifying the source code.
> I would suggest to use log.error(msg, e) instead of log.error(msg). I 
> would even consider log.error(msg) to be harmful since there is almost 
> always an exception which should be logged if you use log.error().
> WDYT?
> 
> Cheers,
> Christoph
> 


Mime
View raw message