[PATCH 3 of 3] bundle2: ignore errors seeking a bundle after an exception (issue4784)

Gregory Szorc gregory.szorc at gmail.com
Sun Apr 16 14:58:09 EDT 2017


# HG changeset patch
# User Gregory Szorc <gregory.szorc at gmail.com>
# Date 1492368908 25200
#      Sun Apr 16 11:55:08 2017 -0700
# Node ID 9ab4130a8302b32f7c86171a669e2ea2bfdd496b
# Parent  420b7094137e851132849b3cc8ddecf255a09bcb
bundle2: ignore errors seeking a bundle after an exception (issue4784)

Many have seen a "stream ended unexpectedly" error. This message is
raised from changegroup.readexactly() when a read(n) operation fails
to return exactly N bytes.

I believe most occurrences of this error in the wild stem from
the code changed in this patch. Before, if bundle2's part applicator
raised an Exception when processing/applying parts, the exception
handler would attempt to iterate the remaining parts. If I/O
during this iteration failed, it would likely raise the
"stream ended unexpectedly" exception.

The problem with this approach is that if we already encountered
an I/O error iterating the bundle2 data during application, then
any further I/O would almost certainly fail. If the original stream
were closed, changegroup.readexactly() would obtain an empty
string, triggering "stream ended unexpectedly" with "got 0." This is
the error message that users would see. What's worse is that the
original I/O related exception would be lost since a new exception
would be raised. This made debugging the actual I/O failure
effectively impossible.

This patch changes the exception handler for bundle2 application to
ignore errors when seeking the underlying stream. When the underlying
error is I/O related, the seek should fail fast and the original
exception will be re-raised. The output changes in
test-http-bad-server.t demonstrate this.

When the underlying error is not I/O related and the stream can be
seeked, the old behavior is preserved.

diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
--- a/mercurial/bundle2.py
+++ b/mercurial/bundle2.py
@@ -354,9 +354,19 @@ def processbundle(repo, unbundler, trans
         for nbpart, part in iterparts:
             _processpart(op, part)
     except Exception as exc:
-        for nbpart, part in iterparts:
-            # consume the bundle content
-            part.seek(0, 2)
+        # Any exceptions seeking to the end of the bundle at this point are
+        # almost certainly related to the underlying stream being bad.
+        # And, chances are that the exception we're handling is related to
+        # getting in that bad state. So, we swallow the seeking error and
+        # re-raise the original error.
+        seekerror = False
+        try:
+            for nbpart, part in iterparts:
+                # consume the bundle content
+                part.seek(0, 2)
+        except Exception:
+            seekerror = True
+
         # Small hack to let caller code distinguish exceptions from bundle2
         # processing from processing the old format. This is mostly
         # needed to handle different return codes to unbundle according to the
@@ -370,7 +380,13 @@ def processbundle(repo, unbundler, trans
             replycaps = op.reply.capabilities
         exc._replycaps = replycaps
         exc._bundle2salvagedoutput = salvaged
-        raise
+
+        # Re-raising from a variable loses the original stack. So only use
+        # that form if we need to.
+        if seekerror:
+            raise exc
+        else:
+            raise
     finally:
         repo.ui.debug('bundle2-input-bundle: %i parts total\n' % nbpart)
 
diff --git a/tests/test-http-bad-server.t b/tests/test-http-bad-server.t
--- a/tests/test-http-bad-server.t
+++ b/tests/test-http-bad-server.t
@@ -686,7 +686,8 @@ Server stops sending after bundle2 part 
   adding changesets
   transaction abort!
   rollback completed
-  abort: stream ended unexpectedly (got 0 bytes, expected 4)
+  abort: HTTP request error (incomplete response)
+  (this may be an intermittent network failure; if the error persists, consider contacting the network or server operator)
   [255]
 
   $ killdaemons.py $DAEMON_PIDS
@@ -715,7 +716,8 @@ Server stops after bundle2 part payload 
   adding changesets
   transaction abort!
   rollback completed
-  abort: stream ended unexpectedly (got 0 bytes, expected 4)
+  abort: HTTP request error (incomplete response)
+  (this may be an intermittent network failure; if the error persists, consider contacting the network or server operator)
   [255]
 
   $ killdaemons.py $DAEMON_PIDS
@@ -745,7 +747,8 @@ Server stops sending in middle of bundle
   adding changesets
   transaction abort!
   rollback completed
-  abort: stream ended unexpectedly (got 0 bytes, expected 4)
+  abort: HTTP request error (incomplete response)
+  (this may be an intermittent network failure; if the error persists, consider contacting the network or server operator)
   [255]
 
   $ killdaemons.py $DAEMON_PIDS


More information about the Mercurial-devel mailing list