Zope 2: ZPublisher exception handling
Hi! Summary: ======== In Zope 2.10 'raise_standardErrorMessage', 'zpublisher_exception_hook' and therefore 'Publish.publish' did always (re-)raise errors after rendering. That made sure 'HTTPResponse.exception' was called and CookieCrumbler was able to hook into 'HTTPResponse._unauthorized'. I'm now trying to figure out how this bug could be fixed: https://bugs.launchpad.net/zope-cmf/+bug/558340 Some details: ============= In Zope 2.10 and earlier the code did (re-)raise errors this way: raise error_type, error_value, traceback 'error_type' is the original error class, 'error_value' the rendered error message. Python creates a new error instance, basically trying to do this: error_instance = error_type(error_value) That doesn't work if the error class constructor requires more than one argument. This was the first change that broke the old behavior: http://svn.zope.org/?view=rev&rev=77459 The comment in Zope2/App/startup.py is interesting because it says "raise the rendered value" while the code actually *returns* it: # Lookup a view for the exception and render it, then # raise the rendered value as the exception value # (basically the same that 'raise_standardErrorMessage' # does. I guess the checkin message for http://svn.zope.org/?view=rev&rev=92767 explains why: "Rather nasty fix to work around Zope 3 exceptions that have more than one positional argument on the constructor." The old machinery doesn't work with ZTK exceptions like zope.publisher.interfaces.NotFound because they take additional arguments and have are more complex __str__ method. There is an inconsistency in revision 92767 ( http://svn.zope.org/Zope/trunk/lib/python/OFS/SimpleItem.py?rev=92767&view=d... ): The checkin message says: "work around Zope 3 exceptions that have *more than one* positional argument". I think that's correct. The comment in SimpleItem.py says something different: # Can we re-raise the exception with a rendered-to-HTML # exception value? To be able to do so, the exception # constructor needs to be able to take more than two # arguments (some Zope exceptions can't). The code follows that comment, so 'can_raise' is sometimes wrong. E.g. can_raise is True for zope.publisher.interfaces.NotFound, but re-raising with the rendered value as only argument doesn't work. Proposal: ========= We could try to fix bugs like the one mentioned above. And we could restore the old behavior for normal Zope 2 exceptions, fixing the CookieCrumbler issue. But the whole thing looks like a big mess. I think we should try to find a way to treat all kinds of errors the same way. I see two possible approaches: 1.) Stop re-raising errors. Always return rendered values and pass them on to the places where the errors are currently catched. 2.) Re-raise the original errors. And store the rendered value somewhere else. Maybe in the response object. What do you think? Any comments are welcome. Cheers, Yuppie
yuppie wrote:
In Zope 2.10 'raise_standardErrorMessage', 'zpublisher_exception_hook' and therefore 'Publish.publish' did always (re-)raise errors after rendering. That made sure 'HTTPResponse.exception' was called and CookieCrumbler was able to hook into 'HTTPResponse._unauthorized'.
I'm now trying to figure out how this bug could be fixed: https://bugs.launchpad.net/zope-cmf/+bug/558340
For the record: Meanwhile I figured out that there is a related launchpad issue https://bugs.launchpad.net/zope2/+bug/372632 and a workaround for the CookieCrumbler issue: http://dev.plone.org/collective/changeset/92340 I also removed some useless code in 'raise_standardErrorMessage': http://svn.zope.org/?rev=110801&view=rev http://svn.zope.org/?rev=110806&view=rev Cheers, Yuppie
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 yuppie wrote:
yuppie wrote:
In Zope 2.10 'raise_standardErrorMessage', 'zpublisher_exception_hook' and therefore 'Publish.publish' did always (re-)raise errors after rendering. That made sure 'HTTPResponse.exception' was called and CookieCrumbler was able to hook into 'HTTPResponse._unauthorized'.
I'm now trying to figure out how this bug could be fixed: https://bugs.launchpad.net/zope-cmf/+bug/558340
For the record:
Meanwhile I figured out that there is a related launchpad issue https://bugs.launchpad.net/zope2/+bug/372632
Do you have a suggestion for resolving those two bugs?
and a workaround for the CookieCrumbler issue: http://dev.plone.org/collective/changeset/92340
Hmm, registering an exception view which re-raises the exception is definitely suggestive of something wrong in the plumbing.
I also removed some useless code in 'raise_standardErrorMessage': http://svn.zope.org/?rev=110801&view=rev http://svn.zope.org/?rev=110806&view=rev
Cool. Tres. - -- =================================================================== Tres Seaver +1 540-429-0999 tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkvEtewACgkQ+gerLs4ltQ7sCQCgwzq/lgYIRYIQq6AQ7oFyl5Yq qrAAnR2N+tVmAqleNsYiibUgTM/5mtwH =buo0 -----END PGP SIGNATURE-----
Hi! Tres Seaver wrote:
yuppie wrote:
yuppie wrote:
In Zope 2.10 'raise_standardErrorMessage', 'zpublisher_exception_hook' and therefore 'Publish.publish' did always (re-)raise errors after rendering. That made sure 'HTTPResponse.exception' was called and CookieCrumbler was able to hook into 'HTTPResponse._unauthorized'.
I'm now trying to figure out how this bug could be fixed: https://bugs.launchpad.net/zope-cmf/+bug/558340
For the record:
Meanwhile I figured out that there is a related launchpad issue https://bugs.launchpad.net/zope2/+bug/372632
Do you have a suggestion for resolving those two bugs?
The original issue reported in https://bugs.launchpad.net/zope2/+bug/372632 seems to be fixed. The issue discussed in comment #15 and later is basically the same as in https://bugs.launchpad.net/zope-cmf/+bug/558340. So there is only *one* issue: If errors are rendered by 'raise_standardErrorMessage' or by an error view they are not re-raised. In that case the error handling in 'publish_module_standard' and 'HTTPResponse.exception' is no longer invoked. I'm not sure how to resolve this. Approach 1: We try to restore the old behavior. The changes were made because the old machinery didn't work with exceptions like zope.publisher.interfaces.NotFound, but I'm optimistic we can find a less intrusive fix for that. Approach 2: The _unauthorized call seems to be the only part of the old error handling that people are actually missing. We could try to add a new hook for that and deprecate big parts of the old code. But is that old code really useless?
and a workaround for the CookieCrumbler issue: http://dev.plone.org/collective/changeset/92340
Hmm, registering an exception view which re-raises the exception is definitely suggestive of something wrong in the plumbing.
Yes. But *if* we don't want restore the old behavior, we could move CookieCrumbler's complete redirect logic into an error view. And get rid of _unauthorized. Cheers, Yuppie
participants (2)
-
Tres Seaver -
yuppie