[patch] show unexpected tracebacks to stderr
Martin Pool
mbp at canonical.com
Thu Jun 15 06:56:31 BST 2006
bzr.log was a nice idea, but it's a bit unobvious.
This patch changes the behaviour when an exception hits the top level to
be this:
- some cases like KeyboardInterrupt or EPIPE are handled specially
- errors caused by a user problem (typo, bad url, etc) are reported
briefly; in general we should only do this when we're confident
that's really what happened.
- everything else is assumed to be a bzr bug and reported to stderr
with a traceback
There are also a handful of cleanups of exceptions and tests for them.
There is scope for more work in cleaning up existing error classes and
tuning just which ones are reported as user errors, but I thought I'd
send this up for +1 as it is.
--
Martin
-------------- next part --------------
=== added file 'bzrlib/tests/blackbox/test_exceptions.py'
--- /dev/null
+++ bzrlib/tests/blackbox/test_exceptions.py
@@ -0,0 +1,37 @@
+# Copyright (C) 2006 by Canonical Ltd
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+
+"""Tests for display of exceptions."""
+
+import os
+import sys
+
+from bzrlib.tests import TestCaseInTempDir, TestCase
+from bzrlib.errors import NotBranchError
+
+class TestExceptionReporting(TestCase):
+
+ def test_report_exception(self):
+ """When an error occurs, display bug report details to stderr"""
+ out, err = self.run_bzr("assert-fail", retcode=3)
+ self.assertContainsRe(err,
+ r'bzr: ERROR: exceptions\.AssertionError: always fails\n')
+ self.assertContainsRe(err, r'please send this report to')
+
+ # TODO: assert-fail doesn't need to always be present; we could just
+ # register (and unregister) it from tests that want to touch it.
+ #
+ # TODO: Some kind of test for the feature of invoking pdb
=== modified file 'bzrlib/branch.py'
--- bzrlib/branch.py
+++ bzrlib/branch.py
@@ -31,7 +31,7 @@
from bzrlib.delta import compare_trees
import bzrlib.errors as errors
from bzrlib.errors import (BzrError, InvalidRevisionNumber, InvalidRevisionId,
- NoSuchRevision, HistoryMissing, NotBranchError,
+ NoSuchRevision, NotBranchError,
DivergedBranches, LockError,
UninitializableFormat,
UnlistableStore,
@@ -300,32 +300,6 @@
If self and other have not diverged, return a list of the revisions
present in other, but missing from self.
-
- >>> from bzrlib.workingtree import WorkingTree
- >>> bzrlib.trace.silent = True
- >>> d1 = bzrdir.ScratchDir()
- >>> br1 = d1.open_branch()
- >>> wt1 = d1.open_workingtree()
- >>> d2 = bzrdir.ScratchDir()
- >>> br2 = d2.open_branch()
- >>> wt2 = d2.open_workingtree()
- >>> br1.missing_revisions(br2)
- []
- >>> wt2.commit("lala!", rev_id="REVISION-ID-1")
- >>> br1.missing_revisions(br2)
- [u'REVISION-ID-1']
- >>> br2.missing_revisions(br1)
- []
- >>> wt1.commit("lala!", rev_id="REVISION-ID-1")
- >>> br1.missing_revisions(br2)
- []
- >>> wt2.commit("lala!", rev_id="REVISION-ID-2A")
- >>> br1.missing_revisions(br2)
- [u'REVISION-ID-2A']
- >>> wt1.commit("lala!", rev_id="REVISION-ID-2B")
- >>> br1.missing_revisions(br2)
- Traceback (most recent call last):
- DivergedBranches: These branches have diverged. Try merge.
"""
self_history = self.revision_history()
self_len = len(self_history)
@@ -572,7 +546,7 @@
except NoSuchFile:
raise NotBranchError(path=transport.base)
except KeyError:
- raise errors.UnknownFormatError(format_string)
+ raise errors.UnknownFormatError(format=format_string)
@classmethod
def get_default_format(klass):
@@ -872,11 +846,7 @@
stacklevel=2)
if (not relax_version_check
and not self._format.is_supported()):
- raise errors.UnsupportedFormatError(
- 'sorry, branch format %r not supported' % fmt,
- ['use a different bzr version',
- 'or remove the .bzr directory'
- ' and "bzr init" again'])
+ raise errors.UnsupportedFormatError(format=fmt)
if deprecated_passed(transport):
warn("BzrBranch.__init__(transport=XXX...): The transport "
"parameter is deprecated as of bzr 0.8. "
=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py
+++ bzrlib/builtins.py
@@ -1937,18 +1937,6 @@
base_rev_id = common_ancestor(last1, last2, source)
print 'merge base is revision %s' % base_rev_id
-
- return
-
- if base_revno is None:
- raise bzrlib.errors.UnrelatedBranches()
-
- print ' r%-6d in %s' % (base_revno, branch)
-
- other_revno = branch2.revision_id_to_revno(base_revid)
-
- print ' r%-6d in %s' % (other_revno, other)
-
class cmd_merge(Command):
=== modified file 'bzrlib/bzrdir.py'
--- bzrlib/bzrdir.py
+++ bzrlib/bzrdir.py
@@ -92,11 +92,7 @@
"""
if not allow_unsupported and not format.is_supported():
# see open_downlevel to open legacy branches.
- raise errors.UnsupportedFormatError(
- 'sorry, format %s not supported' % format,
- ['use a different bzr version',
- 'or remove the .bzr directory'
- ' and "bzr init" again'])
+ raise errors.UnsupportedFormatError(format=format)
def clone(self, url, revision_id=None, basis=None, force_new_repo=False):
"""Clone this bzrdir and its contents to url verbatim.
@@ -945,7 +941,7 @@
except errors.NoSuchFile:
raise errors.NotBranchError(path=transport.base)
except KeyError:
- raise errors.UnknownFormatError(format_string)
+ raise errors.UnknownFormatError(format=format_string)
@classmethod
def get_default_format(klass):
=== modified file 'bzrlib/commands.py'
--- bzrlib/commands.py
+++ bzrlib/commands.py
@@ -34,10 +34,10 @@
import errno
import bzrlib
+import bzrlib.errors as errors
from bzrlib.errors import (BzrError,
+ BzrCommandError,
BzrCheckError,
- BzrCommandError,
- BzrOptionError,
NotBranchError)
from bzrlib.option import Option
from bzrlib.revisionspec import RevisionSpec
@@ -145,7 +145,7 @@
if cmd_obj:
return cmd_obj
- raise BzrCommandError("unknown command %r" % cmd_name)
+ raise BzrCommandError('unknown command "%s"' % cmd_name)
class Command(object):
@@ -233,7 +233,7 @@
for oname in opts:
if oname not in allowed_names:
raise BzrCommandError("option '--%s' is not allowed for"
- " command %r" % (oname, self.name()))
+ " command %r" % (oname, self.name()))
# mix arguments and options into one dictionary
cmdargs = _match_argform(self.name(), self.takes_args, args)
cmdopts = {}
@@ -337,9 +337,7 @@
else:
optname = a[2:]
if optname not in cmd_options:
- raise BzrOptionError('unknown long option %r for'
- ' command %s' %
- (a, command.name()))
+ raise BzrCommandError('unknown option "%s"' % a)
else:
shortopt = a[1:]
if shortopt in Option.SHORT_OPTIONS:
@@ -354,7 +352,7 @@
if shortopt not in Option.SHORT_OPTIONS:
# We didn't find the multi-character name, and we
# didn't find the single char name
- raise BzrError('unknown short option %r' % a)
+ raise BzrCommandError('unknown option "%s"' % a)
optname = Option.SHORT_OPTIONS[shortopt].name
if a[2:]:
@@ -371,15 +369,12 @@
# This option takes an argument, so pack it
# into the array
optarg = a[2:]
-
if optname not in cmd_options:
- raise BzrOptionError('unknown short option %r for'
- ' command %s' %
- (shortopt, command.name()))
+ raise BzrCommandError('unknown option "%s"' % shortopt)
if optname in opts:
# XXX: Do we ever want to support this, e.g. for -r?
if proc_aliasarg:
- raise BzrError('repeated option %r' % a)
+ raise BzrCommandError('repeated option %r' % a)
elif optname in alias_opts:
# Replace what's in the alias with what's in the real
# argument
@@ -388,14 +383,14 @@
proc_argv.insert(0, a)
continue
else:
- raise BzrError('repeated option %r' % a)
+ raise BzrCommandError('repeated option %r' % a)
option_obj = cmd_options[optname]
optargfn = option_obj.type
if optargfn:
if optarg == None:
if not proc_argv:
- raise BzrError('option %r needs an argument' % a)
+ raise BzrCommandError('option %r needs an argument' % a)
else:
optarg = proc_argv.pop(0)
opts[optname] = optargfn(optarg)
@@ -403,7 +398,7 @@
alias_opts[optname] = optargfn(optarg)
else:
if optarg != None:
- raise BzrError('option %r takes no argument' % optname)
+ raise BzrCommandError('option %r takes no argument' % optname)
opts[optname] = True
if proc_aliasarg:
alias_opts[optname] = True
@@ -440,7 +435,7 @@
raise BzrCommandError("command %r needs one or more %s"
% (cmd, argname.upper()))
argdict[argname + '_list'] = args[:-1]
- args[:-1] = []
+ args[:-1] = []
else:
# just a plain arg
argname = ap
@@ -637,7 +632,6 @@
import bzrlib.ui
from bzrlib.ui.text import TextUIFactory
## bzrlib.trace.enable_default_logging()
- bzrlib.trace.log_startup(argv)
bzrlib.ui.ui_factory = TextUIFactory()
ret = run_bzr_catch_errors(argv[1:])
mutter("return code %d", ret)
@@ -654,19 +648,12 @@
except Exception, e:
# used to handle AssertionError and KeyboardInterrupt
# specially here, but hopefully they're handled ok by the logger now
- import errno
- if (isinstance(e, IOError)
- and hasattr(e, 'errno')
- and e.errno == errno.EPIPE):
- bzrlib.trace.note('broken pipe')
- return 3
- else:
- bzrlib.trace.log_exception()
- if os.environ.get('BZR_PDB'):
- print '**** entering debugger'
- import pdb
- pdb.post_mortem(sys.exc_traceback)
- return 3
+ bzrlib.trace.report_exception(sys.exc_info(), sys.stderr)
+ if os.environ.get('BZR_PDB'):
+ print '**** entering debugger'
+ import pdb
+ pdb.post_mortem(sys.exc_traceback)
+ return 3
if __name__ == '__main__':
sys.exit(main(sys.argv))
=== modified file 'bzrlib/commit.py'
--- bzrlib/commit.py
+++ bzrlib/commit.py
@@ -81,7 +81,6 @@
import bzrlib.config
import bzrlib.errors as errors
from bzrlib.errors import (BzrError, PointlessCommit,
- HistoryMissing,
ConflictsInTree,
StrictCommitFailed
)
@@ -498,9 +497,8 @@
if not self.branch.repository.has_revision(parent_id):
if parent_id == self.branch.last_revision():
warning("parent is missing %r", parent_id)
- raise HistoryMissing(self.branch, 'revision', parent_id)
- else:
- mutter("commit will ghost revision %r", parent_id)
+ raise BzrCheckError("branch %s is missing revision {%s}"
+ % (self.branch, parent_id))
def _make_revision(self):
"""Record a new revision object for this commit."""
=== modified file 'bzrlib/errors.py'
--- bzrlib/errors.py
+++ bzrlib/errors.py
@@ -16,6 +16,18 @@
"""Exceptions for bzr, and reporting of them.
+There are 3 different classes of error:
+
+ * KeyboardInterrupt, and OSError with EPIPE - the program terminates
+ with an appropriate short message
+
+ * User errors, indicating a problem caused by the user such as a bad URL.
+ These are printed in a short form.
+
+ * Internal unexpected errors, including most Python builtin errors
+ and some raised from inside bzr. These are printed with a full
+ traceback and an invitation to report the bug.
+
Exceptions are caught at a high level to report errors to the user, and
might also be caught inside the program. Therefore it needs to be
possible to convert them to a meaningful string, and also for them to be
@@ -44,13 +56,14 @@
Therefore:
* create a new exception class for any class of error that can be
- usefully distinguished.
-
- * the printable form of an exception is generated by the base class
- __str__ method
-
-Exception strings should start with a capital letter and not have a final
-fullstop.
+ usefully distinguished. If no callers are likely to want to catch
+ one but not another, don't worry about them.
+
+ * the __str__ method should generate something useful; BzrError provides
+ a good default implementation
+
+Exception strings should start with a capital letter and should not have a
+final fullstop.
"""
from warnings import warn
@@ -81,6 +94,9 @@
class BzrError(StandardError):
+
+ is_user_error = True
+
def __str__(self):
# XXX: Should we show the exception class in
# exceptions that don't provide their own message?
@@ -119,6 +135,8 @@
class BzrCheckError(BzrNewError):
"""Internal check failed: %(message)s"""
+ is_user_error = False
+
def __init__(self, message):
BzrNewError.__init__(self)
self.message = message
@@ -126,6 +144,9 @@
class InvalidEntryName(BzrNewError):
"""Invalid entry name: %(name)s"""
+
+ is_user_error = False
+
def __init__(self, name):
BzrNewError.__init__(self)
self.name = name
@@ -163,7 +184,9 @@
self.url = url
-class BzrCommandError(BzrError):
+class BzrCommandError(BzrNewError):
+ """Error from user command"""
+ is_user_error = True
# Error from malformed user command
# This is being misused as a generic exception
# pleae subclass. RBC 20051030
@@ -172,15 +195,14 @@
# are not intended to be caught anyway. UI code need not subclass
# BzrCommandError, and non-UI code should not throw a subclass of
# BzrCommandError. ADHB 20051211
+ def __init__(self, msg):
+ self.msg = msg
+
def __str__(self):
- return self.args[0]
-
-
-class BzrOptionError(BzrCommandError):
- """Some missing or otherwise incorrect option was supplied."""
-
-
-class StrictCommitFailed(Exception):
+ return self.msg
+
+
+class StrictCommitFailed(BzrNewError):
"""Commit refused because there are unknowns in the tree."""
@@ -199,27 +221,30 @@
class NoSuchFile(PathError):
- """No such file: %(path)r%(extra)s"""
+ """No such file: %(path)s%(extra)s"""
class FileExists(PathError):
- """File exists: %(path)r%(extra)s"""
+ """File exists: %(path)s%(extra)s"""
class DirectoryNotEmpty(PathError):
- """Directory not empty: %(path)r%(extra)s"""
+ """Directory not empty: %(path)s%(extra)s"""
class ResourceBusy(PathError):
- """Device or resource busy: %(path)r%(extra)s"""
+ """Device or resource busy: %(path)s%(extra)s"""
class PermissionDenied(PathError):
- """Permission denied: %(path)r%(extra)s"""
+ """Permission denied: %(path)s%(extra)s"""
class PathNotChild(BzrNewError):
- """Path %(path)r is not a child of path %(base)r%(extra)s"""
+ """Path %(path)s is not a child of path %(base)s%(extra)s"""
+
+ is_user_error = False
+
def __init__(self, path, base, extra=None):
BzrNewError.__init__(self)
self.path = path
@@ -244,7 +269,7 @@
class NoRepositoryPresent(BzrNewError):
- """No repository present: %(path)r"""
+ """No repository present: %(path)s"""
def __init__(self, bzrdir):
BzrNewError.__init__(self)
self.path = bzrdir.transport.clone('..').base
@@ -260,16 +285,12 @@
self.path = path
-class UnsupportedFormatError(BzrError):
- """Specified path is a bzr branch that we recognize but cannot read."""
- def __str__(self):
- return 'unsupported branch format: %s' % self.args[0]
-
-
-class UnknownFormatError(BzrError):
- """Specified path is a bzr branch whose format we do not recognize."""
- def __str__(self):
- return 'unknown branch format: %s' % self.args[0]
+class UnsupportedFormatError(BzrNewError):
+ """Unsupported branch format: %(format)s"""
+
+
+class UnknownFormatError(BzrNewError):
+ """Unknown branch format: %(format)s"""
class IncompatibleFormat(BzrNewError):
@@ -357,7 +378,8 @@
class ObjectNotLocked(LockError):
- """%(obj)r is not locked"""
+ """%(obj)s is not locked"""
+ is_user_error = False
# this can indicate that any particular object is not locked; see also
# LockNotHeld which means that a particular *lock* object is not held by
# the caller -- perhaps they should be unified.
@@ -366,7 +388,7 @@
class ReadOnlyObjectDirtiedError(ReadOnlyError):
- """Cannot change object %(obj)r in read only transaction"""
+ """Cannot change object %(obj)s in read only transaction"""
def __init__(self, obj):
self.obj = obj
@@ -391,7 +413,7 @@
class LockBreakMismatch(LockError):
- """Lock was released and re-acquired before being broken: %(lock)s: held by %(holder)r, wanted to break %(target)r"""
+ """Lock was released and re-acquired before being broken: %(lock)s: held by %(holder)s, wanted to break %(target)r"""
def __init__(self, lock, holder, target):
self.lock = lock
self.holder = holder
@@ -425,42 +447,38 @@
"""Commit refused because there are unknowns in the tree."""
-class NoSuchRevision(BzrError):
+class NoSuchRevision(BzrNewError):
+ """Branch %(branch)s has no revision %(revision)s"""
+
+ is_user_error = False
+
def __init__(self, branch, revision):
self.branch = branch
self.revision = revision
- msg = "Branch %s has no revision %s" % (branch, revision)
- BzrError.__init__(self, msg)
-
-
-class HistoryMissing(BzrError):
- def __init__(self, branch, object_type, object_id):
- self.branch = branch
- BzrError.__init__(self,
- '%s is missing %s {%s}'
- % (branch, object_type, object_id))
-
-
-class DivergedBranches(BzrError):
+
+
+class DivergedBranches(BzrNewError):
+ "These branches have diverged. Use the merge command to reconcile them."""
+
+ is_user_error = True
def __init__(self, branch1, branch2):
- BzrError.__init__(self, "These branches have diverged. Try merge.")
self.branch1 = branch1
self.branch2 = branch2
-class UnrelatedBranches(BzrCommandError):
- def __init__(self):
- msg = "Branches have no common ancestor, and no base revision"\
- " specified."
- BzrCommandError.__init__(self, msg)
-
-
-class NoCommonAncestor(BzrError):
+class UnrelatedBranches(BzrNewError):
+ "Branches have no common ancestor, and no merge base revision was specified."
+
+ is_user_error = True
+
+
+class NoCommonAncestor(BzrNewError):
+ "Revisions have no common ancestor: %(revision_a)s %(revision_b)s"
+
def __init__(self, revision_a, revision_b):
- msg = "Revisions have no common ancestor: %s %s." \
- % (revision_a, revision_b)
- BzrError.__init__(self, msg)
+ self.revision_a = revision_a
+ self.revision_b = revision_b
class NoCommonRoot(BzrError):
=== modified file 'bzrlib/plugin.py'
--- bzrlib/plugin.py
+++ bzrlib/plugin.py
@@ -46,7 +46,7 @@
import bzrlib
from bzrlib.config import config_dir
-from bzrlib.trace import log_error, mutter, log_exception, warning, \
+from bzrlib.trace import log_error, mutter, warning, \
log_exception_quietly
from bzrlib.errors import BzrError
from bzrlib import plugins
=== modified file 'bzrlib/repository.py'
--- bzrlib/repository.py
+++ bzrlib/repository.py
@@ -867,7 +867,7 @@
except errors.NoSuchFile:
raise errors.NoRepositoryPresent(a_bzrdir)
except KeyError:
- raise errors.UnknownFormatError(format_string)
+ raise errors.UnknownFormatError(format=format_string)
def _get_control_store(self, repo_transport, control_files):
"""Return the control store for this repository."""
=== modified file 'bzrlib/tests/__init__.py'
--- bzrlib/tests/__init__.py
+++ bzrlib/tests/__init__.py
@@ -630,7 +630,6 @@
self.log('run bzr: %s', ' '.join(argv))
# FIXME: don't call into logging here
handler = logging.StreamHandler(stderr)
- handler.setFormatter(bzrlib.trace.QuietFormatter())
handler.setLevel(logging.INFO)
logger = logging.getLogger('')
logger.addHandler(handler)
=== modified file 'bzrlib/tests/blackbox/__init__.py'
--- bzrlib/tests/blackbox/__init__.py
+++ bzrlib/tests/blackbox/__init__.py
@@ -48,6 +48,7 @@
'bzrlib.tests.blackbox.test_commit',
'bzrlib.tests.blackbox.test_conflicts',
'bzrlib.tests.blackbox.test_diff',
+ 'bzrlib.tests.blackbox.test_exceptions',
'bzrlib.tests.blackbox.test_export',
'bzrlib.tests.blackbox.test_find_merge_base',
'bzrlib.tests.blackbox.test_help',
=== modified file 'bzrlib/tests/blackbox/test_aliases.py'
--- bzrlib/tests/blackbox/test_aliases.py
+++ bzrlib/tests/blackbox/test_aliases.py
@@ -57,9 +57,9 @@
# If --no-aliases breaks all of bzr, we also get retcode=3
# So we need to catch the output as well
- self.assertEquals(bzr_catch_error('--no-aliases', 'c', 'a',
- retcode=None),
- "bzr: ERROR: unknown command 'c'\n")
+ self.assertEquals(bzr_catch_error('--no-aliases', 'c', 'a',
+ retcode=None),
+ 'bzr: ERROR: unknown command "c"\n')
bzr('c', '-r1', '-r2', retcode=3)
bzr('c1', '-r1', '-r2', retcode=3)
=== modified file 'bzrlib/tests/blackbox/test_init.py'
--- bzrlib/tests/blackbox/test_init.py
+++ bzrlib/tests/blackbox/test_init.py
@@ -70,9 +70,9 @@
out, err = self.run_bzr('init', 'subdir2/nothere', retcode=3)
self.assertEqual('', out)
- self.failUnless(err.startswith(
- 'bzr: ERROR: exceptions.OSError: '
- '[Errno 2] No such file or directory: '))
+ self.assertContainsRe(err,
+ r'^bzr: ERROR: .*'
+ '\[Errno 2\] No such file or directory: ')
os.mkdir('subdir2')
out, err = self.run_bzr('init', 'subdir2')
=== modified file 'bzrlib/tests/blackbox/test_pull.py'
--- bzrlib/tests/blackbox/test_pull.py
+++ bzrlib/tests/blackbox/test_pull.py
@@ -248,7 +248,7 @@
tree_b.commit('commit d')
out = self.runbzr('pull ../branch_a', retcode=3)
self.assertEquals(out,
- ('','bzr: ERROR: These branches have diverged. Try merge.\n'))
+ ('','bzr: ERROR: These branches have diverged. Use the merge command to reconcile them.\n'))
self.assertEquals(abspath(branch_b.get_parent()), abspath(parent))
# test implicit --remember after resolving previous failure
uncommit(branch=branch_b, tree=tree_b)
=== modified file 'bzrlib/tests/test_errors.py'
--- bzrlib/tests/test_errors.py
+++ bzrlib/tests/test_errors.py
@@ -27,8 +27,8 @@
def test_no_repo(self):
dir = bzrdir.BzrDir.create(self.get_url())
error = errors.NoRepositoryPresent(dir)
- self.assertNotEqual(-1, str(error).find(repr(dir.transport.clone('..').base)))
- self.assertEqual(-1, str(error).find(repr(dir.transport.base)))
+ self.assertNotEqual(-1, str(error).find((dir.transport.clone('..').base)))
+ self.assertEqual(-1, str(error).find((dir.transport.base)))
def test_up_to_date(self):
error = errors.UpToDateFormat(bzrdir.BzrDirFormat4())
=== modified file 'bzrlib/tests/test_options.py'
--- bzrlib/tests/test_options.py
+++ bzrlib/tests/test_options.py
@@ -46,7 +46,7 @@
def test_unknown_short_opt(self):
out, err = self.run_bzr_captured(['help', '-r'], retcode=3)
- self.assertContainsRe(err, r'unknown short option')
+ self.assertContainsRe(err, r'unknown option')
# >>> parse_args('log -r 500'.split())
=== modified file 'bzrlib/tests/test_source.py'
--- bzrlib/tests/test_source.py
+++ bzrlib/tests/test_source.py
@@ -72,4 +72,4 @@
# written. Note that this is an exact equality so that when the number
# drops, it is not given a buffer but rather this test updated
# immediately.
- self.assertEqual(4, occurences)
+ self.assertEqual(3, occurences)
=== modified file 'bzrlib/tests/test_trace.py'
--- bzrlib/tests/test_trace.py
+++ bzrlib/tests/test_trace.py
@@ -1,5 +1,6 @@
-# Copyright (C) 2005 by Canonical Ltd
+# Copyright (C) 2005, 2006 by Canonical Ltd
# Authors: Robert Collins <robert.collins at canonical.com>
+# Martin Pool
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -19,50 +20,76 @@
"""Tests for trace library"""
+import errno
import os
import sys
+from StringIO import StringIO
from bzrlib.tests import TestCaseInTempDir, TestCase
-from bzrlib.trace import format_exception_short, mutter
-from bzrlib.errors import NotBranchError, BzrError, BzrNewError
+from bzrlib.trace import mutter, report_exception
+from bzrlib.errors import NotBranchError
+
+
+def _format_exception():
+ """Format an exception as it would normally be displayed to the user"""
+ buf = StringIO()
+ report_exception(sys.exc_info(), buf)
+ return buf.getvalue()
+
class TestTrace(TestCase):
+
def test_format_sys_exception(self):
- """Short formatting of exceptions"""
try:
raise NotImplementedError, "time travel"
except NotImplementedError:
pass
- error_lines = format_exception_short(sys.exc_info()).splitlines()
- self.assertEqualDiff(error_lines[0],
- 'exceptions.NotImplementedError: time travel')
- self.assertContainsRe(error_lines[1],
- r'^ at .*trace\.py line \d+$')
- self.assertContainsRe(error_lines[2],
- r'^ in test_format_sys_exception$')
+ err = _format_exception()
+ self.assertEqualDiff(err.splitlines()[0],
+ 'bzr: ERROR: exceptions.NotImplementedError: time travel')
+ self.assertContainsRe(err,
+ r'File.*test_trace.py')
+
+ def test_format_interrupt_exception(self):
+ try:
+ raise KeyboardInterrupt()
+ except KeyboardInterrupt:
+ # XXX: Some risk that a *real* keyboard interrupt won't be seen
+ pass
+ msg = _format_exception()
+ self.assertTrue(len(msg) > 0)
+ self.assertEqualDiff(msg, 'bzr: interrupted\n')
+
+ def test_format_os_error(self):
+ try:
+ file('nosuchfile22222')
+ except (OSError, IOError):
+ pass
+ msg = _format_exception()
+ self.assertContainsRe(msg, r'^bzr: ERROR: \[Errno .*\] No such file.*nosuchfile')
+
def test_format_exception(self):
- """Short formatting of exceptions"""
+ """Short formatting of bzr exceptions"""
try:
raise NotBranchError, 'wibble'
except NotBranchError:
pass
- msg = format_exception_short(sys.exc_info())
- self.assertEqualDiff(msg, 'Not a branch: wibble')
-
- def test_format_old_exception(self):
- # format a class that doesn't descend from BzrNewError;
- # remove this test when everything is unified there
- self.assertFalse(issubclass(BzrError, BzrNewError))
- try:
- raise BzrError('some old error')
- except BzrError:
- pass
- msg = format_exception_short(sys.exc_info())
- self.assertEqualDiff(msg, 'some old error')
+ msg = _format_exception()
+ self.assertTrue(len(msg) > 0)
+ self.assertEqualDiff(msg, 'bzr: ERROR: Not a branch: wibble\n')
def test_trace_unicode(self):
"""Write Unicode to trace log"""
self.log(u'the unicode character for benzene is \N{BENZENE RING}')
self.assertContainsRe('the unicode character',
self._get_log())
+
+ def test_report_broken_pipe(self):
+ try:
+ raise IOError(errno.EPIPE, 'broken pipe foofofo')
+ except IOError, e:
+ msg = _format_exception()
+ self.assertEquals(msg, "bzr: broken pipe\n")
+ else:
+ self.fail("expected error not raised")
=== modified file 'bzrlib/trace.py'
--- bzrlib/trace.py
+++ bzrlib/trace.py
@@ -1,4 +1,18 @@
-# Copyright (C) 2005, Canonical Ltd
+# Copyright (C) 2005, 2006 by Canonical Ltd
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
"""Messages and logging for bazaar-ng.
@@ -32,16 +46,14 @@
form.
"""
-# TODO: in debug mode, stderr should get full tracebacks and also
-# debug messages. (Is this really needed?)
-
# FIXME: Unfortunately it turns out that python's logging module
# is quite expensive, even when the message is not printed by any handlers.
# We should perhaps change back to just simply doing it here.
+import errno
+import os
import sys
-import os
import logging
import bzrlib
@@ -56,25 +68,6 @@
_bzr_log_file = None
-class QuietFormatter(logging.Formatter):
- """Formatter that supresses the details of errors.
-
- This is used by default on stderr so as not to scare the user.
- """
- # At first I tried overriding formatException to suppress the
- # exception details, but that has global effect: no loggers
- # can get the exception details is we suppress them here.
-
- def format(self, record):
- if record.levelno >= logging.WARNING:
- s = 'bzr: ' + record.levelname + ': '
- else:
- s = ''
- s += record.getMessage()
- if record.exc_info:
- s += '\n' + format_exception_short(record.exc_info)
- return s
-
# configure convenient aliases for output routines
_bzr_logger = logging.getLogger('bzr')
@@ -148,29 +141,6 @@
warning("failed to open trace file: %s" % (e))
-def log_startup(argv):
- debug('\n\nbzr %s invoked on python %s (%s)',
- bzrlib.__version__,
- '.'.join(map(str, sys.version_info)),
- sys.platform)
- debug(' arguments: %r', argv)
- debug(' working dir: %r', os.getcwdu())
-
-
-def log_exception(msg=None):
- """Log the last exception to stderr and the trace file.
-
- The exception string representation is used as the error
- summary, unless msg is given.
- """
- if msg:
- error(msg)
- else:
- exc_str = format_exception_short(sys.exc_info())
- error(exc_str)
- log_exception_quietly()
-
-
def log_exception_quietly():
"""Log the last exception to the trace file only.
@@ -187,7 +157,6 @@
# FIXME: if this is run twice, things get confused
global _stderr_handler, _file_handler, _trace_file, _bzr_log_file
_stderr_handler = logging.StreamHandler()
- _stderr_handler.setFormatter(QuietFormatter())
logging.getLogger('').addHandler(_stderr_handler)
_stderr_handler.setLevel(logging.INFO)
if not _file_handler:
@@ -195,8 +164,7 @@
_trace_file = _bzr_log_file
if _file_handler:
_file_handler.setLevel(logging.DEBUG)
- _bzr_logger.setLevel(logging.DEBUG)
-
+ _bzr_logger.setLevel(logging.DEBUG)
def be_quiet(quiet=True):
@@ -259,32 +227,40 @@
enable_default_logging()
-def format_exception_short(exc_info):
- """Make a short string form of an exception.
-
- This is used for display to stderr. It specially handles exception
- classes without useful string methods.
-
- The result has no trailing newline.
-
- exc_info - typically an exception from sys.exc_info()
- """
- exc_type, exc_object, exc_tb = exc_info
- try:
- if exc_type is None:
- return '(no exception)'
- if isinstance(exc_object, (BzrError, BzrNewError)):
- return str(exc_object)
- else:
- import traceback
- tb = traceback.extract_tb(exc_tb)
- msg = '%s: %s' % (exc_type, exc_object)
- if msg[-1] == '\n':
- msg = msg[:-1]
- if tb:
- msg += '\n at %s line %d\n in %s' % (tb[-1][:3])
- return msg
- except Exception, formatting_exc:
- # XXX: is this really better than just letting it run up?
- return '(error formatting exception of type %s: %s)' \
- % (exc_type, formatting_exc)
+def report_exception(exc_info, err_file):
+ exc_type, exc_object, exc_tb = exc_info
+ if (isinstance(exc_object, IOError)
+ and getattr(exc_object, 'errno', None) == errno.EPIPE):
+ print >>err_file, "bzr: broken pipe"
+ elif isinstance(exc_object, KeyboardInterrupt):
+ print >>err_file, "bzr: interrupted"
+ elif getattr(exc_object, 'is_user_error', False):
+ report_user_error(exc_info, err_file)
+ elif isinstance(exc_object, (OSError, IOError)):
+ # Might be nice to catch all of these and show them as something more
+ # specific, but there are too many cases at the moment.
+ report_user_error(exc_info, err_file)
+ else:
+ report_bug(exc_info, err_file)
+
+
+# TODO: Should these be specially encoding the output?
+def report_user_error(exc_info, err_file):
+ print >>err_file, "bzr: ERROR:", str(exc_info[1])
+
+
+def report_bug(exc_info, err_file):
+ """Report an exception that probably indicates a bug in bzr"""
+ import traceback
+ exc_type, exc_object, exc_tb = exc_info
+ print >>err_file, "bzr: ERROR: %s: %s" % (exc_type, exc_object)
+ print >>err_file
+ traceback.print_exception(exc_type, exc_object, exc_tb, file=err_file)
+ print >>err_file
+ print >>err_file, 'bzr %s on python %s (%s)' % \
+ (bzrlib.__version__,
+ '.'.join(map(str, sys.version_info)),
+ sys.platform)
+ print >>err_file, 'arguments: %r' % sys.argv
+ print >>err_file
+ print >>err_file, "** please send this report to bazaar-ng at lists.ubuntu.com"
=== modified file 'bzrlib/workingtree.py'
--- bzrlib/workingtree.py
+++ bzrlib/workingtree.py
@@ -60,7 +60,6 @@
from bzrlib.errors import (BzrCheckError,
BzrError,
ConflictFormatError,
- DivergedBranches,
WeaveRevisionNotPresent,
NotBranchError,
NoSuchFile,
@@ -1554,7 +1553,7 @@
except NoSuchFile:
raise errors.NoWorkingTree(base=transport.base)
except KeyError:
- raise errors.UnknownFormatError(format_string)
+ raise errors.UnknownFormatError(format=format_string)
@classmethod
def get_default_format(klass):
More information about the bazaar
mailing list