Merge lp:~stub/launchpad/oops into lp:launchpad

Proposed by Stuart Bishop on 2011-11-14
Status: Merged
Approved by: Stuart Bishop on 2011-11-14
Approved revision: 10004
Merged at revision: 14299
Proposed branch: lp:~stub/launchpad/oops
Merge into: lp:launchpad
Prerequisite: lp:~stub/launchpad/trivial
Diff against target: 59 lines (+31/-0)
2 files modified
lib/canonical/launchpad/webapp/errorlog.py (+10/-0)
lib/canonical/launchpad/webapp/tests/test_errorlog.py (+21/-0)
To merge this branch: bzr merge lp:~stub/launchpad/oops
Reviewer Review Type Date Requested Status
Raphaël Badin (community) 2011-11-14 Approve on 2011-11-14
Review via email: mp+82119@code.launchpad.net

Commit message

[r=rvb][no-qa] Attach a link to the previous OOPS generated in the request, if any.

Description of the change

Attach a link to the previous OOPS generated in the request, if any.

To post a comment you must log in.
Raphaël Badin (rvb) wrote :

Looks good.

One remark:

12 + if last_oopsid:

Not sure it's worth it but maybe it would be more safe/explicit to write: "if last_oopsid is not None:".

review: Approve
Stuart Bishop (stub) wrote :

Changed. Thanks!

lp:~stub/launchpad/oops updated on 2011-11-14
10005. By Stuart Bishop on 2011-11-14

Clarity

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/webapp/errorlog.py'
2--- lib/canonical/launchpad/webapp/errorlog.py 2011-11-08 14:05:08 +0000
3+++ lib/canonical/launchpad/webapp/errorlog.py 2011-11-14 11:21:26 +0000
4@@ -161,6 +161,14 @@
5 _ignored_exceptions_for_unauthenticated_users = set(['Unauthorized'])
6
7
8+def attach_previous_oopsid(report, context):
9+ """Add a link to the previous OOPS generated this request, if any."""
10+ request = context.get('http_request')
11+ last_oopsid = getattr(request, 'oopsid', None)
12+ if last_oopsid is not None:
13+ report['last_oops'] = last_oopsid
14+
15+
16 def attach_http_request(report, context):
17 """Add request metadata into the error report.
18
19@@ -312,6 +320,8 @@
20 # In the zope environment we track how long a script / http
21 # request has been running for - this is useful data!
22 self._oops_config.on_create.append(attach_adapter_duration)
23+ # Any previous OOPS reports generated this request.
24+ self._oops_config.on_create.append(attach_previous_oopsid)
25
26 def add_publisher(publisher):
27 if publisher_adapter is not None:
28
29=== modified file 'lib/canonical/launchpad/webapp/tests/test_errorlog.py'
30--- lib/canonical/launchpad/webapp/tests/test_errorlog.py 2011-10-27 05:40:56 +0000
31+++ lib/canonical/launchpad/webapp/tests/test_errorlog.py 2011-11-14 11:21:26 +0000
32@@ -184,6 +184,27 @@
33 # - a notify publisher
34 self.assertEqual(oops_config.publishers[2], notify_publisher)
35
36+ def test_multiple_raises_in_request(self):
37+ """An OOPS links to the previous OOPS in the request, if any."""
38+ utility = ErrorReportingUtility()
39+ del utility._oops_config.publishers[0]
40+
41+ request = TestRequestWithPrincipal()
42+ try:
43+ raise ArbitraryException('foo')
44+ except ArbitraryException:
45+ report = utility.raising(sys.exc_info(), request)
46+
47+ self.assertFalse('last_oops' in report)
48+ last_oopsid = request.oopsid
49+ try:
50+ raise ArbitraryException('foo')
51+ except ArbitraryException:
52+ report = utility.raising(sys.exc_info(), request)
53+
54+ self.assertTrue('last_oops' in report)
55+ self.assertEqual(report['last_oops'], last_oopsid)
56+
57 def test_raising_with_request(self):
58 """Test ErrorReportingUtility.raising() with a request"""
59 utility = ErrorReportingUtility()