launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04610
[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/71634
A bunch of cleanup plus deletion of the 'write oops to disk' part of the code base - that is now just one publisher amongst many in the oops config.
I've removed all disk writing in the errorlog tests as they no longer need it. We can do the same for all tests, once we get rid of getLastOopsReport (which requires cross-process OOPS handoffs, for which I Have A Plan).
--
https://code.launchpad.net/~lifeless/launchpad/useoops/+merge/71634
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/launchpad/useoops into lp:launchpad.
=== modified file 'lib/canonical/config/schema-lazr.conf'
--- lib/canonical/config/schema-lazr.conf 2011-08-14 23:11:45 +0000
+++ lib/canonical/config/schema-lazr.conf 2011-08-16 02:33:19 +0000
@@ -288,9 +288,6 @@
# datatype: string
secret_path:
-# Ignored; delete when lp-prod-configs updated.
-copy_to_zlog: False
-
# See [error_reports].
error_dir: none
=== modified file 'lib/canonical/launchpad/scripts/ftests/test_oops_prune.py'
--- lib/canonical/launchpad/scripts/ftests/test_oops_prune.py 2011-08-12 11:19:40 +0000
+++ lib/canonical/launchpad/scripts/ftests/test_oops_prune.py 2011-08-16 02:33:19 +0000
@@ -23,7 +23,7 @@
import tempfile
import unittest
-from oops import uniquefileallocator
+from oops_datedir_repo import uniquefileallocator
from pytz import UTC
import transaction
=== modified file 'lib/canonical/launchpad/scripts/oops.py'
--- lib/canonical/launchpad/scripts/oops.py 2011-08-12 20:41:50 +0000
+++ lib/canonical/launchpad/scripts/oops.py 2011-08-16 02:33:19 +0000
@@ -20,7 +20,7 @@
import os
import re
-from oops import uniquefileallocator
+from oops_datedir_repo import uniquefileallocator
from pytz import utc
from canonical.database.sqlbase import cursor
=== modified file 'lib/canonical/launchpad/webapp/errorlog.py'
--- lib/canonical/launchpad/webapp/errorlog.py 2011-08-15 03:07:48 +0000
+++ lib/canonical/launchpad/webapp/errorlog.py 2011-08-16 02:33:19 +0000
@@ -19,8 +19,9 @@
import urlparse
from lazr.restful.utils import get_current_browser_request
-from oops.uniquefileallocator import UniqueFileAllocator
-import oops.serializer_rfc822
+import oops
+from oops_datedir_repo import DateDirRepo
+import oops_datedir_repo.serializer_rfc822
import pytz
from zope.component.interfaces import ObjectEvent
from zope.error.interfaces import IErrorReportingUtility
@@ -138,7 +139,7 @@
implements(IErrorReport)
def __init__(self, id, type, value, time, pageid, tb_text, username,
- url, duration, req_vars, db_statements, informational,
+ url, duration, req_vars, db_statements, informational=None,
branch_nick=None, revno=None):
self.id = id
self.type = type
@@ -149,11 +150,11 @@
self.username = username
self.url = url
self.duration = duration
+ # informational is ignored - will be going from the oops module soon too.
self.req_vars = req_vars
self.db_statements = db_statements
self.branch_nick = branch_nick or versioninfo.branch_nick
self.revno = revno or versioninfo.revno
- self.informational = informational
def __repr__(self):
return '<ErrorReport %s %s: %s>' % (self.id, self.type, self.value)
@@ -161,10 +162,18 @@
@classmethod
def read(cls, fp):
# Deprecated: use the oops module directly now, when possible.
- report = oops.serializer_rfc822.read(fp)
+ report = oops_datedir_repo.serializer_rfc822.read(fp)
return cls(**report)
+def notify_publisher(report):
+ if not report.get('id'):
+ report['id'] = str(id(report))
+ notify(ErrorReportEvent(report))
+ return report['id']
+
+
+
class ErrorReportingUtility:
implements(IErrorReportingUtility)
@@ -189,15 +198,17 @@
"""
if section_name is None:
section_name = self._default_config_section
- # Start a new UniqueFileAllocator to activate the new configuration.
- self.log_namer = UniqueFileAllocator(
- output_root=config[section_name].error_dir,
- log_type="OOPS",
- log_subtype=config[section_name].oops_prefix,
- )
+ self._oops_config = oops.Config()
+ self._oops_config.template['branch_nick'] = versioninfo.branch_nick
+ self._oops_config.template['revno'] = versioninfo.revno
+ self._oops_datedir_repo = DateDirRepo(
+ config[section_name].error_dir,
+ config[section_name].oops_prefix)
+ self._oops_config.publishers.append(self._oops_datedir_repo.publish)
+ self._oops_config.publishers.append(notify_publisher)
def setOopsToken(self, token):
- return self.log_namer.setToken(token)
+ return self._oops_datedir_repo.log_namer.setToken(token)
@property
def oops_prefix(self):
@@ -205,17 +216,18 @@
This is the log subtype + anything set via setOopsToken.
"""
- return self.log_namer.get_log_infix()
+ return self._oops_datedir_repo.log_namer.get_log_infix()
def getOopsReport(self, time):
"""Return the contents of the OOPS report logged at 'time'."""
# How this works - get a serial that was logging in the dir
# that logs for time are logged in.
- serial_from_time = self.log_namer._findHighestSerial(
- self.log_namer.output_dir(time))
+ serial_from_time = self._oops_datedir_repo.log_namer._findHighestSerial(
+ self._oops_datedir_repo.log_namer.output_dir(time))
# Calculate a filename which combines this most recent serial,
# the current log_namer naming rules and the exact timestamp.
- oops_filename = self.log_namer.getFilename(serial_from_time, time)
+ oops_filename = self._oops_datedir_repo.log_namer.getFilename(
+ serial_from_time, time)
# Note that if there were no logs written, or if there were two
# oops that matched the time window of directory on disk, this
# call can raise an IOError.
@@ -235,7 +247,8 @@
If no report is found, return None.
"""
suffix = re.search('[0-9]*$', oops_id).group(0)
- for directory, name in self.log_namer.listRecentReportFiles():
+ for directory, name in \
+ self._oops_datedir_repo.log_namer.listRecentReportFiles():
if not name.endswith(suffix):
continue
with open(os.path.join(directory, name), 'r') as oops_report_file:
@@ -261,11 +274,12 @@
"""
now = datetime.datetime.now(UTC)
# Check today
- oopsid, filename = self.log_namer._findHighestSerialFilename(time=now)
+ log_namer = self._oops_datedir_repo.log_namer
+ oopsid, filename = log_namer._findHighestSerialFilename(time=now)
if filename is None:
# Check yesterday, we may have just passed midnight.
yesterday = now - datetime.timedelta(days=1)
- oopsid, filename = self.log_namer._findHighestSerialFilename(
+ oopsid, filename = log_namer._findHighestSerialFilename(
time=yesterday)
if filename is None:
return None
@@ -275,41 +289,17 @@
finally:
oops_report.close()
- def raising(self, info, request=None, now=None):
- """See IErrorReportingUtility.raising()
-
- :param now: The datetime to use as the current time. Will be
- determined if not supplied. Useful for testing. Not part of
- IErrorReportingUtility).
- """
- return self._raising(
- info, request=request, now=now, informational=False)
-
- def _raising(self, info, request=None, now=None, informational=False):
- """Private method used by raising() and handling()."""
- report = self._makeReport(info, request, now, informational)
+ def raising(self, info, request=None):
+ """See IErrorReportingUtility.raising()"""
+ report = self._makeReport(info, request)
if self._filterReport(report):
return
- self._sendReport(report, now=now)
+ self._oops_config.publish(report)
if request:
- request.oopsid = report['id']
+ request.oopsid = report.get('id')
request.oops = report
return report
- def _sendReport(self, report, now=None):
- if now is not None:
- now = now.astimezone(UTC)
- else:
- now = datetime.datetime.now(UTC)
- oopsid, filename = self.log_namer.newId(now)
- report['id'] = oopsid
- oops.serializer_rfc822.write(report, open(filename, 'wb'))
- # Set file permission to: rw-r--r--
- wanted_permission = (
- stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH)
- os.chmod(filename, wanted_permission)
- notify(ErrorReportEvent(report))
-
def filter_session_statement(self, database_id, statement):
"""Replace quoted strings with '%s' in statements on session DB."""
if database_id == 'SQL-' + PGSessionBase.store_name:
@@ -349,22 +339,14 @@
return True
return False
- def _makeReport(self, info, request=None, now=None, informational=False):
+ def _makeReport(self, info, request=None):
"""Create an unallocated OOPS.
:param info: Output of sys.exc_info()
:param request: The IErrorReportRequest which provides context to the
info.
- :param now: The datetime to use as the current time. Will be
- determined if not supplied. Useful for testing.
- :param informational: If true, the report is flagged as informational
- only.
"""
- if now is not None:
- now = now.astimezone(UTC)
- else:
- now = datetime.datetime.now(UTC)
- report = {}
+ report = self._oops_config.create()
report['type'] = _safestr(getattr(info[0], '__name__', info[0]))
report['value'] = _safestr(info[1])
if not isinstance(info[2], basestring):
@@ -374,10 +356,7 @@
tb_text = info[2]
report['tb_text'] = _safestr(tb_text)
report['req_vars'] = []
- report['time'] = now
- report['informational'] = informational
- report['branch_nick'] = versioninfo.branch_nick
- report['revno'] = versioninfo.revno
+ report['time'] = datetime.datetime.now(UTC)
# Because of IUnloggedException being a sidewards lookup we must
# capture this here to filter on later.
report['ignore'] = IUnloggedException.providedBy(info[1])
@@ -460,19 +439,6 @@
args = request.getPositionalArguments()
report['req_vars'].append(('xmlrpc args', _safestr(args)))
- def handling(self, info, request=None, now=None):
- """Flag ErrorReport as informational only.
-
- :param info: Output of sys.exc_info()
- :param request: The IErrorReportRequest which provides context to the
- info.
- :param now: The datetime to use as the current time. Will be
- determined if not supplied. Useful for testing.
- :return: The ErrorReport created.
- """
- return self._raising(
- info, request=request, now=now, informational=True)
-
@contextlib.contextmanager
def oopsMessage(self, message):
"""Add an oops message to be included in oopses from this context."""
@@ -583,7 +549,7 @@
request.oopsid is not None or
not request.annotations.get(LAZR_OOPS_USER_REQUESTED_KEY, False)):
return None
- globalErrorUtility.handling(
+ globalErrorUtility.raising(
(UserRequestOops, UserRequestOops(), None), request)
return request.oopsid
=== modified file 'lib/canonical/launchpad/webapp/publication.py'
--- lib/canonical/launchpad/webapp/publication.py 2011-08-14 07:51:06 +0000
+++ lib/canonical/launchpad/webapp/publication.py 2011-08-16 02:33:19 +0000
@@ -623,10 +623,12 @@
# the publication, so there's nothing we need to do here.
pass
- # Log a soft OOPS for DisconnectionErrors as per Bug #373837.
+ # Log an OOPS for DisconnectionErrors: we don't expect to see
+ # disconnections as a routine event, so having information about them
+ # is important. See Bug #373837 for more information.
# We need to do this before we re-raise the exception as a Retry.
if isinstance(exc_info[1], DisconnectionError):
- getUtility(IErrorReportingUtility).handling(exc_info, request)
+ getUtility(IErrorReportingUtility).raising(exc_info, request)
def should_retry(exc_info):
if not retry_allowed:
@@ -760,7 +762,7 @@
# not happen, as store.rollback() should have been called
# by now. Log an OOPS so we know about this. This
# is Bug #504291 happening.
- getUtility(IErrorReportingUtility).handling(
+ getUtility(IErrorReportingUtility).raising(
sys.exc_info(), request)
# Repair things so the server can remain operational.
store.rollback()
=== modified file 'lib/canonical/launchpad/webapp/tests/test_errorlog.py'
--- lib/canonical/launchpad/webapp/tests/test_errorlog.py 2011-08-15 03:07:48 +0000
+++ lib/canonical/launchpad/webapp/tests/test_errorlog.py 2011-08-16 02:33:19 +0000
@@ -17,11 +17,13 @@
from textwrap import dedent
import traceback
+from fixtures import TempDir
from lazr.batchnavigator.interfaces import InvalidBatchSizeError
from lazr.restful.declarations import error_status
-from oops.uniquefileallocator import UniqueFileAllocator
+from oops_datedir_repo import DateDirRepo
import pytz
import testtools
+from testtools.matchers import StartsWith
from zope.app.publication.tests.test_zopepublication import (
UnauthenticatedPrincipal,
)
@@ -42,6 +44,7 @@
_is_sensitive,
ErrorReport,
ErrorReportingUtility,
+ notify_publisher,
OopsLoggingHandler,
ScriptRequest,
)
@@ -56,7 +59,6 @@
)
from lp.services.osutils import remove_tree
from lp.services.timeline.requesttimeline import get_request_timeline
-from lp.testing import TestCase
from lp_sitecustomize import customize_get_converter
@@ -81,7 +83,7 @@
('name1', 'value3')],
[(1, 5, 'store_a', 'SELECT 1'),
(5, 10, 'store_b', 'SELECT 2')],
- False)
+ )
self.assertEqual(entry.id, 'id')
self.assertEqual(entry.type, 'exc-type')
self.assertEqual(entry.value, 'exc-value')
@@ -92,7 +94,6 @@
self.assertEqual(entry.username, 'username')
self.assertEqual(entry.url, 'url')
self.assertEqual(entry.duration, 42)
- self.assertEqual(entry.informational, False)
self.assertEqual(len(entry.req_vars), 3)
self.assertEqual(entry.req_vars[0], ('name1', 'value1'))
self.assertEqual(entry.req_vars[1], ('name2', 'value2'))
@@ -158,22 +159,13 @@
super(TestErrorReportingUtility, self).setUp()
# ErrorReportingUtility reads the global config to get the
# current error directory.
+ tempdir = self.useFixture(TempDir()).path
test_data = dedent("""
[error_reports]
error_dir: %s
- """ % tempfile.mkdtemp())
+ """ % tempdir)
config.push('test_data', test_data)
- shutil.rmtree(config.error_reports.error_dir, ignore_errors=True)
-
- def tearDown(self):
- shutil.rmtree(config.error_reports.error_dir, ignore_errors=True)
- config.pop('test_data')
- reset_logging()
- super(TestErrorReportingUtility, self).tearDown()
-
- def test_sets_log_namer_to_a_UniqueFileAllocator(self):
- utility = ErrorReportingUtility()
- self.assertIsInstance(utility.log_namer, UniqueFileAllocator)
+ self.addCleanup(config.pop, 'test_data')
def test_configure(self):
"""Test ErrorReportingUtility.setConfigSection()."""
@@ -183,21 +175,30 @@
self.assertEqual(config.error_reports.oops_prefix,
utility.oops_prefix)
self.assertEqual(config.error_reports.error_dir,
- utility.log_namer._output_root)
+ utility._oops_datedir_repo.log_namer._output_root)
# Some external processes may use another config section to
# provide the error log configuration.
utility.configure(section_name='branchscanner')
self.assertEqual(config.branchscanner.oops_prefix,
utility.oops_prefix)
self.assertEqual(config.branchscanner.error_dir,
- utility.log_namer._output_root)
+ utility._oops_datedir_repo.log_namer._output_root)
# The default error section can be restored.
utility.configure()
self.assertEqual(config.error_reports.oops_prefix,
utility.oops_prefix)
self.assertEqual(config.error_reports.error_dir,
- utility.log_namer._output_root)
+ utility._oops_datedir_repo.log_namer._output_root)
+
+ # We should have had two publishers setup:
+ oops_config = utility._oops_config
+ self.assertEqual(2, len(oops_config.publishers))
+ # - a datedir publisher
+ datedir_repo = utility._oops_datedir_repo
+ self.assertEqual(oops_config.publishers[0], datedir_repo.publish)
+ # - a notify publisher
+ self.assertEqual(oops_config.publishers[1], notify_publisher)
def test_setOopsToken(self):
"""Test ErrorReportingUtility.setOopsToken()."""
@@ -209,35 +210,10 @@
utility.setOopsToken('1')
self.assertEqual('T1', utility.oops_prefix)
- def test_raising_permissions(self):
- """Test ErrorReportingUtility.raising() with no request"""
- utility = ErrorReportingUtility()
- report = {'id': 'OOPS-91T1'}
- now = datetime.datetime(2006, 04, 01, 00, 30, 00, tzinfo=UTC)
-
- # Set up default file creation mode to rwx------ as some restrictive
- # servers do.
- umask_permission = stat.S_IRWXG | stat.S_IRWXO
- old_umask = os.umask(umask_permission)
- self.addCleanup(os.umask, old_umask)
- utility._sendReport(report, now)
-
- errorfile = os.path.join(
- utility.log_namer.output_dir(now), '01800.T1')
- self.assertTrue(os.path.exists(errorfile))
-
- # Check errorfile is set with the correct permission: rw-r--r--
- st = os.stat(errorfile)
- wanted_permission = (
- stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH)
- # Get only the permission bits for this file.
- file_permission = stat.S_IMODE(st.st_mode)
- self.assertEqual(file_permission, wanted_permission)
-
def test_raising_with_request(self):
"""Test ErrorReportingUtility.raising() with a request"""
utility = ErrorReportingUtility()
- now = datetime.datetime(2006, 04, 01, 00, 30, 00, tzinfo=UTC)
+ del utility._oops_config.publishers[0]
request = TestRequestWithPrincipal(
environ={
@@ -255,59 +231,27 @@
try:
raise ArbitraryException('xyz\nabc')
except ArbitraryException:
- utility.raising(sys.exc_info(), request, now=now)
-
- errorfile = os.path.join(
- utility.log_namer.output_dir(now), '01800.T1')
- self.assertTrue(os.path.exists(errorfile))
- lines = open(errorfile, 'r').readlines()
-
- # the header
- self.assertEqual(lines.pop(0), 'Oops-Id: OOPS-91T1\n')
- self.assertEqual(lines.pop(0), 'Exception-Type: ArbitraryException\n')
- self.assertEqual(lines.pop(0), 'Exception-Value: xyz abc\n')
- self.assertEqual(lines.pop(0), 'Date: 2006-04-01T00:30:00+00:00\n')
- self.assertEqual(lines.pop(0), 'Page-Id: IFoo:+foo-template\n')
- self.assertEqual(
- lines.pop(0), 'Branch: %s\n' % versioninfo.branch_nick)
- self.assertEqual(lines.pop(0), 'Revision: %s\n' % versioninfo.revno)
- self.assertEqual(
- lines.pop(0), 'User: Login, 42, title, description |\\u25a0|\n')
- self.assertEqual(lines.pop(0), 'URL: http://localhost:9000/foo\n')
- self.assertEqual(lines.pop(0), 'Duration: -1\n')
- self.assertEqual(lines.pop(0), 'Informational: False\n')
- self.assertEqual(lines.pop(0), '\n')
-
- # request vars
- self.assertEqual(lines.pop(0), 'CONTENT_LENGTH=0\n')
- self.assertEqual(
- lines.pop(0), 'GATEWAY_INTERFACE=TestFooInterface/1.0\n')
- self.assertEqual(lines.pop(0), 'HTTP_COOKIE=%3Chidden%3E\n')
- self.assertEqual(lines.pop(0), 'HTTP_HOST=127.0.0.1\n')
- self.assertEqual(
- lines.pop(0), 'SERVER_URL=http://localhost:9000/foo\n')
-
- # non-ASCII request var
- self.assertEqual(lines.pop(0), '\\u25a0=value4\n')
- self.assertEqual(lines.pop(0), 'lp=%3Chidden%3E\n')
- self.assertEqual(lines.pop(0), 'name1=value3 \\xa7\n')
- self.assertEqual(lines.pop(0), 'name2=value2\n')
- self.assertEqual(lines.pop(0), '\n')
-
- # no database statements
- self.assertEqual(lines.pop(0), '\n')
-
- # traceback
- self.assertEqual(lines.pop(0), 'Traceback (most recent call last):\n')
- # Module canonical.launchpad.webapp.ftests.test_errorlog, ...
- # raise ArbitraryException(\'xyz\')
- lines.pop(0)
- lines.pop(0)
- self.assertEqual(lines.pop(0), 'ArbitraryException: xyz\n')
-
+ 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|',
+ report['username'])
+ self.assertEqual('http://localhost:9000/foo', report['url'])
+ self.assertEqual({
+ 'CONTENT_LENGTH': '0',
+ 'GATEWAY_INTERFACE': 'TestFooInterface/1.0',
+ 'HTTP_COOKIE': '<hidden>',
+ 'HTTP_HOST': '127.0.0.1',
+ 'SERVER_URL': 'http://localhost:9000/foo',
+ '\\u25a0': 'value4',
+ 'lp': '<hidden>',
+ 'name1': 'value3 \\xa7',
+ 'name2': 'value2',
+ }, dict(report['req_vars']))
# verify that the oopsid was set on the request
- self.assertEqual(request.oopsid, 'OOPS-91T1')
- self.assertEqual(request.oops['id'], 'OOPS-91T1')
+ self.assertEqual(request.oopsid, report['id'])
+ self.assertEqual(request.oops, report)
def test_raising_with_xmlrpc_request(self):
# Test ErrorReportingUtility.raising() with an XML-RPC request.
@@ -315,6 +259,7 @@
directlyProvides(request, IXMLRPCRequest)
request.getPositionalArguments = lambda: (1, 2)
utility = ErrorReportingUtility()
+ del utility._oops_config.publishers[:]
try:
raise ArbitraryException('xyz\nabc')
except ArbitraryException:
@@ -327,14 +272,14 @@
request = TestRequest()
directlyProvides(request, WebServiceLayer)
utility = ErrorReportingUtility()
- now = datetime.datetime(2006, 04, 01, 00, 30, 00, tzinfo=UTC)
+ del utility._oops_config.publishers[:]
# Exceptions that don't use error_status result in OOPSes.
try:
raise ArbitraryException('xyz\nabc')
except ArbitraryException:
- utility.raising(sys.exc_info(), request, now=now)
- self.assertNotEqual(request.oopsid, None)
+ self.assertNotEqual(None,
+ utility.raising(sys.exc_info(), request))
# Exceptions with a error_status in the 500 range result
# in OOPSes.
@@ -344,8 +289,8 @@
try:
raise InternalServerError("")
except InternalServerError:
- utility.raising(sys.exc_info(), request, now=now)
- self.assertNotEqual(request.oopsid, None)
+ self.assertNotEqual(None,
+ utility.raising(sys.exc_info(), request))
# Exceptions with any other error_status do not result
# in OOPSes.
@@ -355,12 +300,12 @@
try:
raise BadDataError("")
except BadDataError:
- utility.raising(sys.exc_info(), request, now=now)
- self.assertEqual(request.oopsid, None)
+ self.assertEqual(None, utility.raising(sys.exc_info(), request))
def test_raising_for_script(self):
"""Test ErrorReportingUtility.raising with a ScriptRequest."""
utility = ErrorReportingUtility()
+ del utility._oops_config.publishers[:]
req_vars = [
('name2', 'value2'), ('name1', 'value1'),
@@ -384,6 +329,7 @@
__repr__ = __str__
utility = ErrorReportingUtility()
+ del utility._oops_config.publishers[:]
try:
raise UnprintableException()
except UnprintableException:
@@ -397,6 +343,7 @@
def test_raising_unauthorized_without_request(self):
"""Unauthorized exceptions are logged when there's no request."""
utility = ErrorReportingUtility()
+ del utility._oops_config.publishers[:]
try:
raise Unauthorized('xyz')
except Unauthorized:
@@ -407,43 +354,36 @@
"""Unauthorized exceptions are logged when the request has no
principal."""
utility = ErrorReportingUtility()
- now = datetime.datetime(2006, 04, 01, 00, 30, 00, tzinfo=UTC)
+ del utility._oops_config.publishers[:]
request = ScriptRequest([('name2', 'value2')])
try:
raise Unauthorized('xyz')
except Unauthorized:
- utility.raising(sys.exc_info(), request, now=now)
- errorfile = os.path.join(
- utility.log_namer.output_dir(now), '01800.T1')
- self.failUnless(os.path.exists(errorfile))
+ self.assertNotEqual(None,
+ utility.raising(sys.exc_info(), request))
def test_raising_unauthorized_with_unauthenticated_principal(self):
"""Unauthorized exceptions are not logged when the request has an
unauthenticated principal."""
utility = ErrorReportingUtility()
- now = datetime.datetime(2006, 04, 01, 00, 30, 00, tzinfo=UTC)
+ del utility._oops_config.publishers[:]
request = TestRequestWithUnauthenticatedPrincipal()
try:
raise Unauthorized('xyz')
except Unauthorized:
- utility.raising(sys.exc_info(), request, now=now)
- errorfile = os.path.join(
- utility.log_namer.output_dir(now), '01800.T1')
- self.failIf(os.path.exists(errorfile))
+ self.assertEqual(None, utility.raising(sys.exc_info(), request))
def test_raising_unauthorized_with_authenticated_principal(self):
"""Unauthorized exceptions are logged when the request has an
authenticated principal."""
utility = ErrorReportingUtility()
- now = datetime.datetime(2006, 04, 01, 00, 30, 00, tzinfo=UTC)
+ del utility._oops_config.publishers[:]
request = TestRequestWithPrincipal()
try:
raise Unauthorized('xyz')
except Unauthorized:
- utility.raising(sys.exc_info(), request, now=now)
- errorfile = os.path.join(
- utility.log_namer.output_dir(now), '01800.T1')
- self.failUnless(os.path.exists(errorfile))
+ self.assertNotEqual(None,
+ utility.raising(sys.exc_info(), request))
def test_raising_translation_unavailable(self):
"""Test ErrorReportingUtility.raising() with a TranslationUnavailable
@@ -453,23 +393,19 @@
raised.
"""
utility = ErrorReportingUtility()
- now = datetime.datetime(2006, 04, 01, 00, 30, 00, tzinfo=UTC)
-
+ del utility._oops_config.publishers[:]
+ self.assertTrue(
+ TranslationUnavailable.__name__ in utility._ignored_exceptions,
+ 'TranslationUnavailable is not in _ignored_exceptions.')
try:
raise TranslationUnavailable('xyz')
except TranslationUnavailable:
- utility.raising(sys.exc_info(), now=now)
-
- self.assertTrue(
- TranslationUnavailable.__name__ in utility._ignored_exceptions,
- 'TranslationUnavailable is not in _ignored_exceptions.')
- errorfile = os.path.join(
- utility.log_namer.output_dir(now), '01800.T1')
- self.assertFalse(os.path.exists(errorfile))
+ self.assertEqual(None, utility.raising(sys.exc_info()))
def test_ignored_exceptions_for_offsite_referer(self):
# Exceptions caused by bad URLs that may not be an Lp code issue.
utility = ErrorReportingUtility()
+ del utility._oops_config.publishers[:]
errors = set([
GoneError.__name__, InvalidBatchSizeError.__name__,
NotFound.__name__])
@@ -480,7 +416,7 @@
# Oopses are reported when Launchpad is the referer for a URL
# that caused an exception.
utility = ErrorReportingUtility()
- now = datetime.datetime(2006, 04, 01, 00, 30, 00, tzinfo=UTC)
+ del utility._oops_config.publishers[:]
request = TestRequest(
environ={
'SERVER_URL': 'http://launchpad.dev/fnord',
@@ -488,16 +424,14 @@
try:
raise GoneError('fnord')
except GoneError:
- utility.raising(sys.exc_info(), request, now=now)
- errorfile = os.path.join(
- utility.log_namer.output_dir(now), '01800.T1')
- self.assertTrue(os.path.exists(errorfile))
+ self.assertNotEqual(None,
+ utility.raising(sys.exc_info(), request))
def test_ignored_exceptions_for_cross_vhost_referer_reported(self):
# Oopses are reported when a Launchpad vhost is the referer for a URL
# that caused an exception.
utility = ErrorReportingUtility()
- now = datetime.datetime(2006, 04, 01, 00, 30, 00, tzinfo=UTC)
+ del utility._oops_config.publishers[:]
request = TestRequest(
environ={
'SERVER_URL': 'http://launchpad.dev/fnord',
@@ -505,16 +439,14 @@
try:
raise GoneError('fnord')
except GoneError:
- utility.raising(sys.exc_info(), request, now=now)
- errorfile = os.path.join(
- utility.log_namer.output_dir(now), '01800.T1')
- self.assertTrue(os.path.exists(errorfile))
+ self.assertNotEqual(None,
+ utility.raising(sys.exc_info(), request))
def test_ignored_exceptions_for_criss_cross_vhost_referer_reported(self):
# Oopses are reported when a Launchpad referer for a bad URL on a
# vhost that caused an exception.
utility = ErrorReportingUtility()
- now = datetime.datetime(2006, 04, 01, 00, 30, 00, tzinfo=UTC)
+ del utility._oops_config.publishers[:]
request = TestRequest(
environ={
'SERVER_URL': 'http://bazaar.launchpad.dev/fnord',
@@ -522,25 +454,20 @@
try:
raise GoneError('fnord')
except GoneError:
- utility.raising(sys.exc_info(), request, now=now)
- errorfile = os.path.join(
- utility.log_namer.output_dir(now), '01800.T1')
- self.assertTrue(os.path.exists(errorfile))
+ self.assertNotEqual(
+ None, utility.raising(sys.exc_info(), request))
def test_ignored_exceptions_for_offsite_referer_not_reported(self):
# Oopses are not reported when Launchpad is not the referer.
utility = ErrorReportingUtility()
- now = datetime.datetime(2006, 04, 01, 00, 30, 00, tzinfo=UTC)
+ del utility._oops_config.publishers[:]
# There is no HTTP_REFERER header in this request
request = TestRequest(
environ={'SERVER_URL': 'http://launchpad.dev/fnord'})
try:
raise GoneError('fnord')
except GoneError:
- utility.raising(sys.exc_info(), request, now=now)
- errorfile = os.path.join(
- utility.log_namer.output_dir(now), '01800.T1')
- self.assertFalse(os.path.exists(errorfile))
+ self.assertEqual(None, utility.raising(sys.exc_info(), request))
def test_raising_no_referrer_error(self):
"""Test ErrorReportingUtility.raising() with a NoReferrerError
@@ -550,16 +477,11 @@
raised.
"""
utility = ErrorReportingUtility()
- now = datetime.datetime(2006, 04, 01, 00, 30, 00, tzinfo=UTC)
-
+ del utility._oops_config.publishers[:]
try:
raise NoReferrerError('xyz')
except NoReferrerError:
- utility.raising(sys.exc_info(), now=now)
-
- errorfile = os.path.join(
- utility.log_namer.output_dir(now), '01800.T1')
- self.assertFalse(os.path.exists(errorfile))
+ self.assertEqual(None, utility.raising(sys.exc_info()))
def test_raising_with_string_as_traceback(self):
# ErrorReportingUtility.raising() can be called with a string in the
@@ -577,24 +499,15 @@
exc_tb = traceback.format_exc()
utility = ErrorReportingUtility()
+ del utility._oops_config.publishers[:]
report = utility.raising((exc_type, exc_value, exc_tb))
# traceback is what we supplied.
self.assertEqual(exc_tb, report['tb_text'])
- def test_handling(self):
- """Test ErrorReportingUtility.handling()."""
- utility = ErrorReportingUtility()
-
- try:
- raise ArbitraryException('xyz')
- except ArbitraryException:
- report = utility.handling(sys.exc_info())
-
- self.assertEqual(report['informational'], True)
-
def test_oopsMessage(self):
"""oopsMessage pushes and pops the messages."""
utility = ErrorReportingUtility()
+ del utility._oops_config.publishers[:]
with utility.oopsMessage({'a': 'b', 'c': 'd'}):
self.assertEqual(
{0: {'a': 'b', 'c': 'd'}}, utility._oops_messages)
@@ -612,6 +525,7 @@
def test__makeErrorReport_includes_oops_messages(self):
"""The error report should include the oops messages."""
utility = ErrorReportingUtility()
+ del utility._oops_config.publishers[:]
with utility.oopsMessage(dict(a='b', c='d')):
try:
raise ArbitraryException('foo')
@@ -625,6 +539,7 @@
def test__makeErrorReport_combines_request_and_error_vars(self):
"""The oops messages should be distinct from real request vars."""
utility = ErrorReportingUtility()
+ del utility._oops_config.publishers[:]
request = ScriptRequest([('c', 'd')])
with utility.oopsMessage(dict(a='b')):
try:
@@ -639,6 +554,7 @@
def test_filter_session_statement(self):
"""Removes quoted strings if database_id is SQL-session."""
utility = ErrorReportingUtility()
+ del utility._oops_config.publishers[:]
statement = "SELECT 'gone'"
self.assertEqual(
"SELECT '%s'",
@@ -647,6 +563,7 @@
def test_filter_session_statement_noop(self):
"""If database_id is not SQL-session, it's a no-op."""
utility = ErrorReportingUtility()
+ del utility._oops_config.publishers[:]
statement = "SELECT 'gone'"
self.assertEqual(
statement,
@@ -655,6 +572,7 @@
def test_session_queries_filtered(self):
"""Test that session queries are filtered."""
utility = ErrorReportingUtility()
+ del utility._oops_config.publishers[:]
request = ScriptRequest([], URL="test_session_queries_filtered")
set_request_started()
try:
@@ -709,7 +627,7 @@
return u'Login'
-class TestOopsLoggingHandler(TestCase):
+class TestOopsLoggingHandler(testtools.TestCase):
"""Tests for a Python logging handler that logs OOPSes."""
def assertOopsMatches(self, report, exc_type, exc_value):
@@ -719,36 +637,39 @@
:param exc_type: The string of an exception type.
:param exc_value: The string of an exception value.
"""
- self.assertEqual(exc_type, report.type)
- self.assertEqual(exc_value, report.value)
- self.assertTrue(
- report.tb_text.startswith('Traceback (most recent call last):\n'),
- report.tb_text)
- self.assertEqual(None, report.pageid)
- self.assertEqual(None, report.username)
- self.assertEqual(None, report.url)
- self.assertEqual([], report.req_vars)
- self.assertEqual([], report.db_statements)
+ self.assertEqual(exc_type, report['type'])
+ 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('username'))
+ self.assertEqual(None, report.get('url'))
+ self.assertEqual([], report['req_vars'])
+ self.assertEqual([], report['db_statements'])
def setUp(self):
- TestCase.setUp(self)
- self.logger = logging.getLogger(self.factory.getUniqueString())
+ super(TestOopsLoggingHandler, self).setUp()
+ self.logger = logging.getLogger(self.getUniqueString())
self.error_utility = ErrorReportingUtility()
- self.error_utility.log_namer._output_root = tempfile.mkdtemp()
+ self.oopses = []
+ def publish(report):
+ report['id'] = str(len(self.oopses))
+ self.oopses.append(report)
+ return report.get('id')
+ del self.error_utility._oops_config.publishers[:]
+ self.error_utility._oops_config.publishers.append(publish)
self.logger.addHandler(
OopsLoggingHandler(error_utility=self.error_utility))
- self.addCleanup(
- remove_tree, self.error_utility.log_namer._output_root)
def test_exception_records_oops(self):
# When OopsLoggingHandler is a handler for a logger, any exceptions
# logged will have OOPS reports generated for them.
- error_message = self.factory.getUniqueString()
+ error_message = self.getUniqueString()
try:
1 / 0
except ZeroDivisionError:
self.logger.exception(error_message)
- oops_report = self.error_utility.getLastOopsReport()
+ oops_report = self.oopses[-1]
self.assertOopsMatches(
oops_report, 'ZeroDivisionError',
'integer division or modulo by zero')
@@ -756,12 +677,12 @@
def test_warning_does_nothing(self):
# Logging a warning doesn't generate an OOPS.
self.logger.warning("Cheeseburger")
- self.assertIs(None, self.error_utility.getLastOopsReport())
+ self.assertEqual(0, len(self.oopses))
def test_error_does_nothing(self):
# Logging an error without an exception does nothing.
self.logger.error("Delicious ponies")
- self.assertIs(None, self.error_utility.getLastOopsReport())
+ self.assertEqual(0, len(self.oopses))
class TestOopsIgnoring(testtools.TestCase):
=== modified file 'lib/canonical/launchpad/webapp/tests/test_publication.py'
--- lib/canonical/launchpad/webapp/tests/test_publication.py 2011-08-14 07:51:06 +0000
+++ lib/canonical/launchpad/webapp/tests/test_publication.py 2011-08-16 02:33:19 +0000
@@ -293,9 +293,6 @@
self.assertTrue(repr(next_oops).find("DisconnectionError") != -1,
"next_oops was %r" % next_oops)
- # Ensure the OOPS is correctly marked as informational only.
- self.assertEqual(next_oops.informational, 'True')
-
# Ensure that it is different to the last logged OOPS.
self.assertNotEqual(repr(last_oops), repr(next_oops))
@@ -326,9 +323,6 @@
# Ensure the OOPS mentions the correct exception
self.assertNotEqual(repr(next_oops).find("Bug #504291"), -1)
- # Ensure the OOPS is correctly marked as informational only.
- self.assertEqual(next_oops.informational, 'True')
-
# Ensure the store has been rolled back and in a usable state.
self.assertEqual(store._connection._state, STATE_RECONNECT)
store.find(EmailAddress).first() # Confirms Store is working.
=== modified file 'lib/lp/hardwaredb/browser/hwdb.py'
--- lib/lp/hardwaredb/browser/hwdb.py 2010-11-23 23:22:27 +0000
+++ lib/lp/hardwaredb/browser/hwdb.py 2011-08-16 02:33:19 +0000
@@ -69,10 +69,6 @@
missing_fields = expected_fields.difference(submitted_fields)
if len(missing_fields) > 0:
missing_fields = ', '.join(sorted(missing_fields))
- info = (HWSubmissionMissingFields,
- 'Missing form fields: %s' % missing_fields, None)
- errorUtility = getUtility(IErrorReportingUtility)
- errorUtility.handling(info, self.request)
self.addCustomHeader(
'Error: Required fields not contained in POST data: '
+ missing_fields)
=== modified file 'lib/lp/hardwaredb/stories/hwdb/xx-hwdb.txt'
--- lib/lp/hardwaredb/stories/hwdb/xx-hwdb.txt 2011-08-12 06:24:24 +0000
+++ lib/lp/hardwaredb/stories/hwdb/xx-hwdb.txt 2011-08-16 02:33:19 +0000
@@ -217,38 +217,6 @@
POST data: submission_data
...
-Also, an OOPS is reported.
-
- >>> from canonical.launchpad.webapp.errorlog import ErrorReportingUtility
- >>> error_utility = ErrorReportingUtility()
- >>> error_report = error_utility.getLastOopsReport()
- >>> error_report.type
- 'HWSubmissionMissingFields'
- >>> error_report.value
- 'Missing form fields: submission_data'
- >>> error_report.pageid
- 'HWDBApplication:+submit'
-
- >>> del form_data['field.distribution']
- >>> browser.open('http://launchpad.dev/+hwdb/+submit',
- ... data=urlencode(form_data))
- >>> print browser.headers
- Status: 200 Ok
- ...
- X-Launchpad-Hwdb-Submission: Error: Required fields not contained in
- POST data: distribution, submission_data
- ...
-
- >>> error_report = error_utility.getLastOopsReport()
- >>> error_report.type
- 'HWSubmissionMissingFields'
- >>> error_report.value
- 'Missing form fields: distribution, submission_data'
- >>> error_report.pageid
- 'HWDBApplication:+submit'
- >>> error_report.informational
- 'True'
-
= Views for raw submission data. =
The raw submission data is listed by submitter and by system fingerprint.
=== modified file 'lib/lp/services/job/tests/test_runner.py'
--- lib/lp/services/job/tests/test_runner.py 2011-08-12 21:38:36 +0000
+++ lib/lp/services/job/tests/test_runner.py 2011-08-16 02:33:19 +0000
@@ -206,7 +206,7 @@
try:
raise ValueError('Fake exception. Foobar, I say!')
except ValueError:
- reporter.handling(sys.exc_info())
+ reporter.raising(sys.exc_info())
job_1.run = handleError
runner = JobRunner([job_1, job_2])
runner.runAll()
=== modified file 'lib/lp/services/profile/profile.py'
--- lib/lp/services/profile/profile.py 2011-08-12 06:40:54 +0000
+++ lib/lp/services/profile/profile.py 2011-08-16 02:33:19 +0000
@@ -20,7 +20,7 @@
from bzrlib import errors
from bzrlib import lsprof
-import oops.serializer_rfc822
+import oops_datedir_repo.serializer_rfc822
from zope.pagetemplate.pagetemplatefile import PageTemplateFile
from zope.app.publication.interfaces import IEndRequestEvent
from zope.component import (
@@ -303,7 +303,7 @@
# information.
info = (ProfilingOops, None, None)
error_utility = getUtility(IErrorReportingUtility)
- oops_report = error_utility.handling(info, request)
+ oops_report = error_utility.raising(info, request)
oopsid = oops_report['id']
else:
oops_report = request.oops
@@ -328,7 +328,7 @@
# Generate rfc822 OOPS result (might be nice to have an html
# serializer..).
template_context['oops'] = ''.join(
- oops.serializer_rfc822.to_chunks(oops_report))
+ oops_datedir_repo.serializer_rfc822.to_chunks(oops_report))
# Generate profile summaries.
prof_stats.strip_dirs()
for name in ('time', 'cumulative', 'calls'):
=== modified file 'lib/lp/services/profile/tests.py'
--- lib/lp/services/profile/tests.py 2011-08-12 06:40:54 +0000
+++ lib/lp/services/profile/tests.py 2011-08-16 02:33:19 +0000
@@ -571,18 +571,10 @@
of the functionality.
"""
- def test_profiling_oops_is_informational(self):
- self.pushProfilingConfig(profiling_allowed='True')
- request = self.endRequest('/++profile++show/')
- self.assertTrue(request.oops['informational'])
- self.assertEquals(request.oops['type'], 'ProfilingOops')
- self.assertCleanProfilerState()
-
def test_real_oops_trumps_profiling_oops(self):
self.pushProfilingConfig(profiling_allowed='True')
request = self.endRequest('/++profile++show/no-such-file',
KeyError('foo'))
- self.assertFalse(request.oops['informational'])
self.assertEquals(request.oops['type'], 'KeyError')
response = self.getAddedResponse(request)
self.assertIn('Exception-Type: KeyError', response)
=== modified file 'lib/lp/testing/__init__.py'
--- lib/lp/testing/__init__.py 2011-08-11 03:15:16 +0000
+++ lib/lp/testing/__init__.py 2011-08-16 02:33:19 +0000
@@ -83,7 +83,7 @@
)
from bzrlib.transport import get_transport
import fixtures
-import oops.serializer_rfc822
+import oops_datedir_repo.serializer_rfc822
import pytz
import simplejson
from storm.expr import Variable
@@ -556,7 +556,8 @@
if len(self.oopses) > 0:
for (i, report) in enumerate(self.oopses):
content = Content(UTF8_TEXT,
- partial(oops.serializer_rfc822.to_chunks, report))
+ partial(oops_datedir_repo.serializer_rfc822.to_chunks,
+ report))
self.addDetail("oops-%d" % i, content)
def attachLibrarianLog(self, fixture):
=== modified file 'lib/lp/testing/tests/test_testcase.py'
--- lib/lp/testing/tests/test_testcase.py 2011-08-13 04:07:10 +0000
+++ lib/lp/testing/tests/test_testcase.py 2011-08-16 02:33:19 +0000
@@ -8,7 +8,7 @@
from StringIO import StringIO
import sys
-import oops.serializer_rfc822
+import oops_datedir_repo.serializer_rfc822
from storm.store import Store
from zope.component import getUtility
@@ -92,6 +92,6 @@
# Safety net: ensure that no autocasts have occured even on Python 2.6
# which is slightly better.
self.assertIsInstance(content.getvalue(), str)
- from_details = oops.serializer_rfc822.read(content)
+ from_details = oops_datedir_repo.serializer_rfc822.read(content)
oops_report = errorlog.globalErrorUtility.getLastOopsReport()
self.assertEqual(dict(oops_report.__dict__), from_details)
=== modified file 'setup.py'
--- setup.py 2011-08-10 06:50:52 +0000
+++ setup.py 2011-08-16 02:33:19 +0000
@@ -57,6 +57,7 @@
'mocker',
'oauth',
'oops',
+ 'oops_datedir_repo',
'paramiko',
'psycopg2',
'python-memcached',
=== modified file 'versions.cfg'
--- versions.cfg 2011-08-15 13:11:05 +0000
+++ versions.cfg 2011-08-16 02:33:19 +0000
@@ -49,7 +49,8 @@
mocker = 0.10.1
mozrunner = 1.3.4
oauth = 1.0
-oops = 0.0.3
+oops = 0.0.4
+oops-datedir-repo = 0.0.2
paramiko = 1.7.4
Paste = 1.7.2
PasteDeploy = 1.3.3
Follow ups
