Improve error handling in run_command

spacy/util.py

@@ -659,8 +659,8 @@ def join_command(command: List[str]) -> str:
 def run_command(
     command: Union[str, List[str]],
     *,
-    capture: bool = False,
     stdin: Optional[Any] = None,
+    capture: bool=False,
 ) -> Optional[subprocess.CompletedProcess]:
     """Run a command on the command line as a subprocess. If the subprocess
     returns a non-zero exit code, a system exit is performed.
@@ -668,33 +668,46 @@ def run_command(
     command (str / List[str]): The command. If provided as a string, the
         string will be split using shlex.split.
     stdin (Optional[Any]): stdin to read from or None.
-    capture (bool): Whether to capture the output.
+    capture (bool): Whether to capture the output and errors. If False,
+        the stdout and stderr will not be redirected, and if there's an error,
+        sys.exit will be called with the returncode. You should use capture=False
+        when you want to turn over execution to the command, and capture=True
+        when you want to run the command more like a function.
     RETURNS (Optional[CompletedProcess]): The process object.
     """
     if isinstance(command, str):
-        command = split_command(command)
+        cmd_list = split_command(command)
+        cmd_str = command
+    else:
+        cmd_list = command
+        cmd_str = " ".join(command)
     try:
         ret = subprocess.run(
-            command,
+            cmd_list,
             env=os.environ.copy(),
             input=stdin,
             encoding="utf8",
-            check=True,
+            check=False,
             stdout=subprocess.PIPE if capture else None,
-            stderr=subprocess.PIPE if capture else None,
+            stderr=subprocess.STDOUT if capture else None,
         )
     except FileNotFoundError:
+        # Indicates the *command* wasn't found, it's an error before the command
+        # is run.
         raise FileNotFoundError(
-            Errors.E970.format(str_command=" ".join(command), tool=command[0])
+            Errors.E970.format(str_command=cmd_str, tool=cmd_list[0])
         ) from None
-    except subprocess.CalledProcessError as e:
+    if ret.returncode != 0 and capture:
-        # We don't want a duplicate traceback here so we're making sure the
+        message = f"Error running command:\n\n{cmd_str}\n\n"
-        # CalledProcessError isn't re-raised. We also print both the string
+        message += f"Subprocess exited with status {ret.returncode}"
-        # message and the stderr, in case the error only has one of them.
+        if ret.stdout is not None:
-        print(e.stderr)
+            message += f"\n\nProcess log (stdout and stderr):\n\n"
-        print(e)
+            message += ret.stdout
-        sys.exit(1)
+        error = subprocess.SubprocessError(message)
-    if ret.returncode != 0:
+        error.ret = ret
+        error.command = cmd_str
+        raise error
+    elif ret.returncode != 0:
         sys.exit(ret.returncode)
     return ret