Refactor errors for sync command

Per discussion in go/repo-error-update updated aggregated and exit
errors for sync command.

Aggregated errors are errors that result in eventual command failure.
Exit errors are errors that result in immediate command failure.

Also updated main.py to log aggregated and exit errors to git sessions
log

Bug: b/293344017
Change-Id: I77a21f14da32fe2e68c16841feb22de72e86a251
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/379614
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Tested-by: Jason Chang <jasonnc@google.com>
Commit-Queue: Jason Chang <jasonnc@google.com>

1 files changed, 114 insertions, 45 deletions

diff --git a/subcmds/sync.py b/subcmds/sync.py
index 5f8bc2f0..eaca50c9 100644
--- a/subcmds/sync.py
+++ b/subcmds/sync.py
@@ -63,9 +63,16 @@ from command import (
     MirrorSafeCommand,
     WORKER_BATCH_SIZE,
 )
-from error import RepoChangedException, GitError
+from error import (
+    RepoChangedException,
+    GitError,
+    RepoExitError,
+    SyncError,
+    UpdateManifestError,
+    RepoUnhandledExceptionError,
+)
 import platform_utils
-from project import SyncBuffer
+from project import SyncBuffer, DeleteWorktreeError
 from progress import Progress, elapsed_str, jobs_str
 from repo_trace import Trace
 import ssh
@@ -94,6 +101,7 @@ class _FetchOneResult(NamedTuple):
     """
 
     success: bool
+    errors: List[Exception]
     project: Project
     start: float
     finish: float
@@ -110,6 +118,7 @@ class _FetchResult(NamedTuple):
 
     success: bool
     projects: Set[str]
+    errors: List[Exception]
 
 
 class _FetchMainResult(NamedTuple):
@@ -120,6 +129,7 @@ class _FetchMainResult(NamedTuple):
     """
 
     all_projects: List[Project]
+    errors: List[Exception]
 
 
 class _CheckoutOneResult(NamedTuple):
@@ -133,11 +143,24 @@ class _CheckoutOneResult(NamedTuple):
     """
 
     success: bool
+    errors: List[Exception]
     project: Project
     start: float
     finish: float
 
 
+class SuperprojectError(SyncError):
+    """Superproject sync repo."""
+
+
+class SyncFailFastError(SyncError):
+    """Sync exit error when --fail-fast set."""
+
+
+class SmartSyncError(SyncError):
+    """Smart sync exit error."""
+
+
 class Sync(Command, MirrorSafeCommand):
     COMMON = True
     MULTI_MANIFEST_SUPPORT = True
@@ -588,7 +611,7 @@ later is required to fix a server side protocol bug.
                         file=sys.stderr,
                     )
                 if update_result.fatal and opt.use_superproject is not None:
-                    sys.exit(1)
+                    raise SuperprojectError()
         if need_unload:
             m.outer_client.manifest.Unload()
 
@@ -621,6 +644,7 @@ later is required to fix a server side protocol bug.
         self._sync_dict[k] = start
         success = False
         remote_fetched = False
+        errors = []
         buf = io.StringIO()
         try:
             sync_result = project.Sync_NetworkHalf(
@@ -644,6 +668,8 @@ later is required to fix a server side protocol bug.
             )
             success = sync_result.success
             remote_fetched = sync_result.remote_fetched
+            if sync_result.error:
+                errors.append(sync_result.error)
 
             output = buf.getvalue()
             if (opt.verbose or not success) and output:
@@ -659,6 +685,7 @@ later is required to fix a server side protocol bug.
             print(f"Keyboard interrupt while processing {project.name}")
         except GitError as e:
             print("error.GitError: Cannot fetch %s" % str(e), file=sys.stderr)
+            errors.append(e)
         except Exception as e:
             print(
                 "error: Cannot fetch %s (%s: %s)"
@@ -666,11 +693,14 @@ later is required to fix a server side protocol bug.
                 file=sys.stderr,
             )
             del self._sync_dict[k]
+            errors.append(e)
             raise
 
         finish = time.time()
         del self._sync_dict[k]
-        return _FetchOneResult(success, project, start, finish, remote_fetched)
+        return _FetchOneResult(
+            success, errors, project, start, finish, remote_fetched
+        )
 
     @classmethod
     def _FetchInitChild(cls, ssh_proxy):
@@ -701,6 +731,7 @@ later is required to fix a server side protocol bug.
         jobs = opt.jobs_network
         fetched = set()
         remote_fetched = set()
+        errors = []
         pm = Progress(
             "Fetching",
             len(projects),
@@ -745,6 +776,8 @@ later is required to fix a server side protocol bug.
                         finish,
                         success,
                     )
+                    if result.errors:
+                        errors.extend(result.errors)
                     if result.remote_fetched:
                         remote_fetched.add(project)
                     # Check for any errors before running any more tasks.
@@ -813,7 +846,7 @@ later is required to fix a server side protocol bug.
         if not self.outer_client.manifest.IsArchive:
             self._GCProjects(projects, opt, err_event)
 
-        return _FetchResult(ret, fetched)
+        return _FetchResult(ret, fetched, errors)
 
     def _FetchMain(
         self, opt, args, all_projects, err_event, ssh_proxy, manifest
@@ -832,6 +865,7 @@ later is required to fix a server side protocol bug.
             List of all projects that should be checked out.
         """
         rp = manifest.repoProject
