You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Simon Kitching <si...@ecnetwork.co.nz> on 2003/10/27 23:41:25 UTC

Re: [digester] assertion mechanism throws Error [was: plugins module ready for evaluation]

On Tue, 2003-10-28 at 02:53, robert burrell donkin wrote:
> On Sunday, October 5, 2003, at 10:53 PM, Simon Kitching wrote:
> 
> > On Sun, 2003-10-05 at 00:43, robert burrell donkin wrote:
> 
> <snip>
> 
> >> 3. Exception handling
> >>
> >> PluginAssertionError extends Error. i'm not very happy about this. IMHO 
> >> a
> >> well behaved component should not throw any Error subclasses. i'd be
> >> happier for PluginAssertionError to be renamed PluginAssertionException
> >> and extend RuntimeException.
> >
> > Having PluginAssertionError extend Error was a very deliberate choice.
> > It is modelled after the java 1.4 assertion mechanism, where an
> > assertion failure generates AssertionError.
> >
> > PluginAssertionError is never thrown due to a mistake on the part of the
> > user of the Plugins module, nor due to a mistake in the xml of any sort.
> > It is only thrown when a bug in the plugins library is detected.
> 
> i probably should clarify my opinion: no well behaved component should 
> throw an Error in production. in many context's the throwing of an Error 
> in production can be pretty serious. in a J2EE context, a container may 
> mark a bean or a thread which throws an Error as dead. in a production 
> environment, this may cause problems with these pooled resources.
> 
> in a way, though finding a bug in the plugins library in production may 
> irritate a user a little, i'm pretty sure that they will be very angry if 
> a bug in the plugins library causes their J2EE application to fail through 
> resources starvation. so, i'm not too sure what the best way to handle 
> this situation is but i don't think that throwing an Error is the right 
> thing to do. maybe, it could be replaced with a runtime.

I see your point. However it seems to me that what you are arguing is
that the *official* java assertion mechanism is fatally flawed, and will
never be acceptable in Apache code. That might be right, but is a pretty
significant decision.

I just feel that the fellows who designed java assertions are pretty
smart, and must have had a reason for deciding to define AssertionError
instead of AssertionException. Rather than assume I'm smarter than them,
I followed their lead on this issue.

Do you think it is worth opening up this discussion more widely on
commons-dev? What do the Avalon or Tomcat people think about the
java 1.4 assertion model?

I understand it is not a pressing problem for Apache developers as
assertions require a minimum of java1.4; while 1.2 or 1.3 support is
required of apache projects it is only a theoretical problem - unless
there are people like me who are fans of assertions.

I'm happy to change to using a RuntimeException subclass if that is the
general consensus. 

Simon


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [digester] assertion mechanism throws Error [was: plugins module ready for evaluation]

Posted by "Craig R. McClanahan" <cr...@apache.org>.
Simon Kitching wrote:

>On Tue, 2003-10-28 at 02:53, robert burrell donkin wrote:
>  
>
>>On Sunday, October 5, 2003, at 10:53 PM, Simon Kitching wrote:
>>
>>    
>>
>>>On Sun, 2003-10-05 at 00:43, robert burrell donkin wrote:
>>>      
>>>
>><snip>
>>
>>    
>>
>>>>3. Exception handling
>>>>
>>>>PluginAssertionError extends Error. i'm not very happy about this. IMHO 
>>>>a
>>>>well behaved component should not throw any Error subclasses. i'd be
>>>>happier for PluginAssertionError to be renamed PluginAssertionException
>>>>and extend RuntimeException.
>>>>        
>>>>
>>>Having PluginAssertionError extend Error was a very deliberate choice.
>>>It is modelled after the java 1.4 assertion mechanism, where an
>>>assertion failure generates AssertionError.
>>>
>>>PluginAssertionError is never thrown due to a mistake on the part of the
>>>user of the Plugins module, nor due to a mistake in the xml of any sort.
>>>It is only thrown when a bug in the plugins library is detected.
>>>      
>>>
>>i probably should clarify my opinion: no well behaved component should 
>>throw an Error in production. in many context's the throwing of an Error 
>>in production can be pretty serious. in a J2EE context, a container may 
>>mark a bean or a thread which throws an Error as dead. in a production 
>>environment, this may cause problems with these pooled resources.
>>
>>in a way, though finding a bug in the plugins library in production may 
>>irritate a user a little, i'm pretty sure that they will be very angry if 
>>a bug in the plugins library causes their J2EE application to fail through 
>>resources starvation. so, i'm not too sure what the best way to handle 
>>this situation is but i don't think that throwing an Error is the right 
>>thing to do. maybe, it could be replaced with a runtime.
>>    
>>
>
>I see your point. However it seems to me that what you are arguing is
>that the *official* java assertion mechanism is fatally flawed, and will
>never be acceptable in Apache code. That might be right, but is a pretty
>significant decision.
>
>I just feel that the fellows who designed java assertions are pretty
>smart, and must have had a reason for deciding to define AssertionError
>instead of AssertionException. Rather than assume I'm smarter than them,
>I followed their lead on this issue.
>
>Do you think it is worth opening up this discussion more widely on
>commons-dev? What do the Avalon or Tomcat people think about the
>java 1.4 assertion model?
>
>  
>
As a (basically) former Tomcat person, I expect most Tomcat developers 
would consider the assertion mechanism irrelevant at this point in time, 
because Tomcat needs to run on JDK 1.3 (and even 1.2).

>I understand it is not a pressing problem for Apache developers as
>assertions require a minimum of java1.4; while 1.2 or 1.3 support is
>required of apache projects it is only a theoretical problem - unless
>there are people like me who are fans of assertions.
>
>  
>
I'm *not* a fan of assertions in my production code, at least partly 
because of this issue.  I'm a big fan of assertions (JUnit style) in my 
unit tests.

