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 <gavinmak@google.com>
Reviewed-by: Scott Lee <ddoman@google.com>
diff --git a/project.py b/project.py
index 9bfd482..73263d9 100644
--- a/project.py
+++ b/project.py
@@ -1539,18 +1539,14 @@
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 @@
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 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 @@
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 @@
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 @@
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 82c065e..ed2e516 100644
--- a/subcmds/sync.py
+++ b/subcmds/sync.py
@@ -1092,10 +1092,10 @@
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 @@
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 @@
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 5955e40..940c69f 100644
--- a/tests/test_subcmds_sync.py
+++ b/tests/test_subcmds_sync.py
@@ -801,6 +801,7 @@
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 @@
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])
