[Zope3-dev] Re: [Zope-dev] Re: tal:on-error is a bare except...

[Tres Seaver]

Jim Fulton wrote:
> Tres Seaver wrote:
> 
>> Jim Fulton wrote:
>>
>>> Tres Seaver wrote:
>>>
>>>> I don't believe the bare except is *ever* good form in 
>>>> production-quality code.  *No* application-level code has enough zen 
>>>> to swallow some kinds of exceptions, which is why the Java folks 
>>>> have a separate RuntimeError hierarchy.
>>>
>>> This isn't application level code.
>>
>> I guess I don't know what you mean by I "application level":  the code 
>> path occurs because some application developer put '<div 
>> tal:on-error>' into a template to "prettify" the error messages.  That 
>> hardly feels like "infrastructure" coding to me.
> 
> But that's not a bare except. It's the code that implements that that
> has the bare except, and that is infrastructure.

'on-error' has a "bare except" contract at the moment, which makes it 
"broken as designed.":

  - 
http://www.zope.org/Documentation/Books/ZopeBook/2_6Edition/AppendixC.stx

>> Perhaps where we agree is in calling a bug the fact that the 
>> implementation of the on-error handler (like 'hasattr'!) uses a bare 
>> except.
> 
> hasattr is the sort of thing that would be used all over the place,
> by lots of code.  It's especially evil, since clearly, it's about
> attribute errors.
> 
> Are you suggesting that on-error should take an exception?

I think I was venting.  However, if we modified on-error to require an 
exception type (or a list?  maybe a specification instead?), *and* 
reworked ConflictError / POSError such that none of the "normal" 
exception classes were bases, then we might avoid this problem.  We 
could even default the list to be 'Exception' (as long as we *don't* 
derive ConflictError from that!)

>>>> The fact that newer ZODB's *appear* to prevent later commits does 
>>>> not exculpate the use of the bare except in 'tal:on-error' (emphasis 
>>>> added to indicate a non-trivial amount of suspicion against the code 
>>>> paths in question;  we have claimed to have solved that problem 
>>>> before).
>>>
>>> Right, we have to do something different here. Adding a dependency on
>>> ZODB isn't an option.  There needs to be some framework for deciding
>>> which exceptions to catch and which not to.  It's hard though, because
>>> we do want to catch conflict errors. We just don't want to catch them
>>> anywhere but the publisher.
> 
> Well, but it's easier than I suggested, because in the publisher, we
> don't catch it with a bare except.
> 
>>> I'm open to suggestions that don't involve sprinling references to
>>> ConflictError all over the place.
>>
>>
>>
>> We could introduce a new base exception, not dependent on ZODB, and 
>> then sprinkle references to *that* everywhere:
>>
>>   # zExceptions.py -- I don't recall where this belongs in Zope3
>>   class FrameworkError:
>>       """ Base for exceptions not intended to be caught by default.
>>       """
>>
>>   # ZODB3/POSException.py -- likewise
>>   class POSError(FrameworkError):
>>       """Persistent object system error.
>>       """
> 
> 
> This would be great if it was part of Python.
> 
> Hm. I wonder if SystemError would be appropriate here.
> (Zope may abuse SystemError, but we could fix that.)

RuntimeError exists, but has the wrong bases (StandardError, which 
derives from Exception).  SystemError has StandardError as its base, too.

Tres.
-- 
===============================================================
Tres Seaver                                tseaver at zope.com
Zope Corporation      "Zope Dealers"       http://www.zope.com