Rev 5694: (spiv) Give clearer errors when remote operation fails with MemoryError or in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Thu Mar 3 04:33:27 UTC 2011
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 5694 [merge]
revision-id: pqm at pqm.ubuntu.com-20110303043323-76p182iwtfnaqbkt
parent: pqm at pqm.ubuntu.com-20110303035329-o7jglt1fy4cwtw1f
parent: gzlist at googlemail.com-20110302214251-wzt3jdyxmewgzvi0
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Thu 2011-03-03 04:33:23 +0000
message:
(spiv) Give clearer errors when remote operation fails with MemoryError or
other unexpected exception (Martin [gz])
modified:
bzrlib/remote.py remote.py-20060720103555-yeeg2x51vn0rbtdp-1
bzrlib/smart/message.py message.py-20080222013625-ncqmh3nrxjkxab87-1
bzrlib/smart/request.py request.py-20061108095550-gunadhxmzkdjfeek-1
bzrlib/tests/test_remote.py test_remote.py-20060720103555-yeeg2x51vn0rbtdp-2
bzrlib/tests/test_smart_request.py test_smart_request.p-20090211070731-o38wayv3asm25d6a-1
bzrlib/tests/test_smart_transport.py test_ssh_transport.py-20060608202016-c25gvf1ob7ypbus6-2
bzrlib/trace.py trace.py-20050309040759-c8ed824bdcd4748a
doc/en/release-notes/bzr-2.4.txt bzr2.4.txt-20110114053217-k7ym9jfz243fddjm-1
=== modified file 'bzrlib/remote.py'
--- a/bzrlib/remote.py 2011-02-21 23:38:11 +0000
+++ b/bzrlib/remote.py 2011-03-03 04:33:23 +0000
@@ -2973,10 +2973,7 @@
'Missing key %r in context %r', key_err.args[0], context)
raise err
- if err.error_verb == 'IncompatibleRepositories':
- raise errors.IncompatibleRepositories(err.error_args[0],
- err.error_args[1], err.error_args[2])
- elif err.error_verb == 'NoSuchRevision':
+ if err.error_verb == 'NoSuchRevision':
raise NoSuchRevision(find('branch'), err.error_args[0])
elif err.error_verb == 'nosuchrevision':
raise NoSuchRevision(find('repository'), err.error_args[0])
@@ -2989,22 +2986,12 @@
detail=extra)
elif err.error_verb == 'norepository':
raise errors.NoRepositoryPresent(find('bzrdir'))
- elif err.error_verb == 'LockContention':
- raise errors.LockContention('(remote lock)')
elif err.error_verb == 'UnlockableTransport':
raise errors.UnlockableTransport(find('bzrdir').root_transport)
- elif err.error_verb == 'LockFailed':
- raise errors.LockFailed(err.error_args[0], err.error_args[1])
elif err.error_verb == 'TokenMismatch':
raise errors.TokenMismatch(find('token'), '(remote token)')
elif err.error_verb == 'Diverged':
raise errors.DivergedBranches(find('branch'), find('other_branch'))
- elif err.error_verb == 'TipChangeRejected':
- raise errors.TipChangeRejected(err.error_args[0].decode('utf8'))
- elif err.error_verb == 'UnstackableBranchFormat':
- raise errors.UnstackableBranchFormat(*err.error_args)
- elif err.error_verb == 'UnstackableRepositoryFormat':
- raise errors.UnstackableRepositoryFormat(*err.error_args)
elif err.error_verb == 'NotStacked':
raise errors.NotStacked(branch=find('branch'))
elif err.error_verb == 'PermissionDenied':
@@ -3020,6 +3007,24 @@
elif err.error_verb == 'NoSuchFile':
path = get_path()
raise errors.NoSuchFile(path)
+ _translate_error_without_context(err)
+
+
+def _translate_error_without_context(err):
+ """Translate any ErrorFromSmartServer values that don't require context"""
+ if err.error_verb == 'IncompatibleRepositories':
+ raise errors.IncompatibleRepositories(err.error_args[0],
+ err.error_args[1], err.error_args[2])
+ elif err.error_verb == 'LockContention':
+ raise errors.LockContention('(remote lock)')
+ elif err.error_verb == 'LockFailed':
+ raise errors.LockFailed(err.error_args[0], err.error_args[1])
+ elif err.error_verb == 'TipChangeRejected':
+ raise errors.TipChangeRejected(err.error_args[0].decode('utf8'))
+ elif err.error_verb == 'UnstackableBranchFormat':
+ raise errors.UnstackableBranchFormat(*err.error_args)
+ elif err.error_verb == 'UnstackableRepositoryFormat':
+ raise errors.UnstackableRepositoryFormat(*err.error_args)
elif err.error_verb == 'FileExists':
raise errors.FileExists(err.error_args[0])
elif err.error_verb == 'DirectoryNotEmpty':
@@ -3044,4 +3049,7 @@
raise UnicodeEncodeError(encoding, val, start, end, reason)
elif err.error_verb == 'ReadOnlyError':
raise errors.TransportNotPossible('readonly transport')
+ elif err.error_verb == 'MemoryError':
+ raise errors.BzrError("remote server out of memory\n"
+ "Retry non-remotely, or contact the server admin for details.")
raise errors.UnknownErrorFromSmartServer(err)
=== modified file 'bzrlib/smart/message.py'
--- a/bzrlib/smart/message.py 2009-09-11 06:07:05 +0000
+++ b/bzrlib/smart/message.py 2011-03-02 20:16:03 +0000
@@ -303,7 +303,7 @@
mutter(' result: %r', self.args)
if self.status == 'E':
self._wait_for_response_end()
- _translate_error(self.args)
+ _raise_smart_server_error(self.args)
return tuple(self.args)
def read_body_bytes(self, count=-1):
@@ -335,27 +335,17 @@
yield bytes_part
self._read_more()
if self._body_stream_status == 'E':
- _translate_error(self._body_error_args)
+ _raise_smart_server_error(self._body_error_args)
def cancel_read_body(self):
self._wait_for_response_end()
-def _translate_error(error_tuple):
- # Many exceptions need some state from the requestor to be properly
- # translated (e.g. they need a branch object). So this only translates a
- # few errors, and the rest are turned into a generic ErrorFromSmartServer.
- error_name = error_tuple[0]
- error_args = error_tuple[1:]
- if error_name == 'UnknownMethod':
- raise errors.UnknownSmartMethod(error_args[0])
- if error_name == 'LockContention':
- raise errors.LockContention('(remote lock)')
- elif error_name == 'LockFailed':
- raise errors.LockFailed(*error_args[:2])
- elif error_name == 'FileExists':
- raise errors.FileExists(error_args[0])
- elif error_name == 'NoSuchFile':
- raise errors.NoSuchFile(error_args[0])
- else:
- raise errors.ErrorFromSmartServer(error_tuple)
+def _raise_smart_server_error(error_tuple):
+ """Raise exception based on tuple received from smart server
+
+ Specific error translation is handled by bzrlib.remote._translate_error
+ """
+ if error_tuple[0] == 'UnknownMethod':
+ raise errors.UnknownSmartMethod(error_tuple[1])
+ raise errors.ErrorFromSmartServer(error_tuple)
=== modified file 'bzrlib/smart/request.py'
--- a/bzrlib/smart/request.py 2010-11-26 06:31:54 +0000
+++ b/bzrlib/smart/request.py 2011-03-02 20:39:58 +0000
@@ -446,9 +446,13 @@
return ('TokenMismatch', err.given_token, err.lock_token)
elif isinstance(err, errors.LockContention):
return ('LockContention',)
+ elif isinstance(err, MemoryError):
+ # GZ 2011-02-24: Copy bzrlib.trace -Dmem_dump functionality here?
+ return ('MemoryError',)
# Unserialisable error. Log it, and return a generic error
trace.log_exception_quietly()
- return ('error', str(err))
+ return ('error', trace._qualified_exception_name(err.__class__, True),
+ str(err))
class HelloRequest(SmartServerRequest):
=== modified file 'bzrlib/tests/test_remote.py'
--- a/bzrlib/tests/test_remote.py 2011-02-24 16:13:39 +0000
+++ b/bzrlib/tests/test_remote.py 2011-03-03 04:33:23 +0000
@@ -2775,6 +2775,16 @@
('pack collection autopack',)],
client._calls)
+ def test_oom_error_reporting(self):
+ """An out-of-memory condition on the server is reported clearly"""
+ transport_path = 'quack'
+ repo, client = self.setup_fake_client_and_repository(transport_path)
+ client.add_expected_call(
+ 'PackRepository.autopack', ('quack/',),
+ 'error', ('MemoryError',))
+ err = self.assertRaises(errors.BzrError, repo.autopack)
+ self.assertContainsRe(str(err), "^remote server out of mem")
+
class TestErrorTranslationBase(tests.TestCaseWithMemoryTransport):
"""Base class for unit tests for bzrlib.remote._translate_error."""
@@ -2853,6 +2863,13 @@
detail='extra detail')
self.assertEqual(expected_error, translated_error)
+ def test_norepository(self):
+ bzrdir = self.make_bzrdir('')
+ translated_error = self.translateTuple(('norepository',),
+ bzrdir=bzrdir)
+ expected_error = errors.NoRepositoryPresent(bzrdir)
+ self.assertEqual(expected_error, translated_error)
+
def test_LockContention(self):
translated_error = self.translateTuple(('LockContention',))
expected_error = errors.LockContention('(remote lock)')
@@ -2886,6 +2903,12 @@
expected_error = errors.DivergedBranches(branch, other_branch)
self.assertEqual(expected_error, translated_error)
+ def test_NotStacked(self):
+ branch = self.make_branch('')
+ translated_error = self.translateTuple(('NotStacked',), branch=branch)
+ expected_error = errors.NotStacked(branch)
+ self.assertEqual(expected_error, translated_error)
+
def test_ReadError_no_args(self):
path = 'a path'
translated_error = self.translateTuple(('ReadError',), path=path)
@@ -2907,7 +2930,8 @@
def test_PermissionDenied_no_args(self):
path = 'a path'
- translated_error = self.translateTuple(('PermissionDenied',), path=path)
+ translated_error = self.translateTuple(('PermissionDenied',),
+ path=path)
expected_error = errors.PermissionDenied(path)
self.assertEqual(expected_error, translated_error)
@@ -2936,6 +2960,45 @@
expected_error = errors.PermissionDenied(path, extra)
self.assertEqual(expected_error, translated_error)
+ # GZ 2011-03-02: TODO test for PermissionDenied with non-ascii 'extra'
+
+ def test_NoSuchFile_context_path(self):
+ local_path = "local path"
+ translated_error = self.translateTuple(('ReadError', "remote path"),
+ path=local_path)
+ expected_error = errors.ReadError(local_path)
+ self.assertEqual(expected_error, translated_error)
+
+ def test_NoSuchFile_without_context(self):
+ remote_path = "remote path"
+ translated_error = self.translateTuple(('ReadError', remote_path))
+ expected_error = errors.ReadError(remote_path)
+ self.assertEqual(expected_error, translated_error)
+
+ def test_ReadOnlyError(self):
+ translated_error = self.translateTuple(('ReadOnlyError',))
+ expected_error = errors.TransportNotPossible("readonly transport")
+ self.assertEqual(expected_error, translated_error)
+
+ def test_MemoryError(self):
+ translated_error = self.translateTuple(('MemoryError',))
+ self.assertStartsWith(str(translated_error),
+ "remote server out of memory")
+
+ def test_generic_IndexError_no_classname(self):
+ err = errors.ErrorFromSmartServer(('error', "list index out of range"))
+ translated_error = self.translateErrorFromSmartServer(err)
+ expected_error = errors.UnknownErrorFromSmartServer(err)
+ self.assertEqual(expected_error, translated_error)
+
+ # GZ 2011-03-02: TODO test generic non-ascii error string
+
+ def test_generic_KeyError(self):
+ err = errors.ErrorFromSmartServer(('error', 'KeyError', "1"))
+ translated_error = self.translateErrorFromSmartServer(err)
+ expected_error = errors.UnknownErrorFromSmartServer(err)
+ self.assertEqual(expected_error, translated_error)
+
class TestErrorTranslationRobustness(TestErrorTranslationBase):
"""Unit tests for bzrlib.remote._translate_error's robustness.
=== modified file 'bzrlib/tests/test_smart_request.py'
--- a/bzrlib/tests/test_smart_request.py 2010-07-13 19:02:12 +0000
+++ b/bzrlib/tests/test_smart_request.py 2011-03-02 21:04:00 +0000
@@ -41,6 +41,13 @@
raise errors.NoSuchFile('xyzzy')
+class DoUnexpectedErrorRequest(request.SmartServerRequest):
+ """A request that encounters a generic error in self.do()"""
+
+ def do(self):
+ dict()[1]
+
+
class ChunkErrorRequest(request.SmartServerRequest):
"""A request that raises an error from self.do_chunk()."""
@@ -149,6 +156,14 @@
handler.end_received()
self.assertResponseIsTranslatedError(handler)
+ def test_unexpected_error_translation(self):
+ handler = request.SmartServerRequestHandler(
+ None, {'foo': DoUnexpectedErrorRequest}, '/')
+ handler.args_received(('foo',))
+ self.assertEqual(
+ request.FailedSmartServerResponse(('error', 'KeyError', "1")),
+ handler.response)
+
class TestRequestHanderErrorTranslation(TestCase):
"""Tests for bzrlib.smart.request._translate_error."""
@@ -172,6 +187,23 @@
('TokenMismatch', 'some-token', 'actual-token'),
errors.TokenMismatch('some-token', 'actual-token'))
+ def test_MemoryError(self):
+ self.assertTranslationEqual(("MemoryError",), MemoryError())
+
+ def test_generic_Exception(self):
+ self.assertTranslationEqual(('error', 'Exception', ""),
+ Exception())
+
+ def test_generic_BzrError(self):
+ self.assertTranslationEqual(('error', 'BzrError', "some text"),
+ errors.BzrError(msg="some text"))
+
+ def test_generic_zlib_error(self):
+ from zlib import error
+ msg = "Error -3 while decompressing data: incorrect data check"
+ self.assertTranslationEqual(('error', 'zlib.error', msg),
+ error(msg))
+
class TestRequestJail(TestCaseWithMemoryTransport):
=== modified file 'bzrlib/tests/test_smart_transport.py'
--- a/bzrlib/tests/test_smart_transport.py 2011-01-10 22:20:12 +0000
+++ b/bzrlib/tests/test_smart_transport.py 2011-03-02 20:39:58 +0000
@@ -2412,7 +2412,7 @@
self.assertEqual('aaa', stream.next())
self.assertEqual('bbb', stream.next())
exc = self.assertRaises(errors.ErrorFromSmartServer, stream.next)
- self.assertEqual(('error', 'Boom!'), exc.error_tuple)
+ self.assertEqual(('error', 'Exception', 'Boom!'), exc.error_tuple)
def test_interrupted_by_connection_lost(self):
interrupted_body_stream = (
@@ -2815,7 +2815,8 @@
'b\x00\x00\x00\x03aaa' # body part ('aaa')
'b\x00\x00\x00\x03bbb' # body part ('bbb')
'oE' # status flag (error)
- 's\x00\x00\x00\x10l5:error5:Boom!e' # err struct ('error', 'Boom!')
+ # err struct ('error', 'Exception', 'Boom!')
+ 's\x00\x00\x00\x1bl5:error9:Exception5:Boom!e'
'e' # EOM
)
=== modified file 'bzrlib/trace.py'
--- a/bzrlib/trace.py 2011-01-13 00:14:31 +0000
+++ b/bzrlib/trace.py 2011-03-02 21:42:37 +0000
@@ -488,6 +488,21 @@
elif fd is not None:
os.close(fd)
+
+def _qualified_exception_name(eclass, unqualified_bzrlib_errors=False):
+ """Give name of error class including module for non-builtin exceptions
+
+ If `unqualified_bzrlib_errors` is True, errors specific to bzrlib will
+ also omit the module prefix.
+ """
+ class_name = eclass.__name__
+ module_name = eclass.__module__
+ if module_name in ("exceptions", "__main__") or (
+ unqualified_bzrlib_errors and module_name == "bzrlib.errors"):
+ return class_name
+ return "%s.%s" % (module_name, class_name)
+
+
def report_exception(exc_info, err_file):
"""Report an exception to err_file (typically stderr) and to .bzr.log.
=== modified file 'doc/en/release-notes/bzr-2.4.txt'
--- a/doc/en/release-notes/bzr-2.4.txt 2011-03-03 02:28:57 +0000
+++ b/doc/en/release-notes/bzr-2.4.txt 2011-03-03 04:33:23 +0000
@@ -55,6 +55,10 @@
.. Fixes for situations where bzr would previously crash or give incorrect
or undesirable results.
+* A MemoryError thrown on the server during a remote operation will now be
+ usefully reported, and other unexpected errors will include the class name.
+ (Martin [gz], #722416)
+
* ``bzr annotate -r-1 file`` will now properly annotate a deleted file.
(Andrew King, #537442)
More information about the bazaar-commits
mailing list