sync: Record and propagate errors from deferred actions

Failures in deferred sync actions were not recorded because `_Later.Run`
discarded the `GitError` exception. Record the specific error using
`syncbuf.fail()` and propagate it for proper error aggregation and
reporting.

Bug: 438178765
Change-Id: Iad59e389f9677bd6b8d873ee1ea2aa6ce44c86fa
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/498141
Tested-by: Gavin Mak <[email protected]>
Reviewed-by: Scott Lee <[email protected]>

Author: gavinmak

project.py

@@ -1539,18 +1539,14 @@ def Sync_LocalHalf(
         force_checkout=False,
         force_rebase=False,
         submodules=False,
-        errors=None,
         verbose=False,
     ):
         """Perform only the local IO portion of the sync process.
 
         Network access is not required.
         """
-        if errors is None:
-            errors = []
 
         def fail(error: Exception):
-            errors.append(error)
             syncbuf.fail(self, error)
 
         if not os.path.exists(self.gitdir):
@@ -4031,7 +4027,8 @@ def Run(self, syncbuf):
             if not self.quiet:
                 out.nl()
             return True
-        except GitError:
+        except GitError as e:
+            syncbuf.fail(self.project, e)
             out.nl()
             return False
 
@@ -4047,7 +4044,12 @@ def __init__(self, config):
 class SyncBuffer:
     def __init__(self, config, detach_head=False):
         self._messages = []
-        self._failures = []
+
+        # Failures that have not yet been printed. Cleared after printing.
+        self._pending_failures = []
+        # A persistent record of all failures during the buffer's lifetime.
+        self._all_failures = []
+
         self._later_queue1 = []
         self._later_queue2 = []
 
@@ -4062,7 +4064,9 @@ def info(self, project, fmt, *args):
         self._messages.append(_InfoMessage(project, fmt % args))
 
     def fail(self, project, err=None):
-        self._failures.append(_Failure(project, err))
+        failure = _Failure(project, err)
+        self._pending_failures.append(failure)
+        self._all_failures.append(failure)
         self._MarkUnclean()
 
     def later1(self, project, what, quiet):
@@ -4082,6 +4086,11 @@ def Recently(self):
         self.recent_clean = True
         return recent_clean
 
+    @property
+    def errors(self):
+        """Returns a list of all exceptions accumulated in the buffer."""
+        return [f.why for f in self._all_failures if f.why]
+
     def _MarkUnclean(self):
         self.clean = False
         self.recent_clean = False
@@ -4100,18 +4109,18 @@ def _RunQueue(self, queue):
         return True
 
     def _PrintMessages(self):
-        if self._messages or self._failures:
+        if self._messages or self._pending_failures:
             if os.isatty(2):
                 self.out.write(progress.CSI_ERASE_LINE)
             self.out.write("\r")
 
         for m in self._messages:
             m.Print(self)
-        for m in self._failures:
+        for m in self._pending_failures:
             m.Print(self)
 
         self._messages = []
-        self._failures = []
+        self._pending_failures = []
 
 
 class MetaProject(Project):

subcmds/sync.py

@@ -1092,10 +1092,10 @@ def _CheckoutOne(
                 force_sync=force_sync,
                 force_checkout=force_checkout,
                 force_rebase=force_rebase,
-                errors=errors,
                 verbose=verbose,
             )
             success = syncbuf.Finish()
+            errors.extend(syncbuf.errors)
         except KeyboardInterrupt:
             logger.error("Keyboard interrupt while processing %s", project.name)
         except GitError as e:
@@ -1753,10 +1753,10 @@ def _UpdateManifestProject(self, opt, mp, manifest_name, errors):
             mp.Sync_LocalHalf(
                 syncbuf,
                 submodules=mp.manifest.HasSubmodules,
-                errors=errors,
                 verbose=opt.verbose,
             )
             clean = syncbuf.Finish()
+            errors.extend(syncbuf.errors)
             self.event_log.AddSync(
                 mp, event_log.TASK_SYNC_LOCAL, start, time.time(), clean
             )
@@ -2284,19 +2284,17 @@ def _SyncOneProject(cls, opt, project_index, project) -> _SyncResult:
                             project.manifest.manifestProject.config,
                             detach_head=opt.detach_head,
                         )
-                        local_half_errors = []
                         project.Sync_LocalHalf(
                             syncbuf,
                             force_sync=opt.force_sync,
                             force_checkout=opt.force_checkout,
                             force_rebase=opt.rebase,
-                            errors=local_half_errors,
                             verbose=opt.verbose,
                         )
                         checkout_success = syncbuf.Finish()
-                        if local_half_errors:
+                        if syncbuf.errors:
                             checkout_error = SyncError(
-                                aggregate_errors=local_half_errors
+                                aggregate_errors=syncbuf.errors
                             )
                 except KeyboardInterrupt:
                     logger.error(

tests/test_subcmds_sync.py

@@ -801,6 +801,7 @@ def test_worker_successful_sync(self):
         with mock.patch("subcmds.sync.SyncBuffer") as mock_sync_buffer:
             mock_sync_buf_instance = mock.MagicMock()
             mock_sync_buf_instance.Finish.return_value = True
+            mock_sync_buf_instance.errors = []
             mock_sync_buffer.return_value = mock_sync_buf_instance
 
             result_obj = self.cmd._SyncProjectList(opt, [0])
@@ -909,6 +910,7 @@ def test_worker_local_only(self):
         with mock.patch("subcmds.sync.SyncBuffer") as mock_sync_buffer:
             mock_sync_buf_instance = mock.MagicMock()
             mock_sync_buf_instance.Finish.return_value = True
+            mock_sync_buf_instance.errors = []
             mock_sync_buffer.return_value = mock_sync_buf_instance
 
             result_obj = self.cmd._SyncProjectList(opt, [0])