ui: restore behavior to ignore some I/O errors (issue5658)

e9646ff34d55 and 1bfb9a63b98e refactored ui methods to no longer
silently swallow some IOError instances. This is arguably the
correct thing to do. However, it had the unfortunate side-effect
of causing StdioError to bubble up to sensitive code like
transaction aborts, leading to an uncaught exceptions and failures
to e.g. roll back a transaction. This could occur when a remote
HTTP or SSH client connection dropped. The new behavior is
resulting in semi-frequent "abandonded transaction" errors on
multiple high-volume repositories at Mozilla.

This commit effectively reverts e9646ff34d55 and 1bfb9a63b98e to
restore the old behavior.

I agree with the principle that I/O errors shouldn't be ignored.
That makes this change... unfortunate. However, our hands are tied
for what to do on stable. I think the proper solution is for the
ui's behavior to be configurable (possibly via a context manager).
During critical sections like transaction rollback and abort, it
should be possible to suppress errors. But this feature would not
be appropriate on stable.

mercurial/ui.py

@@ -904,7 +904,8 class ui(object):
                 if not getattr(self.ferr, 'closed', False):
                     self.ferr.flush()
         except IOError as inst:
-            raise error.StdioError(inst)
+            if inst.errno not in (errno.EPIPE, errno.EIO, errno.EBADF):
+                raise error.StdioError(inst)
 
     def flush(self):
         # opencode timeblockedsection because this is a critical path
@@ -913,12 +914,14 class ui(object):
             try:
                 self.fout.flush()
             except IOError as err:
-                raise error.StdioError(err)
+                if err.errno not in (errno.EPIPE, errno.EIO, errno.EBADF):
+                    raise error.StdioError(err)
             finally:
                 try:
                     self.ferr.flush()
                 except IOError as err:
-                    raise error.StdioError(err)
+                    if err.errno not in (errno.EPIPE, errno.EIO, errno.EBADF):
+                        raise error.StdioError(err)
         finally:
             self._blockedtimes['stdio_blocked'] += \
                 (util.timer() - starttime) * 1000

tests/test-rollback.t

@@ -302,16 +302,12 An I/O error during pretxncommit is hand
   warn during txnclose
   $ echo 1 > foo
   $ hg --config ui.ioerrors=pretxncommit commit -m 'error during pretxncommit'
-  error: pretxncommit.badui hook raised an exception: [Errno *] simulated epipe (glob)
-  transaction abort!
-  warn during abort
-  rollback completed
-  [255]
+  warn during pretxnclose
+  warn during txnclose
 
   $ hg commit -m 'commit 1'
-  warn during pretxncommit
-  warn during pretxnclose
-  warn during txnclose
+  nothing changed
+  [1]
 
   $ cd ..
 
@@ -328,17 +324,11 An I/O error during pretxnclose is handl
   $ echo 1 > foo
   $ hg --config ui.ioerrors=pretxnclose commit -m 'error during pretxnclose'
   warn during pretxncommit
-  error: pretxnclose.badui hook raised an exception: [Errno *] simulated eio (glob)
-  transaction abort!
-  warn during abort
-  rollback completed
-  abort: simulated eio
-  [255]
+  warn during txnclose
 
   $ hg commit -m 'commit 1'
-  warn during pretxncommit
-  warn during pretxnclose
-  warn during txnclose
+  nothing changed
+  [1]
 
   $ cd ..
 
@@ -356,8 +346,6 An I/O error during txnclose is handled
   $ hg --config ui.ioerrors=txnclose commit -m 'error during txnclose'
   warn during pretxncommit
   warn during pretxnclose
-  error: txnclose.badui hook raised an exception: [Errno *] simulated badf (glob)
-  (run with --traceback for stack trace)
 
   $ hg commit -m 'commit 1'
   nothing changed
@@ -378,15 +366,15 An I/O error writing "transaction abort"
 
   $ echo 1 > foo
   $ hg --config ui.ioerrors=msgabort --config hooks.pretxncommit=false commit -m 'error during abort message'
-  abort: simulated ebadf
-  *: DeprecationWarning: use lock.release instead of del lock (glob)
-    return -1
+  warn during abort
+  rollback completed
+  abort: pretxncommit hook exited with status 1
   [255]
 
   $ hg commit -m 'commit 1'
-  abort: abandoned transaction found!
-  (run 'hg recover' to clean up transaction)
-  [255]
+  warn during pretxncommit
+  warn during pretxnclose
+  warn during txnclose
 
   $ cd ..
 
@@ -404,8 +392,6 An I/O error during txnabort should stil
   $ echo 1 > foo
   $ hg --config ui.ioerrors=txnabort --config hooks.pretxncommit=false commit -m 'error during abort'
   transaction abort!
-  error: txnabort.badui hook raised an exception: [Errno *] simulated epipe (glob)
-  (run with --traceback for stack trace)
   rollback completed
   abort: pretxncommit hook exited with status 1
   [255]
@@ -433,7 +419,6 An I/O error writing "rollback completed
   $ hg --config ui.ioerrors=msgrollback --config hooks.pretxncommit=false commit -m 'error during rollback message'
   transaction abort!
   warn during abort
-  rollback failed - please run hg recover
   abort: pretxncommit hook exited with status 1
   [255]
 
@@ -461,25 +446,12 of a transaction.
   $ echo 1 > foo
 
   $ hg --config ui.ioerrors=pretxncommit,pretxnclose,txnclose,txnabort,msgabort,msgrollback commit -m 'multiple errors'
-  error: pretxncommit.badui hook raised an exception: [Errno *] simulated epipe (glob)
-  abort: simulated ebadf
-  *: DeprecationWarning: use lock.release instead of del lock (glob)
-    return -1
-  [255]
 
   $ hg verify
-  abandoned transaction found - run hg recover
   checking changesets
   checking manifests
-   manifest@?: rev 1 points to nonexistent changeset 1
-   manifest@?: 94e0ee43dbfe not in changesets
   crosschecking files in changesets and manifests
   checking files
-   foo@?: rev 1 points to nonexistent changeset 1
-   (expected 0)
-  1 files, 1 changesets, 2 total revisions
-  1 warnings encountered!
-  3 integrity errors encountered!
-  [1]
+  1 files, 2 changesets, 2 total revisions
 
   $ cd ..