launchpad-reviewers team mailing list archive

[Robert Collins <robertc@xxxxxxxxxxxxxxxxx>]

Robert Collins has proposed merging lp:~lifeless/python-oops-wsgi/extraction into lp:python-oops-wsgi.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~lifeless/python-oops-wsgi/extraction/+merge/71969

Simple addition - grab the url and give that to the oops context when reporting an error.
-- 
https://code.launchpad.net/~lifeless/python-oops-wsgi/extraction/+merge/71969
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/python-oops-wsgi/extraction into lp:python-oops-wsgi.

=== modified file 'oops_wsgi/middleware.py'
--- oops_wsgi/middleware.py	2011-08-17 09:45:30 +0000
+++ oops_wsgi/middleware.py	2011-08-18 00:50:25 +0000
@@ -21,6 +21,8 @@
 import socket
 import sys
 
+from paste.request import construct_url
+
 __all__ = [
     'make_app',
     ]
@@ -100,7 +102,8 @@
             raise
         except Exception:
             exc_info = sys.exc_info()
-            report = config.create(dict(exc_info=exc_info))
+            report = config.create(
+                    dict(exc_info=exc_info, url=construct_url(environ)))
             ids = config.publish(report)
             if not ids or 'write' in state:
                 # No OOPS generated, no oops publisher, or we have already

=== modified file 'oops_wsgi/tests/test_middleware.py'
--- oops_wsgi/tests/test_middleware.py	2011-08-17 09:45:30 +0000
+++ oops_wsgi/tests/test_middleware.py	2011-08-18 00:50:25 +0000
@@ -46,6 +46,12 @@
         if not params:
             params = {}
         app = make_app(inner, config, **params)
+        environ = {}
+        # Shove enough stuff in to let url reconstruction work:
+        environ['wsgi.url_scheme'] = 'http'
+        environ['HTTP_HOST'] = 'example.com'
+        environ['PATH_INFO'] = '/demo'
+        environ['QUERY_STRING'] = 'param=value'
         def start_response(status, headers):
             if failing_start:
                 raise socket.error(errno.EPIPE, 'Connection closed')
@@ -57,9 +63,9 @@
             else:
                 return self.calls.append
         if not failing_server_write:
-            return ''.join(list(app({}, start_response)))
+            return ''.join(list(app(environ, start_response)))
         else:
-            iterator = iter(app({}, start_response))
+            iterator = iter(app(environ, start_response))
             # get one item from it, which is enough to ensure we've activated
             # all the frames.
             step = iterator.next()
@@ -182,6 +188,7 @@
             # First the oops is generated
             {'id': 1,
              'type': 'ValueError',
+             'url': 'http://example.com/demo?param=value',
              'value': u'boom, yo'},
             # Then the middleware responses
             ('500 Internal Server Error', [('Content-Type', 'text/html')]),
@@ -209,6 +216,7 @@
             # First the oops is generated
             {'id': 1,
              'type': 'ValueError',
+             'url': 'http://example.com/demo?param=value',
              'value': u'boom, yo'},
             # Then the middleware responses
             ('500 Internal Server Error', [('Content-Type', 'text/json')]),
@@ -228,8 +236,17 @@
             # First the oops is generated
             {'id': 1,
              'type': 'ValueError',
+             'url': 'http://example.com/demo?param=value',
              'value': u'boom, yo'},
             # Then the middleware responses
             ('500 Internal Server Error', [('Content-Type', 'text/html')]),
             ], self.calls)
         self.assertEqual(iterated, 'woo')
+
+    def test_oops_url_in_context(self):
+        def inner(environ, start_response):
+            raise ValueError('boom, yo')
+        config = self.config_for_oopsing()
+        iterated = self.wrap_and_run(inner, config)
+        self.assertEqual('http://example.com/demo?param=value',
+                self.calls[0]['url'])