Merge lp:~spiv/bzr/win-serve-error into lp:~bzr/bzr/trunk-old

[Andrew Bennetts]

[Robert Collins (lifeless)]

On Wed, 2009-05-27 at 15:25 +0000, Andrew Bennetts wrote:
> - bytes = osutils.until_no_eintr(self.socket.recv,
> _MAX_READ_SIZE)
> + try:
> + bytes = osutils.until_no_eintr(self.socket.recv,
> _MAX_READ_SIZE)
> + except socket.error, e:
> + if e.args[0] in (errno.ECONNRESET, 10054):
> + # The connection was closed by the other side.
> + return ''
> + raise
> self._report_activity(len(bytes), 'read')
> return bytes

How about
- bytes = osutils.until_no_eintr(self.socket.recv, _MAX_READ_SIZE)
+ try:
+ bytes = osutils.until_no_eintr(self.socket.recv, _MAX_READ_SIZE)
+ except socket.error, e:
+ if e.args[0] in (errno.ECONNRESET, 10054):
+ # The connection was closed by the other side.
+ bytes = ''
+ else:
+ raise
         self._report_activity(len(bytes), 'read')
         return bytes

This avoids having a different code path exiting the function and may
scale better as other cases need to be caught.

-Rob

[Andrew Bennetts (spiv)]

Robert Collins wrote:
[...]
> How about
[...]
> + if e.args[0] in (errno.ECONNRESET, 10054):
> + # The connection was closed by the other side.
> + bytes = ''
[...]
> This avoids having a different code path exiting the function and may
> scale better as other cases need to be caught.

Yeah, that's better. Thanks!

-Andrew.

[Ian Clatworthy (ian-clatworthy)]

This looks fine to land after making the tweak Robert suggested.

review: Approve

=== modified file 'NEWS'
--- NEWS	2009-06-05 23:21:51 +0000
+++ NEWS	2009-06-09 03:35:18 +0000
@@ -66,6 +66,9 @@
 * Better message in ``bzr add`` output suggesting using ``bzr ignored`` to
   see which files can also be added.  (Jason Spashett, #76616)
 
+* ``bzr serve`` on Windows no longer displays a traceback simply because a
+  TCP client disconnected. (Andrew Bennetts)
+
 * Clarify the rules for locking and fallback repositories. Fix bugs in how
   ``RemoteRepository`` was handling fallbacks along with the
   ``_real_repository``. (Andrew Bennetts, John Arbash Meinel, #375496)
 
=== modified file 'bzrlib/smart/medium.py'
--- bzrlib/smart/medium.py	2009-05-05 22:11:41 +0000
+++ bzrlib/smart/medium.py	2009-06-09 03:35:18 +0000
@@ -285,11 +285,8 @@
         self._push_back(protocol.unused_data)
 
     def _read_bytes(self, desired_count):
-        # We ignore the desired_count because on sockets it's more efficient to
-        # read large chunks (of _MAX_READ_SIZE bytes) at a time.
-        bytes = osutils.until_no_eintr(self.socket.recv, _MAX_READ_SIZE)
-        self._report_activity(len(bytes), 'read')
-        return bytes
+        return _read_bytes_from_socket(
+            self.socket.recv, desired_count, self._report_activity)
 
     def terminate_due_to_error(self):
         # TODO: This should log to a server log file, but no such thing
@@ -884,19 +881,8 @@
         """See SmartClientMedium.read_bytes."""
         if not self._connected:
             raise errors.MediumNotConnected(self)
-        # We ignore the desired_count because on sockets it's more efficient to
-        # read large chunks (of _MAX_READ_SIZE bytes) at a time.
-        try:
-            bytes = osutils.until_no_eintr(self._socket.recv, _MAX_READ_SIZE)
-        except socket.error, e:
-            if len(e.args) and e.args[0] == errno.ECONNRESET:
-                # Callers expect an empty string in that case
-                return ''
-            else:
-                raise
-        else:
-            self._report_activity(len(bytes), 'read')
-            return bytes
+        return _read_bytes_from_socket(
+            self._socket.recv, count, self._report_activity)
 
 
 class SmartClientStreamMediumRequest(SmartClientMediumRequest):
@@ -938,3 +924,20 @@
         """
         self._medium._flush()
 
+
+def _read_bytes_from_socket(sock, desired_count, report_activity):
+    # We ignore the desired_count because on sockets it's more efficient to
+    # read large chunks (of _MAX_READ_SIZE bytes) at a time.
+    try:
+        bytes = osutils.until_no_eintr(sock, _MAX_READ_SIZE)
+    except socket.error, e:
+        if len(e.args) and e.args[0] in (errno.ECONNRESET, 10054):
+            # The connection was closed by the other side.  Callers expect an
+            # empty string to signal end-of-stream.
+            bytes = ''
+        else:
+            raise
+    else:
+        report_activity(len(bytes), 'read')
+    return bytes
+