From a64149a7a77814132629bbb4c07d922c2222df25 Mon Sep 17 00:00:00 2001 From: Gavin Mak Date: Wed, 13 Aug 2025 22:48:36 -0700 Subject: [PATCH] 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 Reviewed-by: Scott Lee --- project.py | 29 +++++++++++++++++++---------- subcmds/sync.py | 10 ++++------ tests/test_subcmds_sync.py | 2 ++ 3 files changed, 25 insertions(+), 16 deletions(-) diff --git a/project.py b/project.py index 9bfd4827a..73263d934 100644 --- a/project.py +++ b/project.py @@ -1539,18 +1539,14 @@ class Project: 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 @@ class _Later: 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 @@ class _SyncColoring(Coloring): 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 @@ class SyncBuffer: 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 @@ class SyncBuffer: 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 @@ class SyncBuffer: 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): diff --git a/subcmds/sync.py b/subcmds/sync.py index 82c065e20..ed2e516ab 100644 --- a/subcmds/sync.py +++ b/subcmds/sync.py @@ -1092,10 +1092,10 @@ later is required to fix a server side protocol bug. 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 @@ later is required to fix a server side protocol bug. 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 @@ later is required to fix a server side protocol bug. 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( diff --git a/tests/test_subcmds_sync.py b/tests/test_subcmds_sync.py index 5955e4048..940c69fce 100644 --- a/tests/test_subcmds_sync.py +++ b/tests/test_subcmds_sync.py @@ -801,6 +801,7 @@ class InterleavedSyncTest(unittest.TestCase): 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 @@ class InterleavedSyncTest(unittest.TestCase): 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])