+        errors = []
 
         to_fetch = []
         now = time.time()
@@ -843,6 +877,9 @@ later is required to fix a server side protocol bug.
         result = self._Fetch(to_fetch, opt, err_event, ssh_proxy)
         success = result.success
         fetched = result.projects
+        if result.errors:
+            errors.extend(result.errors)
+
         if not success:
             err_event.set()
 
@@ -854,8 +891,11 @@ later is required to fix a server side protocol bug.
                     "\nerror: Exited sync due to fetch errors.\n",
                     file=sys.stderr,
                 )
-                sys.exit(1)
-            return _FetchMainResult([])
+                raise SyncError(
+                    "error: Exited sync due to fetch errors.",
+                    aggregate_errors=errors,
+                )
+            return _FetchMainResult([], errors)
 
         # Iteratively fetch missing and/or nested unregistered submodules.
         previously_missing_set = set()
@@ -883,11 +923,13 @@ later is required to fix a server side protocol bug.
             result = self._Fetch(missing, opt, err_event, ssh_proxy)
             success = result.success
             new_fetched = result.projects
+            if result.errors:
+                errors.extend(result.errors)
             if not success:
                 err_event.set()
             fetched.update(new_fetched)
 
-        return _FetchMainResult(all_projects)
+        return _FetchMainResult(all_projects, errors)
 
     def _CheckoutOne(self, detach_head, force_sync, project):
         """Checkout work tree for one project
@@ -905,8 +947,11 @@ later is required to fix a server side protocol bug.
             project.manifest.manifestProject.config, detach_head=detach_head
         )
         success = False
+        errors = []
         try:
-            project.Sync_LocalHalf(syncbuf, force_sync=force_sync)
+            project.Sync_LocalHalf(
+                syncbuf, force_sync=force_sync, errors=errors
+            )
             success = syncbuf.Finish()
         except GitError as e:
             print(
@@ -914,6 +959,7 @@ later is required to fix a server side protocol bug.
                 % (project.name, str(e)),
                 file=sys.stderr,
             )
+            errors.append(e)
         except Exception as e:
             print(
                 "error: Cannot checkout %s: %s: %s"
@@ -925,9 +971,9 @@ later is required to fix a server side protocol bug.
         if not success:
             print("error: Cannot checkout %s" % (project.name), file=sys.stderr)
         finish = time.time()
-        return _CheckoutOneResult(success, project, start, finish)
+        return _CheckoutOneResult(success, errors, project, start, finish)
 
-    def _Checkout(self, all_projects, opt, err_results):
+    def _Checkout(self, all_projects, opt, err_results, checkout_errors):
         """Checkout projects listed in all_projects
 
         Args:
