Merge lp:~stub/launchpad/bug-373837 into lp:launchpad

[Stuart Bishop]

[Stuart Bishop (stub)]

In my fix for Bug #373837 I was overzealous and logged TransactionRollbackErrors as a soft OOPS when they occurred in addition to DisconnectionErrors. This isn't desirable, as this is a normal occurrence (SERIALIZATION exceptions cannot be avoided and the app is required to handle them). We could just ignore these OOPSes, but they look identical to a case we *do* need to know about - when our retry attempts fail too many times and we give up.

[Stuart Bishop (stub)]

This branch will close Bug #409907

[Brad Crittenden (bac)]

Easy fix, nice branch.

review: Approve

=== modified file 'lib/canonical/launchpad/webapp/publication.py'
--- lib/canonical/launchpad/webapp/publication.py	2009-07-28 13:05:05 +0000
+++ lib/canonical/launchpad/webapp/publication.py	2009-08-06 12:23:39 +0000
@@ -475,12 +475,9 @@
             # the publication, so there's nothing we need to do here.
             pass
 
-        # Log a soft OOPS for DisconnectionErrors and
-        # TransactionRollbackErrors, as per Bug #373837. We need to do
-        # this before we re-raise the excaptionsas a Retry.
-        if isinstance(
-            exc_info[1],
-            (DisconnectionError, TransactionRollbackError)):
+        # Log a soft OOPS for DisconnectionErrors as per Bug #373837.
+        # We need to do this before we re-raise the excaptionsas a Retry.
+        if isinstance(exc_info[1], DisconnectionError):
             getUtility(IErrorReportingUtility).raising(exc_info, request)
 
         def should_retry(exc_info):

=== modified file 'lib/canonical/launchpad/webapp/tests/test_publication.py'
--- lib/canonical/launchpad/webapp/tests/test_publication.py	2009-07-28 13:05:05 +0000
+++ lib/canonical/launchpad/webapp/tests/test_publication.py	2009-08-06 12:23:39 +0000
@@ -74,27 +74,24 @@
 
         # Ensure that OOPS reports are generated for database
         # disconnections, as per Bug #373837.
-        for exception in (DisconnectionError, TransactionRollbackError):
-            request = LaunchpadTestRequest()
-            publication = WebServicePublication(None)
-            da.set_request_started()
-            try:
-                raise exception('Fake')
-            except exception:
-                self.assertRaises(
-                    Retry,
-                    publication.handleException,
-                    None, request, sys.exc_info(), True)
-            da.clear_request_started()
-            next_oops = error_reporting_utility.getLastOopsReport()
-
-            # Ensure the OOPS mentions the correct exception
-            self.assertNotEqual(repr(next_oops).find(exception.__name__), -1)
-
-            # Ensure that it is different to the last logged OOPS.
-            self.assertNotEqual(repr(last_oops), repr(next_oops))
-
-            last_oops = next_oops
+        request = LaunchpadTestRequest()
+        publication = WebServicePublication(None)
+        da.set_request_started()
+        try:
+            raise DisconnectionError('Fake')
+        except DisconnectionError:
+            self.assertRaises(
+                Retry,
+                publication.handleException,
+                None, request, sys.exc_info(), True)
+        da.clear_request_started()
+        next_oops = error_reporting_utility.getLastOopsReport()
+
+        # Ensure the OOPS mentions the correct exception
+        self.assertNotEqual(repr(next_oops).find(exception.__name__), -1)
+
+        # Ensure that it is different to the last logged OOPS.
+        self.assertNotEqual(repr(last_oops), repr(next_oops))
 
 
 def test_suite():