launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04637
[Merge] lp:~lifeless/launchpad/useoops into lp:launchpad
Robert Collins has proposed merging lp:~lifeless/launchpad/useoops into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~lifeless/launchpad/useoops/+merge/71796
More code extraction:
- datestamps shifted out,
- low level escaping done by the oops publisher
I think this completes enough work to get gpgverify running, so thats my next experiment.
--
https://code.launchpad.net/~lifeless/launchpad/useoops/+merge/71796
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/launchpad/useoops into lp:launchpad.
=== modified file 'lib/canonical/launchpad/webapp/errorlog.py'
--- lib/canonical/launchpad/webapp/errorlog.py 2011-08-16 06:58:32 +0000
+++ lib/canonical/launchpad/webapp/errorlog.py 2011-08-17 01:17:27 +0000
@@ -20,7 +20,7 @@
import urlparse
from lazr.restful.utils import get_current_browser_request
-import oops
+import oops.createhooks
from oops_datedir_repo import DateDirRepo
import oops_datedir_repo.serializer_rfc822
import pytz
@@ -56,52 +56,6 @@
LAZR_OOPS_USER_REQUESTED_KEY = 'lazr.oops.user_requested'
-# Restrict the rate at which errors are sent to the Zope event Log
-# (this does not affect generation of error reports).
-_rate_restrict_pool = {}
-
-# The number of seconds that must elapse on average between sending two
-# exceptions of the same name into the Event Log. one per minute.
-_rate_restrict_period = datetime.timedelta(seconds=60)
-
-# The number of exceptions to allow in a burst before the above limit
-# kicks in. We allow five exceptions, before limiting them to one per
-# minute.
-_rate_restrict_burst = 5
-
-
-def _normalise_whitespace(s):
- """Normalise the whitespace in a string to spaces"""
- if s is None:
- return None
- return ' '.join(s.split())
-
-
-def _safestr(obj):
- if isinstance(obj, unicode):
- return obj.replace('\\', '\\\\').encode('ASCII',
- 'backslashreplace')
- # A call to str(obj) could raise anything at all.
- # We'll ignore these errors, and print something
- # useful instead, but also log the error.
- # We disable the pylint warning for the blank except.
- try:
- value = str(obj)
- except:
- logging.getLogger('SiteError').exception(
- 'Error in ErrorReportingService while getting a str '
- 'representation of an object')
- value = '<unprintable %s object>' % (
- str(type(obj).__name__))
- # Some str() calls return unicode objects.
- if isinstance(value, unicode):
- return _safestr(value)
- # encode non-ASCII characters
- value = value.replace('\\', '\\\\')
- value = re.sub(r'[\x80-\xff]',
- lambda match: '\\x%02x' % ord(match.group(0)), value)
- return value
-
def _is_sensitive(request, name):
"""Return True if the given request variable name is sensitive.
@@ -139,14 +93,16 @@
class ErrorReport:
implements(IErrorReport)
- def __init__(self, id, type, value, time, pageid, tb_text, username,
+ def __init__(self, id, type, value, time, tb_text, username,
url, duration, req_vars, db_statements, informational=None,
- branch_nick=None, revno=None):
+ branch_nick=None, revno=None, topic=None, reporter=None):
self.id = id
self.type = type
self.value = value
self.time = time
- self.pageid = pageid
+ self.topic = topic
+ if reporter is not None:
+ self.reporter = reporter
self.tb_text = tb_text
self.username = username
self.url = url
@@ -179,10 +135,6 @@
# for.
report['duration'] = get_request_duration()
-def attach_date(report, context):
- """Set the time key in report to a datetime of now."""
- report['time'] = datetime.datetime.now(UTC)
-
def attach_exc_info(report, context):
"""Attach exception info to the report.
@@ -196,14 +148,14 @@
info = context.get('exc_info')
if info is None:
return
- report['type'] = _safestr(getattr(info[0], '__name__', info[0]))
- report['value'] = _safestr(info[1])
+ report['type'] = getattr(info[0], '__name__', info[0])
+ report['value'] = oops.createhooks.safe_unicode(info[1])
if not isinstance(info[2], basestring):
tb_text = ''.join(format_exception(*info,
**{'as_html': False}))
else:
tb_text = info[2]
- report['tb_text'] = _safestr(tb_text)
+ report['tb_text'] = tb_text
_ignored_exceptions_for_unauthenticated_users = set(['Unauthorized'])
@@ -216,7 +168,7 @@
* url
* ignore
* username
- * pageid
+ * topic
* req_vars
"""
info = context.get('exc_info')
@@ -226,7 +178,7 @@
# XXX jamesh 2005-11-22: Temporary fix, which Steve should
# undo. URL is just too HTTPRequest-specific.
if safe_hasattr(request, 'URL'):
- report['url'] = _safestr(request.URL)
+ report['url'] = oops.createhooks.safe_unicode(request.URL)
if WebServiceLayer.providedBy(request) and info is not None:
webservice_error = getattr(
@@ -251,27 +203,25 @@
report['ignore'] = True
if principal is not None and principal is not missing:
- username = _safestr(
- ', '.join([
- unicode(login),
- unicode(request.principal.id),
- unicode(request.principal.title),
- unicode(request.principal.description)]))
+ username = ', '.join([
+ unicode(login),
+ unicode(request.principal.id),
+ unicode(request.principal.title),
+ unicode(request.principal.description)])
report['username'] = username
if getattr(request, '_orig_env', None):
- report['pageid'] = request._orig_env.get(
+ report['topic'] = request._orig_env.get(
'launchpad.pageid', '')
for key, value in request.items():
if _is_sensitive(request, key):
- report['req_vars'].append((_safestr(key), '<hidden>'))
+ report['req_vars'].append((key, '<hidden>'))
else:
- report['req_vars'].append(
- (_safestr(key), _safestr(value)))
+ report['req_vars'].append((key, value))
if IXMLRPCRequest.providedBy(request):
args = request.getPositionalArguments()
- report['req_vars'].append(('xmlrpc args', _safestr(args)))
+ report['req_vars'].append(('xmlrpc args', args))
def attach_ignore_from_exception(report, context):
@@ -306,7 +256,7 @@
start, end, category, detail = action.logTuple()
detail = _filter_session_statement(category, detail)
statements.append(
- (start, end, _safestr(category), _safestr(detail)))
+ (start, end, category, detail))
report['db_statements'] = statements
return report
@@ -344,8 +294,6 @@
self._oops_config.template['req_vars'] = []
# Exceptions, with the zope formatter.
self._oops_config.on_create.append(attach_exc_info)
- # Datestamps.
- self._oops_config.on_create.append(attach_date)
# Ignore IUnloggedException exceptions
self._oops_config.on_create.append(attach_ignore_from_exception)
# Zope HTTP requests have lots of goodies.
=== modified file 'lib/canonical/launchpad/webapp/tests/test_errorlog.py'
--- lib/canonical/launchpad/webapp/tests/test_errorlog.py 2011-08-16 06:58:32 +0000
+++ lib/canonical/launchpad/webapp/tests/test_errorlog.py 2011-08-17 01:17:27 +0000
@@ -68,24 +68,21 @@
class TestErrorReport(testtools.TestCase):
- def tearDown(self):
- reset_logging()
- super(TestErrorReport, self).tearDown()
-
def test___init__(self):
"""Test ErrorReport.__init__()"""
entry = ErrorReport('id', 'exc-type', 'exc-value', 'timestamp',
- 'pageid', 'traceback-text', 'username', 'url', 42,
+ 'traceback-text', 'username', 'url', 42,
[('name1', 'value1'), ('name2', 'value2'),
('name1', 'value3')],
[(1, 5, 'store_a', 'SELECT 1'),
(5, 10, 'store_b', 'SELECT 2')],
+ topic='pageid',
)
self.assertEqual(entry.id, 'id')
self.assertEqual(entry.type, 'exc-type')
self.assertEqual(entry.value, 'exc-value')
self.assertEqual(entry.time, 'timestamp')
- self.assertEqual(entry.pageid, 'pageid')
+ self.assertEqual(entry.topic, 'pageid')
self.assertEqual(entry.branch_nick, versioninfo.branch_nick)
self.assertEqual(entry.revno, versioninfo.revno)
self.assertEqual(entry.username, 'username')
@@ -130,7 +127,7 @@
self.assertEqual(entry.value, 'error message')
self.assertEqual(
entry.time, datetime.datetime(2005, 4, 1, tzinfo=UTC))
- self.assertEqual(entry.pageid, 'IFoo:+foo-template')
+ self.assertEqual(entry.topic, 'IFoo:+foo-template')
self.assertEqual(entry.tb_text, 'traceback-text')
self.assertEqual(entry.username, 'Sample User')
self.assertEqual(entry.url, 'http://localhost:9000/foo')
@@ -230,9 +227,9 @@
except ArbitraryException:
report = utility.raising(sys.exc_info(), request)
- # page id is obtained from the request
- self.assertEqual('IFoo:+foo-template', report['pageid'])
- self.assertEqual('Login, 42, title, description |\\u25a0|',
+ # topic is obtained from the request
+ self.assertEqual('IFoo:+foo-template', report['topic'])
+ self.assertEqual(u'Login, 42, title, description |\u25a0|',
report['username'])
self.assertEqual('http://localhost:9000/foo', report['url'])
self.assertEqual({
@@ -241,9 +238,9 @@
'HTTP_COOKIE': '<hidden>',
'HTTP_HOST': '127.0.0.1',
'SERVER_URL': 'http://localhost:9000/foo',
- '\\u25a0': 'value4',
+ u'\u25a0': 'value4',
'lp': '<hidden>',
- 'name1': 'value3 \\xa7',
+ 'name1': 'value3 \xa7',
'name2': 'value2',
}, dict(report['req_vars']))
# verify that the oopsid was set on the request
@@ -261,7 +258,7 @@
raise ArbitraryException('xyz\nabc')
except ArbitraryException:
report = utility.raising(sys.exc_info(), request)
- self.assertEqual(('xmlrpc args', '(1, 2)'), report['req_vars'][-1])
+ self.assertEqual(('xmlrpc args', (1, 2)), report['req_vars'][-1])
def test_raising_with_webservice_request(self):
# Test ErrorReportingUtility.raising() with a WebServiceRequest
@@ -631,7 +628,7 @@
self.assertEqual(exc_value, report['value'])
self.assertThat(report['tb_text'],
StartsWith('Traceback (most recent call last):\n'))
- self.assertEqual(None, report.get('pageid'))
+ self.assertEqual(None, report.get('topic'))
self.assertEqual(None, report.get('username'))
self.assertEqual(None, report.get('url'))
self.assertEqual([], report['req_vars'])
=== modified file 'lib/lp/testing/tests/test_testcase.py'
--- lib/lp/testing/tests/test_testcase.py 2011-08-16 20:35:11 +0000
+++ lib/lp/testing/tests/test_testcase.py 2011-08-17 01:17:27 +0000
@@ -93,8 +93,5 @@
# which is slightly better.
self.assertIsInstance(content.getvalue(), str)
from_details = oops_datedir_repo.serializer_rfc822.read(content)
- # informational is deprecated, but the serializer still inserts it in
- # 0.0.5.
- from_details.pop('informational', None)
oops_report = errorlog.globalErrorUtility.getLastOopsReport()
self.assertEqual(dict(oops_report.__dict__), from_details)
=== modified file 'versions.cfg'
--- versions.cfg 2011-08-16 20:38:35 +0000
+++ versions.cfg 2011-08-17 01:17:27 +0000
@@ -49,8 +49,8 @@
mocker = 0.10.1
mozrunner = 1.3.4
oauth = 1.0
-oops = 0.0.5
-oops-datedir-repo = 0.0.2
+oops = 0.0.6
+oops-datedir-repo = 0.0.3
paramiko = 1.7.4
Paste = 1.7.2
PasteDeploy = 1.3.3
