Fix hg recover throwing an error

Summary:
# Bug Description
When there is an interrupted transaction, a journal file will be left in the .hg/store file
We use hg recover to fix this. When hg recover is run with no interrupted transactions, it will return with error code 1.
We check the existence of the journal file to determine if we should run hg recover inside eden doctor. If something cleans up the journal file before we do, an about hg recover returning error code 1 is reported.

# This Diff
Catch the error when running hg recover. If it is the one where there is nothing to fix, ignore the error

Reviewed By: genevievehelsel

Differential Revision: D66900023

fbshipit-source-id: 5a0c4c808ae8447e84cbd37f055c9ea0f3593ac6

eden/fs/cli/doctor/check_hg.py

@@ -342,7 +342,14 @@ def check_for_error(self) -> List[str]:
         return []
 
     def repair(self) -> None:
-        self.backing_repo._run_hg(["recover"])
+        try:
+            self.backing_repo._run_hg(["recover"], subprocess.PIPE)
+        except subprocess.CalledProcessError as ex:
+            # hg recover exits unsuccessfully when there is nothing to recover
+            # Journal has been cleaned up before we could, just ignore the error
+            if ex.stderr.decode() == "no interrupted transaction available\n":
+                return
+            raise ex
 
 
 class PreviousEdenFSCrashedDuringCheckout(Problem):

eden/fs/cli/doctor/test/corrupt_hg_test.py

@@ -9,7 +9,10 @@
 
 import os
 import shutil
+import subprocess
+import sys
 import typing
+from pathlib import Path
 from unittest.mock import patch
 
 import eden.fs.cli.doctor as doctor
@@ -28,6 +31,12 @@ class CorruptHgTest(DoctorTestBase):
     # pyre-fixme[4]: Attribute must be annotated.
     maxDiff = None
 
+    def format_win_path_for_regex(self, path: str) -> str:
+        # Formats the path to be compatible with regex matching on windows
+        if sys.platform == "win32":
+            return path.replace("\\", "\\\\")
+        return path
+
     def setUp(self) -> None:
         self.instance = FakeEdenInstance(self.make_temporary_directory())
         self.checkout = self.instance.create_test_mount("test_mount", scm_type="hg")
@@ -143,6 +152,87 @@ def test_interrupted_transaction(self) -> None:
             out.getvalue(),
         )
 
+    def test_interrupted_transaction_race_condition(self) -> None:
+        store = self.backing_repo / ".hg" / "store"
+        store.mkdir()
+        journal: Path = store / "journal"
+        journal.write_text("")
+        outputs: typing.List[str] = [
+            "Found a journal file in backing repo, might have an interrupted transaction"
+        ]
+
+        def patched_func() -> typing.List[str]:
+            if outputs:
+                os.unlink(journal)
+                return [outputs.pop()]
+            return []
+
+        with patch(
+            "eden.fs.cli.doctor.test.lib.fake_hg_repo.FakeHgRepo._run_hg"
+        ) as mock_run_hg:
+            mock_run_hg.side_effect = subprocess.CalledProcessError(
+                1, "hg", b"", stderr=b"no interrupted transaction available\n"
+            )
+            with patch(
+                "eden.fs.cli.doctor.check_hg.AbandonedTransactionChecker.check_for_error",
+                wraps=patched_func,
+            ) as mock_check_error:
+                out = self.cure_what_ails_you(dry_run=False)
+                mock_check_error.assert_called_with()
+            self.assertEqual(
+                f"""\
+Checking {self.checkout.path}
+<yellow>- Found problem:<reset>
+Found inconsistent/missing data in {self.checkout.hg_dot_path}:
+  Found a journal file in backing repo, might have an interrupted transaction
+Repairing hg directory contents for {self.checkout.path}...<green>fixed<reset>
+
+<yellow>Successfully fixed 1 problem.<reset>
+""",
+                out.getvalue(),
+            )
+
+    def test_interrupted_transaction_repair_error(self) -> None:
+        store = self.backing_repo / ".hg" / "store"
+        store.mkdir()
+        journal: Path = store / "journal"
+        journal.write_text("")
+        outputs: typing.List[str] = [
+            "Found a journal file in backing repo, might have an interrupted transaction"
+        ]
+
+        def patched_func() -> typing.List[str]:
+            if outputs:
+                os.unlink(journal)
+                return [outputs.pop()]
+            return []
+
+        with patch(
+            "eden.fs.cli.doctor.test.lib.fake_hg_repo.FakeHgRepo._run_hg"
+        ) as mock_run_hg:
+            mock_run_hg.side_effect = subprocess.CalledProcessError(
+                1, "hg", b"", stderr=b"repair error\n"
+            )
+            with patch(
+                "eden.fs.cli.doctor.check_hg.AbandonedTransactionChecker.check_for_error",
+                wraps=patched_func,
+            ) as mock_check_error:
+                out = self.cure_what_ails_you(dry_run=False)
+                mock_check_error.assert_called_with()
+            checkout_path = self.format_win_path_for_regex(str(self.checkout.path))
+            hg_dot_path = self.format_win_path_for_regex(str(self.checkout.hg_dot_path))
+            self.assertRegex(
+                out.getvalue(),
+                rf"""Checking {checkout_path}
+<yellow>- Found problem:<reset>
+Found inconsistent/missing data in {hg_dot_path}:
+  Found a journal file in backing repo, might have an interrupted transaction
+Repairing hg directory contents for {checkout_path}...<red>error<reset>
+(.*\n)+
+<red>Failed to fix 1 problem.<reset>
+.*""",
+            )
+
     def _verify_hg_dir(self) -> None:
         hg_dir = self.checkout.path / ".hg"
         self.assertTrue((hg_dir / "dirstate").is_file())