@@ -949,6 +995,10 @@ later is required to fix a server side protocol bug.
                 self.event_log.AddSync(
                     project, event_log.TASK_SYNC_LOCAL, start, finish, success
                 )
+
+                if result.errors:
+                    checkout_errors.extend(result.errors)
+
                 # Check for any errors before running any more tasks.
                 # ...we'll let existing jobs finish, though.
                 if success:
@@ -1214,10 +1264,9 @@ later is required to fix a server side protocol bug.
                             revisionId=None,
                             groups=None,
                         )
-                        if not project.DeleteWorktree(
+                        project.DeleteWorktree(
                             quiet=opt.quiet, force=opt.force_remove_dirty
-                        ):
-                            return 1
+                        )
 
         new_project_paths.sort()
         with open(file_path, "w") as fd:
@@ -1260,7 +1309,7 @@ later is required to fix a server side protocol bug.
                         file=sys.stderr,
                     )
                     platform_utils.remove(copylinkfile_path)
-                    return False
+                    raise
 
             need_remove_files = []
             need_remove_files.extend(
@@ -1285,12 +1334,10 @@ later is required to fix a server side protocol bug.
 
     def _SmartSyncSetup(self, opt, smart_sync_manifest_path, manifest):
         if not manifest.manifest_server:
-            print(
+            raise SmartSyncError(
                 "error: cannot smart sync: no manifest server defined in "
-                "manifest",
-                file=sys.stderr,
+                "manifest"
             )
-            sys.exit(1)
 
         manifest_server = manifest.manifest_server
         if not opt.quiet:
@@ -1368,33 +1415,28 @@ later is required to fix a server side protocol bug.
                     with open(smart_sync_manifest_path, "w") as f:
                         f.write(manifest_str)
                 except IOError as e:
-                    print(
+                    raise SmartSyncError(
                         "error: cannot write manifest to %s:\n%s"
                         % (smart_sync_manifest_path, e),
-                        file=sys.stderr,
+                        aggregate_errors=[e],
                     )
-                    sys.exit(1)
                 self._ReloadManifest(manifest_name, manifest)
             else:
-                print(
-                    "error: manifest server RPC call failed: %s" % manifest_str,
-                    file=sys.stderr,
+                raise SmartSyncError(
+                    "error: manifest server RPC call failed: %s" % manifest_str
                 )
-                sys.exit(1)
         except (socket.error, IOError, xmlrpc.client.Fault) as e:
-            print(
+            raise SmartSyncError(
                 "error: cannot connect to manifest server %s:\n%s"
                 % (manifest.manifest_server, e),
-                file=sys.stderr,
+                aggregate_errors=[e],
             )
-            sys.exit(1)
         except xmlrpc.client.ProtocolError as e:
-            print(
+            raise SmartSyncError(
                 "error: cannot connect to manifest server %s:\n%d %s"
                 % (manifest.manifest_server, e.errcode, e.errmsg),
-                file=sys.stderr,
+                aggregate_errors=[e],
             )
-            sys.exit(1)
 
         return manifest_name
 
@@ -1436,7 +1478,7 @@ later is required to fix a server side protocol bug.
         """
         if not opt.local_only:
             start = time.time()
-            success = mp.Sync_NetworkHalf(
+            result = mp.Sync_NetworkHalf(
                 quiet=opt.quiet,
                 verbose=opt.verbose,
                 current_branch_only=self._GetCurrentBranchOnly(
@@ -1453,19 +1495,24 @@ later is required to fix a server side protocol bug.
             )
             finish = time.time()
             self.event_log.AddSync(
-                mp, event_log.TASK_SYNC_NETWORK, start, finish, success
+                mp, event_log.TASK_SYNC_NETWORK, start, finish, result.success
             )
 
         if mp.HasChanges:
+            errors = []
             syncbuf = SyncBuffer(mp.config)
             start = time.time()
-            mp.Sync_LocalHalf(syncbuf, submodules=mp.manifest.HasSubmodules)
+            mp.Sync_LocalHalf(
+                syncbuf, submodules=mp.manifest.HasSubmodules, errors=errors
+            )
             clean = syncbuf.Finish()
             self.event_log.AddSync(
                 mp, event_log.TASK_SYNC_LOCAL, start, time.time(), clean
             )
             if not clean:
-                sys.exit(1)
+                raise UpdateManifestError(
+                    aggregate_errors=errors, project=mp.name
+                )
             self._ReloadManifest(manifest_name, mp.manifest)
 
     def ValidateOptions(self, opt, args):
@@ -1546,6 +1593,15 @@ later is required to fix a server side protocol bug.
         opt.jobs_checkout = min(opt.jobs_checkout, jobs_soft_limit)
 
     def Execute(self, opt, args):
+        errors = []
+        try:
+            self._ExecuteHelper(opt, args, errors)
+        except RepoExitError:
+            raise
+        except (KeyboardInterrupt, Exception) as e:
+            raise RepoUnhandledExceptionError(e, aggregate_errors=errors)
+
+    def _ExecuteHelper(self, opt, args, errors):
         manifest = self.outer_manifest
         if not opt.outer_manifest:
             manifest = self.manifest
@@ -1695,6 +1751,8 @@ later is required to fix a server side protocol bug.
                     result = self._FetchMain(
                         opt, args, all_projects, err_event, ssh_proxy, manifest
                     )
+                    if result.errors:
+                        errors.extend(result.errors)
                     all_projects = result.all_projects
 
             if opt.network_only:
@@ -1712,36 +1770,47 @@ later is required to fix a server side protocol bug.
                         "`repo sync -l` will update some local checkouts.",
                         file=sys.stderr,
                     )
-                    sys.exit(1)
+                    raise SyncFailFastError(aggregate_errors=errors)
 
         for m in self.ManifestList(opt):
             if m.IsMirror or m.IsArchive:
                 # Bail out now, we have no working tree.
                 continue
 
-            if self.UpdateProjectList(opt, m):
+            try:
+                self.UpdateProjectList(opt, m)
+            except Exception as e:
                 err_event.set()
                 err_update_projects = True
+                errors.append(e)
+                if isinstance(e, DeleteWorktreeError):
+                    errors.extend(e.aggregate_errors)
                 if opt.fail_fast:
                     print(
                         "\nerror: Local checkouts *not* updated.",
                         file=sys.stderr,
                     )
-                    sys.exit(1)
+                    raise SyncFailFastError(aggregate_errors=errors)
 
-            err_update_linkfiles = not self.UpdateCopyLinkfileList(m)
-            if err_update_linkfiles:
+            err_update_linkfiles = False
+            try:
+                self.UpdateCopyLinkfileList(m)
+            except Exception as e:
+                err_update_linkfiles = True
+                errors.append(e)
                 err_event.set()
                 if opt.fail_fast:
                     print(
                         "\nerror: Local update copyfile or linkfile failed.",
                         file=sys.stderr,
                     )
-                    sys.exit(1)
+                    raise SyncFailFastError(aggregate_errors=errors)
 
         err_results = []
         # NB: We don't exit here because this is the last step.
-        err_checkout = not self._Checkout(all_projects, opt, err_results)
+        err_checkout = not self._Checkout(
+            all_projects, opt, err_results, errors
+        )
         if err_checkout:
             err_event.set()
 
@@ -1784,7 +1853,7 @@ later is required to fix a server side protocol bug.
                 "error.",
                 file=sys.stderr,
             )
-            sys.exit(1)
+            raise SyncError(aggregate_errors=errors)
 
         # Log the previous sync analysis state from the config.
         self.git_event_log.LogDataConfigEvents(
@@ -1842,7 +1911,7 @@ def _PostRepoFetch(rp, repo_verify=True, verbose=False):
             try:
                 rp.work_git.reset("--keep", new_rev)
             except GitError as e:
-                sys.exit(str(e))
+                raise RepoUnhandledExceptionError(e)
             print("info: Restarting repo with latest version", file=sys.stderr)
             raise RepoChangedException(["--repo-upgraded"])
         else: