Sylvain Wallez wrote:
Why such a complicated exception handling?
Same as before.
I see several problems here:
- SourceNotFoundException is a nominal case (BTW why not using
Source.exists()?) and should not log an exception
* Source.exists() will not work here:
SitemapSource:
/**
* Returns true always.
* @see org.apache.excalibur.source.Source#exists()
*/
public boolean exists() {
return true;
}
BlobSource:
/**
* @see org.apache.excalibur.source.Source#exists()
*/
public boolean exists() {
// FIXME
return true;
}
* Exception is *not* logged (in the INFO level, that is):
+ } else if (getLogger().isInfoEnabled()) {
+ getLogger().info("Bundle <" + sourceURI + "> not loaded: Source
URI not found");
+ }
* Exception *must* be logged if Category is in DEBUG
level - exactly what you would expect if you are trying to
debug i18n component. Anything else is counter productive
and counter intuitive (what you would do if you need to see
those exeptions? Set a breakpoint? Patch implementation?
Roll your own implementation?)
- Bad formed XML files and other serious exceptions are semi-silently
ignored. By semi-silently, I mean they're just logged and don't bubble
up higher in the call stack, thus giving the false impression that the
system works.
Such exceptions must not bubble up upstream: if exception is let through, your
whole site goes down simply due to single bug in i18n catalogue. With existing
exception handling, i18n (and your whole site) continues functioning with older
version of the catalogue, but reports an error into the log file (that's what
you've got monitoring for). That's the i18n behaviour as it was originally
designed. See "Keep existing loaded values" comment.
OTOH, if source was deleted, it wipes out old values, as that is what you are
expecting it to do in case you want to delete existing catalogue.
I will fix the 1st point (it scares my users to see all this in the
logs), and would really understand the rationale for the second.
If "fix" means always drop original exception, -1: Original exception must be
logged somewhere in case DEBUG is turned on (tell your users to use INFO or WARN
level - DEBUG is for *debugging*).
Vadim
