[xwiki-devs] [xwiki-notifications] r4689 - xwiki-platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/store

[Vincent Massol]

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 at xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs