[Zope3-checkins] CVS: Zope3/src/zope/app/publication - zopepublication.py:1.10

Jeremy Hylton jeremy@zope.com
Mon, 3 Feb 2003 12:20:26 -0500


Update of /cvs-repository/Zope3/src/zope/app/publication
In directory cvs.zope.org:/tmp/cvs-serv1462

Modified Files:
	zopepublication.py 
Log Message:
Remove unneeded try/finally.

There is no need to explicitly zap a traceback object passed as an
argument to a function.  In general, if an exception is raised in a
function and that traceback is stored, then the function's frame will
be kept alive because the traceback has a reference to it.  This
function doesn't create a traceback, so it isn't directly responsible
for extending the life of the frame.

It may be the case that the caller of handleException() must be
careful.  On the other hand, the garbage collector will be able to
collect a cycle involving traceback and frame unless some objects with
finalizers are involved.


=== Zope3/src/zope/app/publication/zopepublication.py 1.9 => 1.10 ===
--- Zope3/src/zope/app/publication/zopepublication.py:1.9	Mon Feb  3 10:03:06 2003
+++ Zope3/src/zope/app/publication/zopepublication.py	Mon Feb  3 12:20:24 2003
@@ -161,75 +161,69 @@
         get_transaction().commit()
 
     def handleException(self, object, request, exc_info, retry_allowed=1):
-        try:
-            # Abort the transaction.
-            get_transaction().abort()
+        # Abort the transaction.
+        get_transaction().abort()
 
+        try:
+            errService = getService(object, 'ErrorReportingService')
+        except ComponentLookupError:
+            pass
+        else:
             try:
-                errService = getService(object,'ErrorReportingService')
-            except ComponentLookupError:
-                pass
-            else:
-                try:
-                    errService.raising(exc_info, request)
-                # It is important that an error in errService.raising
-                # does not propagate outside of here. Otherwise, nothing
-                # meaningful will be returned to the user.
-                #
-                # The error reporting service should not be doing database
-                # stuff, so we shouldn't get a conflict error.
-                # Even if we do, it is more important that we log this
-                # error, and proceed with the normal course of events.
-                # We should probably (somehow!) append to the standard
-                # error handling that this error occurred while using
-                # the ErrorReportingService, and that it will be in
-                # the zope log.
-                except:
-                    logging.getLogger('SiteError').exception(
-                        'Error while reporting an error to the '
-                        'ErrorReportingService')
-
-            # Delegate Unauthorized errors to the authentication service
-            # XXX Is this the right way to handle Unauthorized?  We need
-            # to understand this better.
-            if isinstance(exc_info[1], Unauthorized):
-                sm = getSecurityManager()
-                id = sm.getPrincipal()
-                prin_reg.unauthorized(id, request) # May issue challenge
-                request.response.handleException(exc_info)
-                return
-
-            # XXX This is wrong. Should use getRequstView:
+                errService.raising(exc_info, request)
+            # It is important that an error in errService.raising
+            # does not propagate outside of here. Otherwise, nothing
+            # meaningful will be returned to the user.
             #
-            #
-            # # Look for a component to handle the exception.
-            # traversed = request.traversed
-            # if traversed:
-            #     context = traversed[-1]
-            #     #handler = getExceptionHandler(context, t, IBrowserPublisher)
-            #     handler = None  # no getExceptionHandler() exists yet.
-            #     if handler is not None:
-            #         handler(request, exc_info)
-            #         return
-
-            # Convert ConflictErrors to Retry exceptions.
-            if retry_allowed and isinstance(exc_info[1], ConflictError):
-                #XXX this code path needs a unit test
-                logging.getLogger('ZopePublication').warn(
-                    'Competing writes/reads at %s',
-                    request.get('PATH_INFO', '???'),
-                    exc_info=True)
-                raise Retry
-
-            # Let the response handle it as best it can.
-            # XXX Is this what we want in the long term?
-            response = request.response
-            response.handleException(exc_info)
+            # The error reporting service should not be doing database
+            # stuff, so we shouldn't get a conflict error.
+            # Even if we do, it is more important that we log this
+            # error, and proceed with the normal course of events.
+            # We should probably (somehow!) append to the standard
+            # error handling that this error occurred while using
+            # the ErrorReportingService, and that it will be in
+            # the zope log.
+            except:
+                logging.getLogger('SiteError').exception(
+                    'Error while reporting an error to the '
+                    'ErrorReportingService')
+
+        # Delegate Unauthorized errors to the authentication service
+        # XXX Is this the right way to handle Unauthorized?  We need
+        # to understand this better.
+        if isinstance(exc_info[1], Unauthorized):
+            sm = getSecurityManager()
+            id = sm.getPrincipal()
+            prin_reg.unauthorized(id, request) # May issue challenge
+            request.response.handleException(exc_info)
             return
-        finally:
-            # Avoid leaking
-            exc_info = 0
 
+        # XXX This is wrong. Should use getRequstView:
+        #
+        #
+        # # Look for a component to handle the exception.
+        # traversed = request.traversed
+        # if traversed:
+        #     context = traversed[-1]
+        #     #handler = getExceptionHandler(context, t, IBrowserPublisher)
+        #     handler = None  # no getExceptionHandler() exists yet.
+        #     if handler is not None:
+        #         handler(request, exc_info)
+        #         return
+
+        # Convert ConflictErrors to Retry exceptions.
+        if retry_allowed and isinstance(exc_info[1], ConflictError):
+            #XXX this code path needs a unit test
+            logging.getLogger('ZopePublication').warn(
+                'Competing writes/reads at %s',
+                request.get('PATH_INFO', '???'),
+                exc_info=True)
+            raise Retry
+
+        # Let the response handle it as best it can.
+        # XXX Is this what we want in the long term?
+        request.response.handleException(exc_info)
+        return
 
     def _parameterSetskin(self, pname, pval, request):
         request.setViewSkin(pval)