D1860: dispatch: handle IOError when writing to stderr

[indygreg (Gregory Szorc)]

indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Previously, attempts to write to stderr in dispatch.run() may lead to
  an exception being thrown. This would likely be handled by Python's
  default exception handler, which would print the exception and exit
  
  1.
  
  Code in this function is already catching IOError for stdout failures
  and converting to exit code 255 (-1 & 255 == 255). Why we weren't
  doing the same for stderr for the sake of consistency, I don't know.
  I do know that chg and hg diverged in behavior here (as the changed
  test-basic.t shows).
  
  After this commit, we catch I/O failure on stderr and change the
  exit code to 255. chg and hg now behave consistently. As a bonus,
  Rust hg also now passes this test.
  
  I'm skeptical at changing the exit code due to failures this late
  in the process. I think we should consider preserving the current
  exit code - assuming it is non-0. And, we may want to preserve the
  exit code completely if the I/O error is EPIPE (and potentially
  other special error classes). There's definitely room to tweak
  behavior. But for now, let's at least prevent the uncaught exception.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/dispatch.py
  tests/test-basic.t

CHANGE DETAILS

diff --git a/tests/test-basic.t b/tests/test-basic.t
--- a/tests/test-basic.t
+++ b/tests/test-basic.t
@@ -34,15 +34,7 @@
   [255]
 #endif
 
-#if devfull no-chg
-  $ hg status >/dev/full 2>&1
-  [1]
-
-  $ hg status ENOENT 2>/dev/full
-  [1]
-#endif
-
-#if devfull chg
+#if devfull
   $ hg status >/dev/full 2>&1
   [255]
 
diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -96,10 +96,16 @@
             err = e
             status = -1
     if util.safehasattr(req.ui, 'ferr'):
-        if err is not None and err.errno != errno.EPIPE:
-            req.ui.ferr.write('abort: %s\n' %
-                              encoding.strtolocal(err.strerror))
-        req.ui.ferr.flush()
+        try:
+            if err is not None and err.errno != errno.EPIPE:
+                req.ui.ferr.write('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
+
     sys.exit(status & 255)
 
 def _initstdio():



To: indygreg, #hg-reviewers
Cc: mercurial-devel