Patchwork D10074: wireprotov1peer: don't raise internal errors in some cases

login
register
mail settings
Submitter phabricator
Date Feb. 25, 2021, 3:37 p.m.
Message ID <differential-rev-PHID-DREV-lfnr2mpxqbrbdqnnbiup-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/48389/
State Superseded
Headers show

Comments

phabricator - Feb. 25, 2021, 3:37 p.m.
valentin.gatienbaron created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  Specifically, when the peer is closed in the middle of a batch of rpcs.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D10074

AFFECTED FILES
  mercurial/scmutil.py
  mercurial/wireprotov1peer.py
  tests/test-ssh-batch.t

CHANGE DETAILS




To: valentin.gatienbaron, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/tests/test-ssh-batch.t b/tests/test-ssh-batch.t
--- a/tests/test-ssh-batch.t
+++ b/tests/test-ssh-batch.t
@@ -9,5 +9,7 @@ 
 fails (thus causing the sshpeer to be stopped), the errors from the
 further lookups don't result in tracebacks.
 
-  $ hg pull -r b0 -r nosuchbookmark $(for i in $($TESTDIR/seq.py 1 20); do echo -r b$i; done) -e "\"$PYTHON\" \"$TESTDIR/dummyssh\"" ssh://user@dummy/$(pwd)/../a |& tail -n 1
-  StopIteration
+  $ hg pull -r b0 -r nosuchbookmark $(for i in $($TESTDIR/seq.py 1 20); do echo -r b$i; done) -e "\"$PYTHON\" \"$TESTDIR/dummyssh\"" ssh://user@dummy/$(pwd)/../a
+  pulling from ssh://user@dummy/$TESTTMP/b/../a
+  abort: unknown revision 'nosuchbookmark'
+  [255]
diff --git a/mercurial/wireprotov1peer.py b/mercurial/wireprotov1peer.py
--- a/mercurial/wireprotov1peer.py
+++ b/mercurial/wireprotov1peer.py
@@ -310,7 +310,7 @@ 
                 if not f.done():
                     f.set_exception(
                         error.ResponseError(
-                            _(b'unfulfilled batch command response')
+                            _(b'unfulfilled batch command response'), None
                         )
                     )
 
@@ -322,16 +322,27 @@ 
         for command, f, batchable, fremote in states:
             # Grab raw result off the wire and teach the internal future
             # about it.
-            remoteresult = next(wireresults)
-            fremote.set(remoteresult)
+            try:
+                remoteresult = next(wireresults)
+            except StopIteration:
+                # This can happen in particular because next(batchable)
+                # in the previous iteration can call peer._abort, which
+                # may close the peer.
+                f.set_exception(
+                    error.ResponseError(
+                        _(b'unfulfilled batch command response'), None
+                    )
+                )
+            else:
+                fremote.set(remoteresult)
 
-            # And ask the coroutine to decode that value.
-            try:
-                result = next(batchable)
-            except Exception:
-                pycompat.future_set_exception_info(f, sys.exc_info()[1:])
-            else:
-                f.set_result(result)
+                # And ask the coroutine to decode that value.
+                try:
+                    result = next(batchable)
+                except Exception:
+                    pycompat.future_set_exception_info(f, sys.exc_info()[1:])
+                else:
+                    f.set_result(result)
 
 
 @interfaceutil.implementer(
diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
--- a/mercurial/scmutil.py
+++ b/mercurial/scmutil.py
@@ -201,7 +201,9 @@ 
         msg = inst.args[1]
         if isinstance(msg, type(u'')):
             msg = pycompat.sysbytes(msg)
-        if not isinstance(msg, bytes):
+        if msg is None:
+            ui.error(b"\n")
+        elif not isinstance(msg, bytes):
             ui.error(b" %r\n" % (msg,))
         elif not msg:
             ui.error(_(b" empty string\n"))