launchpad-reviewers team mailing list archive

[Stuart Bishop <stuart.bishop@xxxxxxxxxxxxx>]

Stuart Bishop has proposed merging lp:~stub/launchpad/oops into lp:launchpad with lp:~stub/launchpad/trivial as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~stub/launchpad/oops/+merge/82119

Attach a link to the previous OOPS generated in the request, if any.
-- 
https://code.launchpad.net/~stub/launchpad/oops/+merge/82119
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stub/launchpad/oops into lp:launchpad.

=== modified file 'lib/canonical/launchpad/webapp/errorlog.py'
--- lib/canonical/launchpad/webapp/errorlog.py	2011-11-08 14:05:08 +0000
+++ lib/canonical/launchpad/webapp/errorlog.py	2011-11-14 10:32:40 +0000
@@ -161,6 +161,14 @@
 _ignored_exceptions_for_unauthenticated_users = set(['Unauthorized'])
 
 
+def attach_previous_oopsid(report, context):
+    """Add a link to the previous OOPS generated this request, if any."""
+    request = context.get('http_request')
+    last_oopsid = getattr(request, 'oopsid', None)
+    if last_oopsid:
+        report['last_oops'] = last_oopsid
+
+
 def attach_http_request(report, context):
     """Add request metadata into the error report.
 
@@ -312,6 +320,8 @@
         # In the zope environment we track how long a script / http
         # request has been running for - this is useful data!
         self._oops_config.on_create.append(attach_adapter_duration)
+        # Any previous OOPS reports generated this request.
+        self._oops_config.on_create.append(attach_previous_oopsid)
 
         def add_publisher(publisher):
             if publisher_adapter is not None:

=== modified file 'lib/canonical/launchpad/webapp/tests/test_errorlog.py'
--- lib/canonical/launchpad/webapp/tests/test_errorlog.py	2011-10-27 05:40:56 +0000
+++ lib/canonical/launchpad/webapp/tests/test_errorlog.py	2011-11-14 10:32:40 +0000
@@ -184,6 +184,27 @@
         # - a notify publisher
         self.assertEqual(oops_config.publishers[2], notify_publisher)
 
+    def test_multiple_raises_in_request(self):
+        """An OOPS links to the previous OOPS in the request, if any."""
+        utility = ErrorReportingUtility()
+        del utility._oops_config.publishers[0]
+
+        request = TestRequestWithPrincipal()
+        try:
+            raise ArbitraryException('foo')
+        except ArbitraryException:
+            report = utility.raising(sys.exc_info(), request)
+
+        self.assertFalse('last_oops' in report)
+        last_oopsid = request.oopsid
+        try:
+            raise ArbitraryException('foo')
+        except ArbitraryException:
+            report = utility.raising(sys.exc_info(), request)
+
+        self.assertTrue('last_oops' in report)
+        self.assertEqual(report['last_oops'], last_oopsid)
+
     def test_raising_with_request(self):
         """Test ErrorReportingUtility.raising() with a request"""
         utility = ErrorReportingUtility()