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.

mercurial/bundle2.py

@@ -354,9 +354,19 b' 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 b' 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)
 

tests/test-http-bad-server.t

@@ -688,7 +688,8 b' 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
@@ -717,7 +718,8 b' 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
@@ -747,7 +749,8 b' 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