sync: Fix missing error details in interleaved summary

When checkout errors occurred in interleaved sync, they were wrapped in
a SyncError with no message, causing blank lines in the final summary.
Refactor _SyncResult to hold a list of exceptions, ensuring the original
error messages are propagated correctly.

Bug: 438178765
Change-Id: Ic25e515068959829cb6290cfd9e4c2d3963bbbea
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/498342
Reviewed-by: Scott Lee <ddoman@google.com>
Tested-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>

2 files changed, 27 insertions, 29 deletions

diff --git a/subcmds/sync.py b/subcmds/sync.py
index ed2e516a..582bd057 100644
--- a/subcmds/sync.py
+++ b/subcmds/sync.py
@@ -204,14 +204,13 @@ class _SyncResult(NamedTuple):
       relpath (str): The project's relative path from the repo client top.
       remote_fetched (bool): True if the remote was actually queried.
       fetch_success (bool): True if the fetch operation was successful.
-      fetch_error (Optional[Exception]): The Exception from a failed fetch,
-          or None.
+      fetch_errors (List[Exception]): The Exceptions from a failed fetch.
       fetch_start (Optional[float]): The time.time() when fetch started.
       fetch_finish (Optional[float]): The time.time() when fetch finished.
       checkout_success (bool): True if the checkout operation was
           successful.
-      checkout_error (Optional[Exception]): The Exception from a failed
-          checkout, or None.
+      checkout_errors (List[Exception]): The Exceptions from a failed
+          checkout.
       checkout_start (Optional[float]): The time.time() when checkout
           started.
       checkout_finish (Optional[float]): The time.time() when checkout
@@ -224,12 +223,12 @@ class _SyncResult(NamedTuple):
 
     remote_fetched: bool
     fetch_success: bool
-    fetch_error: Optional[Exception]
+    fetch_errors: List[Exception]
     fetch_start: Optional[float]
     fetch_finish: Optional[float]
 
     checkout_success: bool
-    checkout_error: Optional[Exception]
+    checkout_errors: List[Exception]
     checkout_start: Optional[float]
     checkout_finish: Optional[float]
 
@@ -2210,7 +2209,7 @@ later is required to fix a server side protocol bug.
         """Syncs a single project for interleaved sync."""
         fetch_success = False
         remote_fetched = False
-        fetch_error = None
+        fetch_errors = []
         fetch_start = None
         fetch_finish = None
         network_output = ""
@@ -2243,16 +2242,17 @@ later is required to fix a server side protocol bug.
                 )
                 fetch_success = sync_result.success
                 remote_fetched = sync_result.remote_fetched
-                fetch_error = sync_result.error
+                if sync_result.error:
+                    fetch_errors.append(sync_result.error)
             except KeyboardInterrupt:
                 logger.error(
                     "Keyboard interrupt while processing %s", project.name
                 )
             except GitError as e:
-                fetch_error = e
+                fetch_errors.append(e)
                 logger.error("error.GitError: Cannot fetch %s", e)
             except Exception as e:
-                fetch_error = e
+                fetch_errors.append(e)
                 logger.error(
                     "error: Cannot fetch %s (%s: %s)",
                     project.name,
@@ -2264,7 +2264,7 @@ later is required to fix a server side protocol bug.
                 network_output = network_output_capture.getvalue()
 
         checkout_success = False
-        checkout_error = None
+        checkout_errors = []
         checkout_start = None
         checkout_finish = None
         checkout_stderr = ""
@@ -2293,22 +2293,20 @@ later is required to fix a server side protocol bug.
                         )
                         checkout_success = syncbuf.Finish()
                         if syncbuf.errors:
-                            checkout_error = SyncError(
-                                aggregate_errors=syncbuf.errors
-                            )
+                            checkout_errors.extend(syncbuf.errors)
                 except KeyboardInterrupt:
                     logger.error(
                         "Keyboard interrupt while processing %s", project.name
                     )
                 except GitError as e:
-                    checkout_error = e
+                    checkout_errors.append(e)
                     logger.error(
                         "error.GitError: Cannot checkout %s: %s",
                         project.name,
                         e,
                     )
                 except Exception as e:
-                    checkout_error = e
+                    checkout_errors.append(e)
                     logger.error(
                         "error: Cannot checkout %s: %s: %s",
                         project.name,
@@ -2333,8 +2331,8 @@ later is required to fix a server side protocol bug.
             fetch_success=fetch_success,
             remote_fetched=remote_fetched,
             checkout_success=checkout_success,
-            fetch_error=fetch_error,
-            checkout_error=checkout_error,
+            fetch_errors=fetch_errors,
+            checkout_errors=checkout_errors,
             stderr_text=stderr_text.strip(),
             fetch_start=fetch_start,
             fetch_finish=fetch_finish,
@@ -2429,14 +2427,14 @@ later is required to fix a server side protocol bug.
                 if not success:
                     ret = False
                     err_event.set()
-                    if result.fetch_error:
-                        errors.append(result.fetch_error)
+                    if result.fetch_errors:
+                        errors.extend(result.fetch_errors)
                         self._interleaved_err_network = True
                         self._interleaved_err_network_results.append(
                             result.relpath
                         )
-                    if result.checkout_error:
-                        errors.append(result.checkout_error)
+                    if result.checkout_errors:
+                        errors.extend(result.checkout_errors)
                         self._interleaved_err_checkout = True
                         self._interleaved_err_checkout_results.append(
                             result.relpath
diff --git a/tests/test_subcmds_sync.py b/tests/test_subcmds_sync.py
index 940c69fc..6c9cc9ab 100644
--- a/tests/test_subcmds_sync.py
+++ b/tests/test_subcmds_sync.py
@@ -810,8 +810,8 @@ class InterleavedSyncTest(unittest.TestCase):
             result = result_obj.results[0]
             self.assertTrue(result.fetch_success)
             self.assertTrue(result.checkout_success)
-            self.assertIsNone(result.fetch_error)
-            self.assertIsNone(result.checkout_error)
+            self.assertEqual(result.fetch_errors, [])
+            self.assertEqual(result.checkout_errors, [])
             project.Sync_NetworkHalf.assert_called_once()
             project.Sync_LocalHalf.assert_called_once()
 
@@ -833,8 +833,8 @@ class InterleavedSyncTest(unittest.TestCase):
 
         self.assertFalse(result.fetch_success)
         self.assertFalse(result.checkout_success)
-        self.assertEqual(result.fetch_error, fetch_error)
-        self.assertIsNone(result.checkout_error)
+        self.assertEqual(result.fetch_errors, [fetch_error])
+        self.assertEqual(result.checkout_errors, [])
         project.Sync_NetworkHalf.assert_called_once()
         project.Sync_LocalHalf.assert_not_called()
 
@@ -871,7 +871,7 @@ class InterleavedSyncTest(unittest.TestCase):
 
         self.assertFalse(result.fetch_success)
         self.assertFalse(result.checkout_success)
-        self.assertEqual(result.fetch_error, fetch_error)
+        self.assertEqual(result.fetch_errors, [fetch_error])
         project.Sync_NetworkHalf.assert_called_once()
         project.Sync_LocalHalf.assert_not_called()
 
@@ -893,8 +893,8 @@ class InterleavedSyncTest(unittest.TestCase):
 
             self.assertTrue(result.fetch_success)
             self.assertFalse(result.checkout_success)
-            self.assertIsNone(result.fetch_error)
-            self.assertEqual(result.checkout_error, checkout_error)
+            self.assertEqual(result.fetch_errors, [])
+            self.assertEqual(result.checkout_errors, [checkout_error])
             project.Sync_NetworkHalf.assert_called_once()
             project.Sync_LocalHalf.assert_called_once()