cocoon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vadim Gritsenko <va...@reverycodes.com>
Subject Re: svn commit: r312968 - in /cocoon/branches/BRANCH_2_1_X: ./ src/java/org/apache/cocoon/components/source/impl/validity/ src/java/org/apache/cocoon/i18n/ src/java/org/apache/cocoon/transformation/ src/webapp/WEB-INF/ src/webapp/samples/i18n/translations/
Date Mon, 24 Oct 2005 20:49:30 GMT
Sylvain Wallez wrote:
> Vadim Gritsenko wrote:
> 
>> Sylvain Wallez wrote:
>>
>>> Vadim Gritsenko wrote:
> 
>>>>> - 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.
>>>
>>> Ok. So you mean that i18n allows broken message files to exist?
>>
>> Exactly.
> 
> Wow. I really dislike that.

Well I can see that in dev environment you would prefer to get error right away, 
but historically i18n was allowing content editors to push broken XML files and 
site admins to scream on editors, while all other users can continue to use 
older version of a site.

I would prefer to keep this functionality, but for development environment you 
can make it configurable, by adding a parameter, something like

   <i18n
     <fail-safe>false</fail-safe>


<snip/>
>> I guess it takes some getting used to it.
> 
> C'mon! What does it mean "getting used to it"? If it's broken, let's fix 
> it!! It's too late for 2.1.8, but I'd like this subject to be discussed, 
> as it really seems a bad thing to me.

It's not broken; and it can be improved - see above.


>> More general note - ignored exceptions is the single most frustrating 
>> experience you can have with Cocoon in particular and Java in general. 
>> Hence I'm proponent of having the ability to see exception if so desired.
> 
> Me too, but in this particular case, most exceptions will just say that 
> the source doesn't exists.

Agreed. Hence there is INFO level :-P


>>> SNFE is used here as a substitute for source.exist(), probably 
>>> because two implementations don't have a proper implementation for 
>>> it. Better fix the implementations or log the exception only if 
>>> source.exists() returns true rather than fill the logs with 
>>> meaningless exceptions.
>>
>> Won't argue with that. OTOH, there might be broken sources out there 
>> where even if source.exists() it can still throw SNFE.
>>
>> You also have to take into an account a situation where source WAS 
>> existing at the moment of .exists(), but was removed before you tried 
>> to .getInputStream() it. So, SNFE handling still has to be present.
> 
> Ok, so what about :
> 
> catch (SNFE snfe) {
>    if (!source.exists()) {
>        getLogger.info("bundle " + source.getURI() + " doesn't exist");
>    } else {
>        getLogger.info("bundle " + source.getURI() + " is said to exist 
> but could not be loaded", sfne);
>    }
> }
> 
> That way, we avoid logging an exception that just says that the source 
> doesn't exist, but still log it whenever there is an inconsistency 
> between exists() and getInputStream(), whatever its cause.
> 
> Deal?

No objections to this one.

Vadim

Mime
View raw message