[Request] [+-- ] D9517: commandserver: handle IOError related to flushing of streams

[pulkit (Pulkit Goyal)]

pulkit created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  After dispatch, without chg we have handling of flushing of streams and
  exception handling related to it. The exception handling part is important
  because there can be exceptions when flushing fout or ferr.
  
  One such case is in `test-basic.t` which was failing on python3+chg without this
  patch as this handling was missing from chg.
  
  Failure can be seen at
  https://foss.heptapod.net/octobus/mercurial-devel/-/jobs/128399
  
  Honestly I am not sure which one of `chgserver.py` or `commandserver.py` the
  change should go in.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/commandserver.py
  mercurial/dispatch.py

CHANGE DETAILS

diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -104,6 +104,36 @@
                 raise exc
 
 
+def closestdio(ui, err):
+    status = None
+    # In all cases we try to flush stdio streams.
+    if util.safehasattr(ui, b'fout'):
+        assert ui is not None  # help pytype
+        assert ui.fout is not None  # help pytype
+        try:
+            ui.fout.flush()
+        except IOError as e:
+            err = e
+            status = -1
+
+    if util.safehasattr(ui, b'ferr'):
+        assert ui is not None  # help pytype
+        assert ui.ferr is not None  # help pytype
+        try:
+            if err is not None and err.errno != errno.EPIPE:
+                ui.ferr.write(
+                    b'abort: %s\n' % encoding.strtolocal(err.strerror)
+                )
+            ui.ferr.flush()
+        # There's not much we can do about an I/O error here. So (possibly)
+        # change the status code and move on.
+        except IOError:
+            status = -1
+
+    _silencestdio()
+    return status
+
+
 def run():
     """run the command in sys.argv"""
     try:
@@ -117,31 +147,9 @@
             err = e
             status = -1
 
-        # In all cases we try to flush stdio streams.
-        if util.safehasattr(req.ui, b'fout'):
-            assert req.ui is not None  # help pytype
-            assert req.ui.fout is not None  # help pytype
-            try:
-                req.ui.fout.flush()
-            except IOError as e:
-                err = e
-                status = -1
-
-        if util.safehasattr(req.ui, b'ferr'):
-            assert req.ui is not None  # help pytype
-            assert req.ui.ferr is not None  # help pytype
-            try:
-                if err is not None and err.errno != errno.EPIPE:
-                    req.ui.ferr.write(
-                        b'abort: %s\n' % encoding.strtolocal(err.strerror)
-                    )
-                req.ui.ferr.flush()
-            # There's not much we can do about an I/O error here. So (possibly)
-            # change the status code and move on.
-            except IOError:
-                status = -1
-
-        _silencestdio()
+        ret = closestdio(req.ui, err)
+        if ret:
+            status = ret
     except KeyboardInterrupt:
         # Catch early/late KeyboardInterrupt as last ditch. Here nothing will
         # be printed to console to avoid another IOError/KeyboardInterrupt.
diff --git a/mercurial/commandserver.py b/mercurial/commandserver.py
--- a/mercurial/commandserver.py
+++ b/mercurial/commandserver.py
@@ -355,7 +355,18 @@
         )
 
         try:
-            ret = self._dispatchcommand(req) & 255
+            err = None
+            try:
+                status = self._dispatchcommand(req)
+            except error.StdioError as e:
+                status = -1
+                err = e
+
+            retval = dispatch.closestdio(req.ui, err)
+            if retval:
+                status = retval
+
+            ret = status & 255
             # If shutdown-on-interrupt is off, it's important to write the
             # result code *after* SIGINT handler removed. If the result code
             # were lost, the client wouldn't be able to continue processing.



To: pulkit, #hg-reviewers
Cc: mercurial-patches, mercurial-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mercurial-scm.org/pipermail/mercurial-patches/attachments/20201204/349e2f63/attachment-0001.html>