[Checkins] SVN: zope.app.publication/trunk/ log problems that cause an abort to fail.

[Gary Poster]

Log message for revision 102692:
  log problems that cause an abort to fail.

Changed:
  U   zope.app.publication/trunk/CHANGES.txt
  U   zope.app.publication/trunk/src/zope/app/publication/tests/test_zopepublication.py
  U   zope.app.publication/trunk/src/zope/app/publication/zopepublication.py

-=-
Modified: zope.app.publication/trunk/CHANGES.txt
===================================================================
--- zope.app.publication/trunk/CHANGES.txt	2009-08-11 21:32:45 UTC (rev 102691)
+++ zope.app.publication/trunk/CHANGES.txt	2009-08-11 22:34:23 UTC (rev 102692)
@@ -2,10 +2,16 @@
 CHANGES
 =======
 
+3.8.2 (unreleased)
+------------------
+
+- An abort within handleExceptions could have failed without logging what
+  caused the error. It now logs the original problem.
+
 3.8.1 (2009-06-21)
 ------------------
 
-- Bug fix: The publication traverseName methoid used ProxyFactory
+- Bug fix: The publication traverseName method used ProxyFactory
   rather than the publication proxy method.
 
 3.8.0 (2009-06-20)

Modified: zope.app.publication/trunk/src/zope/app/publication/tests/test_zopepublication.py
===================================================================
--- zope.app.publication/trunk/src/zope/app/publication/tests/test_zopepublication.py	2009-08-11 21:32:45 UTC (rev 102691)
+++ zope.app.publication/trunk/src/zope/app/publication/tests/test_zopepublication.py	2009-08-11 22:34:23 UTC (rev 102692)
@@ -414,7 +414,46 @@
         self.assert_(isinstance(error_info[1], FooError))
         self.assert_(request is self.request)
 
+    def testLogBeforeAbort(self):
+        # If we get an exception, and then (a catastrophe, but one that has
+        # been experienced) transaction.abort fails, we really want to know
+        # what happened before that abort.
+        # (Set up:)
+        zope.component.provideUtility(ErrorReportingUtility())
+        abort = transaction.abort
+        class AbortError(Exception):
+            pass
+        class AnEarlierError(Exception):
+            pass
+        def faux_abort():
+            raise AbortError
+        try:
+            raise AnEarlierError()
+        except AnEarlierError:
+            pass
+        transaction.abort = faux_abort
+        try:
+            # (Test:)
+            try:
+                self.publication.handleException(
+                    self.object, self.request, sys.exc_info(),
+                    retry_allowed=False)
+            except AbortError:
+                pass
+            else:
+                self.fail('Aborting should have failed')
+            # we expect a message in our logging utility
+            error_log = zope.component.getUtility(IErrorReportingUtility)
+            self.assertEqual(len(error_log.exceptions), 1)
+            error_info, request = error_log.exceptions[0]
+            self.assertEqual(error_info[0], AnEarlierError)
+            self.failUnless(isinstance(error_info[1], AnEarlierError))
+            self.failUnless(request is self.request)
+        finally:
+            # (Tear down:)
+            transaction.abort = abort
 
+
 class ZopePublicationTests(BasePublicationTests):
 
     def testGlobalAuth(self):

Modified: zope.app.publication/trunk/src/zope/app/publication/zopepublication.py
===================================================================
--- zope.app.publication/trunk/src/zope/app/publication/zopepublication.py	2009-08-11 21:32:45 UTC (rev 102691)
+++ zope.app.publication/trunk/src/zope/app/publication/zopepublication.py	2009-08-11 22:34:23 UTC (rev 102692)
@@ -285,7 +285,12 @@
     def handleException(self, object, request, exc_info, retry_allowed=True):
         # This transaction had an exception that reached the publisher.
         # It must definitely be aborted.
-        transaction.abort()
+        try:
+            transaction.abort()
+        except:
+            # Hm, a catastrophe.  We might want to know what preceded it.
+            self._logErrorWithErrorReportingUtility(object, request, exc_info)
+            raise
 
         # Reraise Retry exceptions for the publisher to deal with.
         if retry_allowed and isinstance(exc_info[1], Retry):
@@ -305,7 +310,7 @@
         # handling determine whether a retry is allowed or not?
         # Assume not for now.
 
-        # Record the error with the ErrorReportingUtility
+        # Record the error with the ErrorReportingUtility.
         self._logErrorWithErrorReportingUtility(object, request, exc_info)
 
         response = request.response