[PATCH 2 of 2 STABLE] transaction: ignore I/O errors during abort logging (issue5658)

[Gregory Szorc]

# HG changeset patch
# User Gregory Szorc <gregory.szorc at gmail.com>
# Date 1502742785 25200
#      Mon Aug 14 13:33:05 2017 -0700
# Branch stable
# Node ID 7e80460cf08e68d812c0e2e662e3b93201cafe4f
# Parent  0a33f202bca4ee7ea126e7638bb74b5d58775858
transaction: ignore I/O errors during abort logging (issue5658)

e9646ff34d55 and 1bfb9a63b98e refactored ui methods to no longer
silently swallow some IOError instances.

This had the unfortunate side-effect of causing StdioError to
bubble up to transaction aborts, leading to an uncaught exception
and failure to roll back a transaction. This could occur when a
remote HTTP or SSH client connection dropped (possibly via ^C).

This commit adds code in transaction abort to explicitly ignore
StdioError in some cases.

The solution here is extremely brittle and not at all complete.
For example, if an abort callback handler performs a ui.write()
and gets a StdioError, we still have an incomplete rollback. We
can't ignore StdioError from transaction._abort() because we have
no clue in what context it was raised and if the abort callback
did its job.

The proper solution is to make some stdio I/O errors non-fatal
during transaction abort. Individual call sites shouldn't have
to know to catch and ignore errors during logging, IMO. This was
the previous behavior before e9646ff34d55 and 1bfb9a63b98e.
I'm not sure how to implement this in a manner appropriate for
stable because the abort method and transaction object don't have
an explicit handle on the ui instance. We could potentially
revert e9646ff34d55 and 1bfb9a63b98e. But I'm not sure of the
consequences.

diff --git a/mercurial/transaction.py b/mercurial/transaction.py
--- a/mercurial/transaction.py
+++ b/mercurial/transaction.py
@@ -569,9 +569,23 @@ class transaction(object):
                     self.opener.unlink(self.journal)
                 return
 
-            self.report(_("transaction abort!\n"))
+            # stdio here may be connected to a client, which may have
+            # disconnected, leading to an I/O error. We treat these as
+            # non-fatal because failure to print a logging message should
+            # not interfere with basic transaction behavior.
+            # TODO consider using a context manager or a wrapper around the
+            # underlying ui that makes I/O errors non-fatal. Because trapping
+            # each call site is laborious and error prone.
+            try:
+                self.report(_("transaction abort!\n"))
+            except error.StdioError:
+                pass
 
             try:
+                # We can't reliably catch StdioError here because we don't
+                # know what the appropriate action to take for any given
+                # callback is. So, errors here will result in incomplete
+                # rollback until we ignore StdioError at the source.
                 for cat in sorted(self._abortcallback):
                     self._abortcallback[cat](self)
                 # Prevent double usage and help clear cycles.
@@ -579,7 +593,11 @@ class transaction(object):
                 _playback(self.journal, self.report, self.opener, self._vfsmap,
                           self.entries, self._backupentries, False,
                           checkambigfiles=self.checkambigfiles)
-                self.report(_("rollback completed\n"))
+
+                try:
+                    self.report(_("rollback completed\n"))
+                except error.StdioError:
+                    pass
             except BaseException:
                 self.report(_("rollback failed - please run hg recover\n"))
         finally:
diff --git a/tests/test-rollback.t b/tests/test-rollback.t
--- a/tests/test-rollback.t
+++ b/tests/test-rollback.t
@@ -263,14 +263,12 @@ An I/O error writing "transaction abort"
 
   $ echo 1 > foo
   $ hg --config ui.ioerrors=txnabort --config hooks.pretxncommit=false commit -m 'error during abort message'
-  *: 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]
 
   $ cd ..
 
@@ -296,7 +294,6 @@ An I/O error writing "rollback completed
   $ hg --config ui.ioerrors=txnrollback --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]