Re: [xwiki-devs] [xwiki-notifications] r4689 - xwiki-platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/store - xwiki-devs@xwiki.org

2 Sep 2007

Hi Sergiu,

On Sep 2, 2007, at 3:37 AM, Sergiu Dumitriu wrote:

[snip]
...
           catch (Exception e) {
 -            if ( log.isErrorEnabled() ) log.error("Failed updating  
 schema: " + e.getMessage());
 +            if ( log.isErrorEnabled() ) {
 +                log.error("While executing query: " + sql);
 +                log.error("Failed updating schema: " + e.getMessage 
 ());
 +            }
          }
          finally {
              try { 
I have 2 comments:

* I think a single line is better as having the same sentence on 2  
differents log statements is strange. Something like:

"Failed updating schema while executing query [" + sql + "]"

* I think we need to pass "e" and not "e.getMessage()" so that we log 

the wrapped exceptions if any and get to the root of the problem.  
Especially as this is an error and I think we need to have stack  
traces for errors.

So I propose something like:

log.error("Failed updating schema while executing query [" + sql +  
"]", e);

WDYT?

Note: I had meant to improve storage errors in the past, see http:// 
jira.xwiki.org/jira/browse/XWIKI-466

Thanks
-Vincent

_______________________________________________
devs mailing list
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs