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>
This commit is contained in:
parent
3e6acf2778
commit
a64149a7a7
3 changed files with 25 additions and 16 deletions
29
project.py
29
project.py
|
@ -1539,18 +1539,14 @@ class Project:
|
||||||
force_checkout=False,
|
force_checkout=False,
|
||||||
force_rebase=False,
|
force_rebase=False,
|
||||||
submodules=False,
|
submodules=False,
|
||||||
errors=None,
|
|
||||||
verbose=False,
|
verbose=False,
|
||||||
):
|
):
|
||||||
"""Perform only the local IO portion of the sync process.
|
"""Perform only the local IO portion of the sync process.
|
||||||
|
|
||||||
Network access is not required.
|
Network access is not required.
|
||||||
"""
|
"""
|
||||||
if errors is None:
|
|
||||||
errors = []
|
|
||||||
|
|
||||||
def fail(error: Exception):
|
def fail(error: Exception):
|
||||||
errors.append(error)
|
|
||||||
syncbuf.fail(self, error)
|
syncbuf.fail(self, error)
|
||||||
|
|
||||||
if not os.path.exists(self.gitdir):
|
if not os.path.exists(self.gitdir):
|
||||||
|
@ -4031,7 +4027,8 @@ class _Later:
|
||||||
if not self.quiet:
|
if not self.quiet:
|
||||||
out.nl()
|
out.nl()
|
||||||
return True
|
return True
|
||||||
except GitError:
|
except GitError as e:
|
||||||
|
syncbuf.fail(self.project, e)
|
||||||
out.nl()
|
out.nl()
|
||||||
return False
|
return False
|
||||||
|
|
||||||
|
@ -4047,7 +4044,12 @@ class _SyncColoring(Coloring):
|
||||||
class SyncBuffer:
|
class SyncBuffer:
|
||||||
def __init__(self, config, detach_head=False):
|
def __init__(self, config, detach_head=False):
|
||||||
self._messages = []
|
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_queue1 = []
|
||||||
self._later_queue2 = []
|
self._later_queue2 = []
|
||||||
|
|
||||||
|
@ -4062,7 +4064,9 @@ class SyncBuffer:
|
||||||
self._messages.append(_InfoMessage(project, fmt % args))
|
self._messages.append(_InfoMessage(project, fmt % args))
|
||||||
|
|
||||||
def fail(self, project, err=None):
|
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()
|
self._MarkUnclean()
|
||||||
|
|
||||||
def later1(self, project, what, quiet):
|
def later1(self, project, what, quiet):
|
||||||
|
@ -4082,6 +4086,11 @@ class SyncBuffer:
|
||||||
self.recent_clean = True
|
self.recent_clean = True
|
||||||
return recent_clean
|
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):
|
def _MarkUnclean(self):
|
||||||
self.clean = False
|
self.clean = False
|
||||||
self.recent_clean = False
|
self.recent_clean = False
|
||||||
|
@ -4100,18 +4109,18 @@ class SyncBuffer:
|
||||||
return True
|
return True
|
||||||
|
|
||||||
def _PrintMessages(self):
|
def _PrintMessages(self):
|
||||||
if self._messages or self._failures:
|
if self._messages or self._pending_failures:
|
||||||
if os.isatty(2):
|
if os.isatty(2):
|
||||||
self.out.write(progress.CSI_ERASE_LINE)
|
self.out.write(progress.CSI_ERASE_LINE)
|
||||||
self.out.write("\r")
|
self.out.write("\r")
|
||||||
|
|
||||||
for m in self._messages:
|
for m in self._messages:
|
||||||
m.Print(self)
|
m.Print(self)
|
||||||
for m in self._failures:
|
for m in self._pending_failures:
|
||||||
m.Print(self)
|
m.Print(self)
|
||||||
|
|
||||||
self._messages = []
|
self._messages = []
|
||||||
self._failures = []
|
self._pending_failures = []
|
||||||
|
|
||||||
|
|
||||||
class MetaProject(Project):
|
class MetaProject(Project):
|
||||||
|
|
|
@ -1092,10 +1092,10 @@ later is required to fix a server side protocol bug.
|
||||||
force_sync=force_sync,
|
force_sync=force_sync,
|
||||||
force_checkout=force_checkout,
|
force_checkout=force_checkout,
|
||||||
force_rebase=force_rebase,
|
force_rebase=force_rebase,
|
||||||
errors=errors,
|
|
||||||
verbose=verbose,
|
verbose=verbose,
|
||||||
)
|
)
|
||||||
success = syncbuf.Finish()
|
success = syncbuf.Finish()
|
||||||
|
errors.extend(syncbuf.errors)
|
||||||
except KeyboardInterrupt:
|
except KeyboardInterrupt:
|
||||||
logger.error("Keyboard interrupt while processing %s", project.name)
|
logger.error("Keyboard interrupt while processing %s", project.name)
|
||||||
except GitError as e:
|
except GitError as e:
|
||||||
|
@ -1753,10 +1753,10 @@ later is required to fix a server side protocol bug.
|
||||||
mp.Sync_LocalHalf(
|
mp.Sync_LocalHalf(
|
||||||
syncbuf,
|
syncbuf,
|
||||||
submodules=mp.manifest.HasSubmodules,
|
submodules=mp.manifest.HasSubmodules,
|
||||||
errors=errors,
|
|
||||||
verbose=opt.verbose,
|
verbose=opt.verbose,
|
||||||
)
|
)
|
||||||
clean = syncbuf.Finish()
|
clean = syncbuf.Finish()
|
||||||
|
errors.extend(syncbuf.errors)
|
||||||
self.event_log.AddSync(
|
self.event_log.AddSync(
|
||||||
mp, event_log.TASK_SYNC_LOCAL, start, time.time(), clean
|
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,
|
project.manifest.manifestProject.config,
|
||||||
detach_head=opt.detach_head,
|
detach_head=opt.detach_head,
|
||||||
)
|
)
|
||||||
local_half_errors = []
|
|
||||||
project.Sync_LocalHalf(
|
project.Sync_LocalHalf(
|
||||||
syncbuf,
|
syncbuf,
|
||||||
force_sync=opt.force_sync,
|
force_sync=opt.force_sync,
|
||||||
force_checkout=opt.force_checkout,
|
force_checkout=opt.force_checkout,
|
||||||
force_rebase=opt.rebase,
|
force_rebase=opt.rebase,
|
||||||
errors=local_half_errors,
|
|
||||||
verbose=opt.verbose,
|
verbose=opt.verbose,
|
||||||
)
|
)
|
||||||
checkout_success = syncbuf.Finish()
|
checkout_success = syncbuf.Finish()
|
||||||
if local_half_errors:
|
if syncbuf.errors:
|
||||||
checkout_error = SyncError(
|
checkout_error = SyncError(
|
||||||
aggregate_errors=local_half_errors
|
aggregate_errors=syncbuf.errors
|
||||||
)
|
)
|
||||||
except KeyboardInterrupt:
|
except KeyboardInterrupt:
|
||||||
logger.error(
|
logger.error(
|
||||||
|
|
|
@ -801,6 +801,7 @@ class InterleavedSyncTest(unittest.TestCase):
|
||||||
with mock.patch("subcmds.sync.SyncBuffer") as mock_sync_buffer:
|
with mock.patch("subcmds.sync.SyncBuffer") as mock_sync_buffer:
|
||||||
mock_sync_buf_instance = mock.MagicMock()
|
mock_sync_buf_instance = mock.MagicMock()
|
||||||
mock_sync_buf_instance.Finish.return_value = True
|
mock_sync_buf_instance.Finish.return_value = True
|
||||||
|
mock_sync_buf_instance.errors = []
|
||||||
mock_sync_buffer.return_value = mock_sync_buf_instance
|
mock_sync_buffer.return_value = mock_sync_buf_instance
|
||||||
|
|
||||||
result_obj = self.cmd._SyncProjectList(opt, [0])
|
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:
|
with mock.patch("subcmds.sync.SyncBuffer") as mock_sync_buffer:
|
||||||
mock_sync_buf_instance = mock.MagicMock()
|
mock_sync_buf_instance = mock.MagicMock()
|
||||||
mock_sync_buf_instance.Finish.return_value = True
|
mock_sync_buf_instance.Finish.return_value = True
|
||||||
|
mock_sync_buf_instance.errors = []
|
||||||
mock_sync_buffer.return_value = mock_sync_buf_instance
|
mock_sync_buffer.return_value = mock_sync_buf_instance
|
||||||
|
|
||||||
result_obj = self.cmd._SyncProjectList(opt, [0])
|
result_obj = self.cmd._SyncProjectList(opt, [0])
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue