Rev 4901: If we get ConnectionReset and we are aborting the call reset the request. in http://bazaar.launchpad.net/~jameinel/bzr/2.1-client-reconnect-819604

[John Arbash Meinel]

At http://bazaar.launchpad.net/~jameinel/bzr/2.1-client-reconnect-819604

------------------------------------------------------------
revno: 4901
revision-id: john at arbash-meinel.com-20111007121231-v1oo7htubsh9qiro
parent: john at arbash-meinel.com-20111007120719-snkpzqcm6hyksu28
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: 2.1-client-reconnect-819604
timestamp: Fri 2011-10-07 14:12:31 +0200
message:
  If we get ConnectionReset and we are aborting the call reset the request.
  
  Even if body_stream is set, we just kill the connection because we can't do anything
  with it. We may as well allow a future request to try to reconnect.
-------------- next part --------------
=== modified file 'bzrlib/smart/client.py'
--- a/bzrlib/smart/client.py	2011-10-07 11:57:42 +0000
+++ b/bzrlib/smart/client.py	2011-10-07 12:12:31 +0000
@@ -74,15 +74,16 @@
             # haven't started waiting for the reply yet. So try the request
             # again. We only issue a single retry, because if the connection
             # really is down, there is no reason to loop endlessly.
-            # XXX: If body_stream is not None, then we probably have a problem
-            #      here, because the body stream is partially consumed.
+
+            # Connection is dead, so close our end of it.
+            self._medium.reset()
             if body_stream is not None:
+                # We can't determine how much of body_stream got consumed
+                # before we noticed the connection is down, so we don't retry
+                # here.
                 raise
             trace.log_exception_quietly()
             trace.warning('ConnectionReset calling %s, retrying' % (method,))
-            self._medium.reset()
-            # encoder._medium_request.finished_writing()
-            # encoder._medium_request.finished_reading()
             encoder, response_handler = self._construct_protocol(
                 protocol_version)
             self._send_request_no_retry(encoder, method, args, body=body,

=== modified file 'bzrlib/tests/test_smart_transport.py'
--- a/bzrlib/tests/test_smart_transport.py	2011-10-07 12:07:19 +0000
+++ b/bzrlib/tests/test_smart_transport.py	2011-10-07 12:12:31 +0000
@@ -3448,6 +3448,28 @@
             vendor.calls)
         self.assertRaises(errors.ConnectionReset, handler.read_response_tuple)
 
+    def test__send_request_doesnt_retry_body_stream(self):
+        # We don't know how much of body_stream would get iterated as part of
+        # _send_request before it failed to actually send the request, so we
+        # just always fail in this condition.
+        response = StringIO()
+        output = StringIO()
+        vendor = FirstRejectedStringIOSSHVendor(response, output)
+        client_medium = medium.SmartSSHClientMedium(
+            'a host', 'a port', 'a user', 'a pass', 'base', vendor,
+            'bzr')
+        smart_client = client._SmartClient(client_medium)
+        self.assertRaises(errors.ConnectionReset,
+            smart_client._send_request, 3, 'hello', (), body_stream=['a', 'b'])
+        # We got one connect, but it fails, so we disconnect, but we don't
+        # retry it
+        self.assertEqual(
+            [('connect_ssh', 'a user', 'a pass', 'a host', 'a port',
+              ['bzr', 'serve', '--inet', '--directory=/', '--allow-writes']),
+             ('close',),
+            ],
+            vendor.calls)
+
 
 class LengthPrefixedBodyDecoder(tests.TestCase):