Logging of exceptions

[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