Patchwork D1860: dispatch: handle IOError when writing to stderr

login
register
mail settings
Submitter phabricator
Date Jan. 15, 2018, 5 a.m.
Message ID <differential-rev-PHID-DREV-sjrbuthrpkxl3wsg22kj-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/26756/
State Superseded
Headers show

Comments

phabricator - Jan. 15, 2018, 5 a.m.
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




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

Patch

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():