From cdd942ea5a93942b472a9254b6868fc9e744737b Mon Sep 17 00:00:00 2001
From: Eric Froemling <ericfroemling@gmail.com>
Date: Thu, 17 Aug 2023 20:19:34 -0500
Subject: [PATCH] better pcommandbatch error handling

---
 .idea/dictionaries/ericf.xml     |  1 +
 tools/efrotools/pcommand.py      | 21 +++++++--------------
 tools/efrotools/pcommandbatch.py | 19 ++++++++++---------
 3 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/.idea/dictionaries/ericf.xml b/.idea/dictionaries/ericf.xml
index b77ba5ac..ab6e0ed0 100644
--- a/.idea/dictionaries/ericf.xml
+++ b/.idea/dictionaries/ericf.xml
@@ -1675,6 +1675,7 @@
       <w>loginid</w>
       <w>logintoken</w>
       <w>logitech</w>
+      <w>logpath</w>
       <w>logput</w>
       <w>loofa</w>
       <w>loosey</w>
diff --git a/tools/efrotools/pcommand.py b/tools/efrotools/pcommand.py
index 1a255dc7..8357f7ac 100644
--- a/tools/efrotools/pcommand.py
+++ b/tools/efrotools/pcommand.py
@@ -204,9 +204,7 @@ def is_batch() -> bool:
     return _g_batch_server_mode
 
 
-def run_client_pcommand(
-    args: list[str], log_path: str, isatty: bool
-) -> tuple[int, str]:
+def run_client_pcommand(args: list[str], isatty: bool) -> tuple[int, str]:
     """Call a pcommand function when running as a batch server."""
     assert _g_batch_server_mode
     assert _g_thread_local_storage is not None
@@ -222,20 +220,15 @@ def run_client_pcommand(
     _g_thread_local_storage.argv = args
     _g_thread_local_storage.isatty = isatty
 
-    # Run the command.
+    # Run the command. This may return an explicit code or may throw an
+    # exception.
     resultcode: int = _run_pcommand(args)
 
-    # Return the result code and any output the command provided.
-    output = getattr(_g_thread_local_storage, 'output', '')
-
+    # Handle error result-codes consistently with exceptions.
     if resultcode != 0:
-        if output:
-            output += '\n'
-        output += (
-            f'Error: pcommandbatch command failed: {args}.'
-            f" For more info, see '{log_path}', or rerun with"
-            ' env var BA_PCOMMANDBATCH_DISABLE=1 to see all output here.\n'
-        )
+        raise RuntimeError(f'client pcommand returned error code {resultcode}.')
+
+    output = getattr(_g_thread_local_storage, 'output', '')
 
     assert isinstance(output, str)
     return (resultcode, output)
diff --git a/tools/efrotools/pcommandbatch.py b/tools/efrotools/pcommandbatch.py
index 1093a9dd..01abe87f 100644
--- a/tools/efrotools/pcommandbatch.py
+++ b/tools/efrotools/pcommandbatch.py
@@ -333,14 +333,7 @@ class Server:
                         output,
                     ) = await asyncio.get_running_loop().run_in_executor(
                         None,
-                        lambda: run_client_pcommand(
-                            argv,
-                            # Show log file path relative to project.
-                            self._worker_log_file_path.removeprefix(
-                                f'{self._project_dir}/'
-                            ),
-                            isatty,
-                        ),
+                        lambda: run_client_pcommand(argv, isatty),
                     )
                     if VERBOSE:
                         print(
@@ -359,7 +352,15 @@ class Server:
                         file=sys.stderr,
                     )
                 resultcode = 1
-                output = ''
+                logpath = self._worker_log_file_path.removeprefix(
+                    f'{self._project_dir}/'
+                )
+                output = (
+                    f'Error: pcommandbatch command failed: {argv}.'
+                    f" For more info, see '{logpath}', or rerun with"
+                    ' env var BA_PCOMMANDBATCH_DISABLE=1 to see'
+                    ' all output here.\n'
+                )
 
             writer.write(json.dumps({'r': resultcode, 'o': output}).encode())
             writer.close()