>I'm happy to change to using a RuntimeException subclass if that is the
>general consensus. 
>
>  
>
+1.

IMHO, things like IllegalArgumentException and NullPointerExcepton are 
not "Errors" for a reason; they are "Exceptions" -- caused by either the 
user botching their use of your class, or your class botching its 
specified contracts.  In most cases, they represent conditions that you 
should be able to, or should at least try to, recover from.

Error is (to quote the Javadocs) "a subclass of Throwable that indicates 
serious problems that a reasonable application should not try to 
catch."  Lumping AssertionError into that definition is pretty 
problematic -- I'd probably never knowingly run an app with assertions 
enabled in production.

>Simon
>
>  
>
Craig



---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [digester] assertion mechanism throws Error [was: plugins module ready for evaluation]

Posted by Simon Kitching <si...@ecnetwork.co.nz>.
On Tue, 2003-10-28 at 12:12, robert burrell donkin wrote:
> On Monday, October 27, 2003, at 10:41 PM, Simon Kitching wrote:

Just a note: I'm mostly on the side of a RuntimeException-based approach
already. I just proposed an Error-based exception to ensure the issue
was debated. A shame the "debate" is Robert & me so far..

If no-one can mount a good defence of the Java-1.4-like Error-based
approach, I will happily prepare a patch.

[Robert: If you could commit the logging patch submitted earlier today,
that would make it easier to create the patch, hint, hint :-].


> 
> i like assertions but i don't want error's thrown from library code when 
> it's used in production.

Well, I'll have a final stab at arguing the toss on this topic..

It seems the difference between the existing plugins code and the
equivalent implementation using java 1.4 assertions in "production" mode
is:
* the current plugins approach would throw Error, which may have nasty
side-effects on the container, vs
* the java 1.4 assertion mechanism would have been disabled, so the code
would zoom straight past the point at which the assertion would have
detected invalid state, and kept on going with unpredictable
consequences.

It's tough to have to choose between these options!

NB: If the java 1.4 assertions are not disabled, then the two
implementations behave identically.

Personally, I think the JSR assertion group got it wrong. Assertion
failures should indeed throw a RuntimeException subclass. Then it would
not be any great problem to leave assertions enabled in production code
(providing the performance hit was acceptable) without the nasty
problems that Robert described. 

I didn't want to say so at first, because a "RuntimeException" approach
would probably have been accepted without comment, and I hoped to get
some comment for/against the two options.


Regards,

Simon


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [digester] assertion mechanism throws Error [was: plugins module ready for evaluation]

Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
On Monday, October 27, 2003, at 10:41 PM, Simon Kitching wrote:

> On Tue, 2003-10-28 at 02:53, robert burrell donkin wrote:
>> On Sunday, October 5, 2003, at 10:53 PM, Simon Kitching wrote:
>>
>>> On Sun, 2003-10-05 at 00:43, robert burrell donkin wrote:
>>
>> <snip>
>>
>>>> 3. Exception handling
>>>>
>>>> PluginAssertionError extends Error. i'm not very happy about this. 
>>>> IMHO
>>>> a
>>>> well behaved component should not throw any Error subclasses. i'd be
>>>> happier for PluginAssertionError to be renamed 
>>>> PluginAssertionException
>>>> and extend RuntimeException.
>>>
>>> Having PluginAssertionError extend Error was a very deliberate choice.
>>> It is modelled after the java 1.4 assertion mechanism, where an
>>> assertion failure generates AssertionError.
>>>
>>> PluginAssertionError is never thrown due to a mistake on the part of the
>>> user of the Plugins module, nor due to a mistake in the xml of any sort.
>>> It is only thrown when a bug in the plugins library is detected.
>>
>> i probably should clarify my opinion: no well behaved component should
>> throw an Error in production. in many context's the throwing of an Error
>> in production can be pretty serious. in a J2EE context, a container may
>> mark a bean or a thread which throws an Error as dead. in a production
>> environment, this may cause problems with these pooled resources.
>>
>> in a way, though finding a bug in the plugins library in production may
>> irritate a user a little, i'm pretty sure that they will be very angry if
>> a bug in the plugins library causes their J2EE application to fail 
>> through
>> resources starvation. so, i'm not too sure what the best way to handle
>> this situation is but i don't think that throwing an Error is the right
>> thing to do. maybe, it could be replaced with a runtime.
>
> I see your point. However it seems to me that what you are arguing is
> that the *official* java assertion mechanism is fatally flawed, and will
> never be acceptable in Apache code. That might be right, but is a pretty
> significant decision.
>
> I just feel that the fellows who designed java assertions are pretty
> smart, and must have had a reason for deciding to define AssertionError
> instead of AssertionException. Rather than assume I'm smarter than them,
> I followed their lead on this issue.

they were smart enough to allow assertions to be turned off for production 
:)

i had considered suggesting a system property flag but it seems like it'd 
require a lot of explanation for such a small feature.

> Do you think it is worth opening up this discussion more widely on
> commons-dev? What do the Avalon or Tomcat people think about the
> java 1.4 assertion model?

the place where this kind of thing used to be debated was on general. 
(commons-dev is pretty high volume and i'd say it's a bit impolite posting 
to everyone.) but there are quite a few people who read digester who might 
like to jump in now.

> I understand it is not a pressing problem for Apache developers as
> assertions require a minimum of java1.4; while 1.2 or 1.3 support is
> required of apache projects it is only a theoretical problem - unless
> there are people like me who are fans of assertions.

i like assertions but i don't want error's thrown from library code when 
it's used in production.

- robert


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org