Rev 4066: Improve error-handling while sending body_streams in HPSS requests in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Mon Mar 2 06:21:05 GMT 2009
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 4066
revision-id: pqm at pqm.ubuntu.com-20090302062102-9hcytxohu3q8qxan
parent: pqm at pqm.ubuntu.com-20090301222412-o2s34646lnn958f3
parent: andrew.bennetts at canonical.com-20090302053855-sqch1772xzbctghf
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Mon 2009-03-02 06:21:02 +0000
message:
Improve error-handling while sending body_streams in HPSS requests
and responses. (Andrew Bennetts)
modified:
bzrlib/smart/protocol.py protocol.py-20061108035435-ot0lstk2590yqhzr-1
bzrlib/smart/request.py request.py-20061108095550-gunadhxmzkdjfeek-1
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
------------------------------------------------------------
revno: 4064.1.4
revision-id: andrew.bennetts at canonical.com-20090302053855-sqch1772xzbctghf
parent: andrew.bennetts at canonical.com-20090302042253-3q6wedwz1lxb7a1c
committer: Andrew Bennetts <andrew.bennetts at canonical.com>
branch nick: streaming-body-error
timestamp: Mon 2009-03-02 16:38:55 +1100
message:
Remove overly-specific test (we don't care about exactly how that layer handles bad requests).
modified:
bzrlib/tests/test_smart_request.py test_smart_request.p-20090211070731-o38wayv3asm25d6a-1
------------------------------------------------------------
revno: 4064.1.3
revision-id: andrew.bennetts at canonical.com-20090302042253-3q6wedwz1lxb7a1c
parent: andrew.bennetts at canonical.com-20090302041511-uq1s5tns2qnh803c
committer: Andrew Bennetts <andrew.bennetts at canonical.com>
branch nick: streaming-body-error
timestamp: Mon 2009-03-02 15:22:53 +1100
message:
Fix bug in _iter_with_errors; it should not report StopIteration as an exception.
modified:
bzrlib/smart/protocol.py protocol.py-20061108035435-ot0lstk2590yqhzr-1
------------------------------------------------------------
revno: 4064.1.2
revision-id: andrew.bennetts at canonical.com-20090302041511-uq1s5tns2qnh803c
parent: andrew.bennetts at canonical.com-20090302033829-7fzta4klg2yshnut
committer: Andrew Bennetts <andrew.bennetts at canonical.com>
branch nick: streaming-body-error
timestamp: Mon 2009-03-02 15:15:11 +1100
message:
Refactor server-side error translation, improve tests.
modified:
bzrlib/smart/protocol.py protocol.py-20061108035435-ot0lstk2590yqhzr-1
bzrlib/smart/request.py request.py-20061108095550-gunadhxmzkdjfeek-1
bzrlib/tests/test_smart_transport.py test_ssh_transport.py-20060608202016-c25gvf1ob7ypbus6-2
------------------------------------------------------------
revno: 4064.1.1
revision-id: andrew.bennetts at canonical.com-20090302033829-7fzta4klg2yshnut
parent: pqm at pqm.ubuntu.com-20090301121857-3lxdqurjbln7ybd2
committer: Andrew Bennetts <andrew.bennetts at canonical.com>
branch nick: streaming-body-error
timestamp: Mon 2009-03-02 14:38:29 +1100
message:
Add TestResponseEncodingProtocolThree.test_send_broken_body_stream, and make it pass.
modified:
bzrlib/smart/protocol.py protocol.py-20061108035435-ot0lstk2590yqhzr-1
bzrlib/tests/test_smart_transport.py test_ssh_transport.py-20060608202016-c25gvf1ob7ypbus6-2
=== modified file 'bzrlib/smart/protocol.py'
--- a/bzrlib/smart/protocol.py 2009-02-23 15:42:47 +0000
+++ b/bzrlib/smart/protocol.py 2009-03-02 04:22:53 +0000
@@ -1158,12 +1158,55 @@
if response.body is not None:
self._write_prefixed_body(response.body)
elif response.body_stream is not None:
- for chunk in response.body_stream:
- self._write_prefixed_body(chunk)
- self.flush()
+ for exc_info, chunk in _iter_with_errors(response.body_stream):
+ if exc_info is not None:
+ self._write_error_status()
+ error_struct = request._translate_error(exc_info[1])
+ self._write_structure(error_struct)
+ break
+ else:
+ self._write_prefixed_body(chunk)
+ self.flush()
self._write_end()
+def _iter_with_errors(iterable):
+ """Handle errors from iterable.next().
+
+ Use like::
+
+ for exc_info, value in _iter_with_errors(iterable):
+ ...
+
+ This is a safer alternative to::
+
+ try:
+ for value in iterable:
+ ...
+ except:
+ ...
+
+ Because the latter will catch errors from the for-loop body, not just
+ iterable.next()
+
+ If an error occurs, exc_info will be a exc_info tuple, and the generator
+ will terminate. Otherwise exc_info will be None, and value will be the
+ value from iterable.next(). Note that KeyboardInterrupt and SystemExit
+ will not be itercepted.
+ """
+ iterator = iter(iterable)
+ while True:
+ try:
+ yield None, iterator.next()
+ except StopIteration:
+ return
+ except (KeyboardInterrupt, SystemExit):
+ raise
+ except Exception:
+ yield sys.exc_info(), None
+ return
+
+
class ProtocolThreeRequester(_ProtocolThreeEncoder, Requester):
def __init__(self, medium_request):
@@ -1242,20 +1285,19 @@
# have finished sending the stream. We would notice at the end
# anyway, but if the medium can deliver it early then it's good
# to short-circuit the whole request...
- try:
- for part in stream:
+ for exc_info, part in _iter_with_errors(stream):
+ if exc_info is not None:
+ # Iterating the stream failed. Cleanly abort the request.
+ self._write_error_status()
+ # Currently the client unconditionally sends ('error',) as the
+ # error args.
+ self._write_structure(('error',))
+ self._write_end()
+ self._medium_request.finished_writing()
+ raise exc_info[0], exc_info[1], exc_info[2]
+ else:
self._write_prefixed_body(part)
self.flush()
- except Exception:
- exc_info = sys.exc_info()
- # Iterating the stream failed. Cleanly abort the request.
- self._write_error_status()
- # Currently the client unconditionally sends ('error',) as the
- # error args.
- self._write_structure(('error',))
- self._write_end()
- self._medium_request.finished_writing()
- raise exc_info[0], exc_info[1], exc_info[2]
self._write_end()
self._medium_request.finished_writing()
=== modified file 'bzrlib/smart/request.py'
--- a/bzrlib/smart/request.py 2009-02-26 04:25:00 +0000
+++ b/bzrlib/smart/request.py 2009-03-02 04:15:11 +0000
@@ -33,6 +33,7 @@
errors,
registry,
revision,
+ trace,
urlutils,
)
from bzrlib.lazy_import import lazy_import
@@ -277,48 +278,11 @@
# be in SmartServerVFSRequestHandler somewhere.
try:
return callable(*args, **kwargs)
- except errors.NoSuchFile, e:
- return FailedSmartServerResponse(('NoSuchFile', e.path))
- except errors.FileExists, e:
- return FailedSmartServerResponse(('FileExists', e.path))
- except errors.DirectoryNotEmpty, e:
- return FailedSmartServerResponse(('DirectoryNotEmpty', e.path))
- except errors.ShortReadvError, e:
- return FailedSmartServerResponse(('ShortReadvError',
- e.path, str(e.offset), str(e.length), str(e.actual)))
- except errors.UnstackableRepositoryFormat, e:
- return FailedSmartServerResponse(('UnstackableRepositoryFormat',
- str(e.format), e.url))
- except errors.UnstackableBranchFormat, e:
- return FailedSmartServerResponse(('UnstackableBranchFormat',
- str(e.format), e.url))
- except errors.NotStacked, e:
- return FailedSmartServerResponse(('NotStacked',))
- except UnicodeError, e:
- # If it is a DecodeError, than most likely we are starting
- # with a plain string
- str_or_unicode = e.object
- if isinstance(str_or_unicode, unicode):
- # XXX: UTF-8 might have \x01 (our protocol v1 and v2 seperator
- # byte) in it, so this encoding could cause broken responses.
- # Newer clients use protocol v3, so will be fine.
- val = 'u:' + str_or_unicode.encode('utf-8')
- else:
- val = 's:' + str_or_unicode.encode('base64')
- # This handles UnicodeEncodeError or UnicodeDecodeError
- return FailedSmartServerResponse((e.__class__.__name__,
- e.encoding, val, str(e.start), str(e.end), e.reason))
- except errors.TransportNotPossible, e:
- if e.msg == "readonly transport":
- return FailedSmartServerResponse(('ReadOnlyError', ))
- else:
- raise
- except errors.ReadError, e:
- # cannot read the file
- return FailedSmartServerResponse(('ReadError', e.path))
- except errors.PermissionDenied, e:
- return FailedSmartServerResponse(
- ('PermissionDenied', e.path, e.extra))
+ except (KeyboardInterrupt, SystemExit):
+ raise
+ except Exception, err:
+ err_struct = _translate_error(err)
+ return FailedSmartServerResponse(err_struct)
def headers_received(self, headers):
# Just a no-op at the moment.
@@ -342,6 +306,49 @@
pass
+def _translate_error(err):
+ if isinstance(err, errors.NoSuchFile):
+ return ('NoSuchFile', err.path)
+ elif isinstance(err, errors.FileExists):
+ return ('FileExists', err.path)
+ elif isinstance(err, errors.DirectoryNotEmpty):
+ return ('DirectoryNotEmpty', err.path)
+ elif isinstance(err, errors.ShortReadvError):
+ return ('ShortReadvError', err.path, str(err.offset), str(err.length),
+ str(err.actual))
+ elif isinstance(err, errors.UnstackableRepositoryFormat):
+ return (('UnstackableRepositoryFormat', str(err.format), err.url))
+ elif isinstance(err, errors.UnstackableBranchFormat):
+ return ('UnstackableBranchFormat', str(err.format), err.url)
+ elif isinstance(err, errors.NotStacked):
+ return ('NotStacked',)
+ elif isinstance(err, UnicodeError):
+ # If it is a DecodeError, than most likely we are starting
+ # with a plain string
+ str_or_unicode = err.object
+ if isinstance(str_or_unicode, unicode):
+ # XXX: UTF-8 might have \x01 (our protocol v1 and v2 seperator
+ # byte) in it, so this encoding could cause broken responses.
+ # Newer clients use protocol v3, so will be fine.
+ val = 'u:' + str_or_unicode.encode('utf-8')
+ else:
+ val = 's:' + str_or_unicode.encode('base64')
+ # This handles UnicodeEncodeError or UnicodeDecodeError
+ return (err.__class__.__name__, err.encoding, val, str(err.start),
+ str(err.end), err.reason)
+ elif isinstance(err, errors.TransportNotPossible):
+ if err.msg == "readonly transport":
+ return ('ReadOnlyError', )
+ elif isinstance(err, errors.ReadError):
+ # cannot read the file
+ return ('ReadError', err.path)
+ elif isinstance(err, errors.PermissionDenied):
+ return ('PermissionDenied', err.path, err.extra)
+ # Unserialisable error. Log it, and return a generic error
+ trace.log_exception_quietly()
+ return ('error', str(err))
+
+
class HelloRequest(SmartServerRequest):
"""Answer a version request with the highest protocol version this server
supports.
=== modified file 'bzrlib/tests/test_smart_request.py'
--- a/bzrlib/tests/test_smart_request.py 2009-02-11 09:54:36 +0000
+++ b/bzrlib/tests/test_smart_request.py 2009-03-02 05:38:55 +0000
@@ -43,20 +43,3 @@
handler.end_received()
# Request done, no exception was raised.
- def test_unexpected_body(self):
- """If a request implementation receives an unexpected body, it
- raises an error.
- """
- # Create a SmartServerRequestHandler with a SmartServerRequest subclass
- # that does not implement do_body.
- handler = request.SmartServerRequestHandler(
- None, {'foo': NoBodyRequest}, '/')
- # Emulate a request with a body
- handler.args_received(('foo',))
- handler.accept_body('some body bytes')
- # Note that the exception currently occurs at the end of the request.
- # In principle it would also be ok for it to happen earlier, during
- # accept_body.
- exc = self.assertRaises(errors.SmartProtocolError, handler.end_received)
- self.assertEquals('Request does not expect a body', exc.details)
-
=== modified file 'bzrlib/tests/test_smart_transport.py'
--- a/bzrlib/tests/test_smart_transport.py 2009-02-23 15:29:35 +0000
+++ b/bzrlib/tests/test_smart_transport.py 2009-03-02 04:15:11 +0000
@@ -2385,21 +2385,12 @@
return response_handler
def test_interrupted_by_error(self):
- interrupted_body_stream = (
- 'oS' # successful response
- 's\0\0\0\x02le' # empty args
- 'b\0\0\0\x09chunk one' # first chunk
- 'b\0\0\0\x09chunk two' # second chunk
- 'oE' # error flag
- 's\0\0\0\x0el5:error3:abce' # bencoded error
- 'e' # message end
- )
response_handler = self.make_response_handler(interrupted_body_stream)
stream = response_handler.read_streamed_body()
- self.assertEqual('chunk one', stream.next())
- self.assertEqual('chunk two', stream.next())
+ self.assertEqual('aaa', stream.next())
+ self.assertEqual('bbb', stream.next())
exc = self.assertRaises(errors.ErrorFromSmartServer, stream.next)
- self.assertEqual(('error', 'abc'), exc.error_tuple)
+ self.assertEqual(('error', 'Boom!'), exc.error_tuple)
def test_interrupted_by_connection_lost(self):
interrupted_body_stream = (
@@ -2796,6 +2787,17 @@
self.calls.append('finished_writing')
+interrupted_body_stream = (
+ 'oS' # status flag (success)
+ 's\x00\x00\x00\x08l4:argse' # args struct ('args,')
+ '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!')
+ 'e' # EOM
+ )
+
+
class TestResponseEncodingProtocolThree(tests.TestCase):
def make_response_encoder(self):
@@ -2817,6 +2819,22 @@
# end of message
'e')
+ def test_send_broken_body_stream(self):
+ encoder, out_stream = self.make_response_encoder()
+ encoder._headers = {}
+ def stream_that_fails():
+ yield 'aaa'
+ yield 'bbb'
+ raise Exception('Boom!')
+ response = _mod_request.SuccessfulSmartServerResponse(
+ ('args',), body_stream=stream_that_fails())
+ encoder.send_response(response)
+ expected_response = (
+ 'bzr message 3 (bzr 1.6)\n' # protocol marker
+ '\x00\x00\x00\x02de' # headers dict (empty)
+ + interrupted_body_stream)
+ self.assertEqual(expected_response, out_stream.getvalue())
+
class TestResponseEncoderBufferingProtocolThree(tests.TestCase):
"""Tests for buffering of responses.
More information about the bazaar-commits
mailing list