feat: split stderr for better error presentation

craft_parts/errors.py

@@ -16,16 +16,18 @@
 
 """Craft parts errors."""
 
-import dataclasses
+import contextlib
 import pathlib
 from collections.abc import Iterable
+from io import StringIO
 from typing import TYPE_CHECKING
 
+from overrides import override
+
 if TYPE_CHECKING:
     from pydantic.error_wrappers import ErrorDict, Loc
 
 
-@dataclasses.dataclass(repr=True)
 class PartsError(Exception):
     """Unexpected error.
 
@@ -38,10 +40,21 @@ class PartsError(Exception):
     """
 
     brief: str
-    details: str | None = None
     resolution: str | None = None
     doc_slug: str | None = None
 
+    def __init__(
+        self,
+        brief: str,
+        details: str | None = None,
+        resolution: str | None = None,
+        doc_slug: str | None = None,
+    ) -> None:
+        self.brief = brief
+        self._details = details
+        self.resolution = resolution
+        self.doc_slug = doc_slug
+
     def __str__(self) -> str:
         components = [self.brief]
 
@@ -53,6 +66,14 @@ def __str__(self) -> str:
 
         return "\n".join(components)
 
+    def __repr__(self) -> str:
+        return f"{self.__class__.__name__}(brief={self.brief!r}, details={self.details!r}, resolution={self.resolution!r}, doc_slug={self.doc_slug!r})"
+
+    @property
+    def details(self) -> str | None:
+        """Further details on the error."""
+        return self._details
+
 
 class FeatureError(PartsError):
     """A feature is not configured as expected."""
@@ -482,15 +503,49 @@ class PluginBuildError(PartsError):
     :param plugin_name: The name of the plugin being processed.
     """
 
-    def __init__(self, *, part_name: str, plugin_name: str) -> None:
+    def __init__(
+        self, *, part_name: str, plugin_name: str, stderr: bytes | None = None
+    ) -> None:
         self.part_name = part_name
         self.plugin_name = plugin_name
+        self.stderr = stderr
         brief = f"Failed to run the build script for part {part_name!r}."
         resolution = f"Check the build output and verify the project can work with the {plugin_name!r} plugin."
         super().__init__(
             brief=brief, resolution=resolution, doc_slug="/reference/plugins.html"
         )
 
+    @property
+    @override
+    def details(self) -> str | None:
+        """Further details on the error.
+
+        Discards all trace lines that come before the last-executed script line
+        """
+        with contextlib.closing(StringIO()) as details_io:
+            if self.stderr is None:
+                return None
+
+            stderr = self.stderr.decode("utf-8", errors="replace")
+            details_io.write("\nCaptured standard error:")
+
+            stderr_lines = stderr.split("\n")
+            # Find the final command captured in the logs
+            last_command = None
+            for idx, line in enumerate(reversed(stderr_lines)):
+                if line.startswith("+"):
+                    last_command = len(stderr_lines) - idx - 1
+                    break
+            else:
+                # Fallback to printing the whole log
+                last_command = 0
+
+            for line in stderr_lines[last_command:]:
+                if line:
+                    details_io.write(f"\n:: {line}")
+
+            return details_io.getvalue()
+
 
 class PluginCleanError(PartsError):
     """Script to clean strict build preparation failed at runtime.

craft_parts/executor/step_handler.py

@@ -34,7 +34,7 @@
 from craft_parts.plugins import Plugin
 from craft_parts.sources.local_source import SourceHandler
 from craft_parts.steps import Step
-from craft_parts.utils import file_utils
+from craft_parts.utils import file_utils, process
 
 from . import filesets
 from .filesets import Fileset
@@ -122,10 +122,8 @@ def _builtin_pull(self) -> StepContents:
                     stdout=self._stdout,
                     stderr=self._stderr,
                 )
-            except subprocess.CalledProcessError as process_error:
-                raise errors.PluginPullError(
-                    part_name=self._part.name
-                ) from process_error
+            except process.ProcessError:
+                raise errors.PluginPullError(part_name=self._part.name)
 
         return StepContents()
 
@@ -153,9 +151,11 @@ def _builtin_build(self) -> StepContents:
                 stdout=self._stdout,
                 stderr=self._stderr,
             )
-        except subprocess.CalledProcessError as process_error:
+        except process.ProcessError as process_error:
             raise errors.PluginBuildError(
-                part_name=self._part.name, plugin_name=self._part.plugin_name
+                part_name=self._part.name,
+                plugin_name=self._part.plugin_name,
+                stderr=process_error.result.stderr,
             ) from process_error
 
         return StepContents()
@@ -458,10 +458,4 @@ def _create_and_run_script(
     script_path.chmod(0o755)
     logger.debug("Executing %r", script_path)
 
-    subprocess.run(
-        [script_path],
-        cwd=cwd,
-        check=True,
-        stdout=stdout,
-        stderr=stderr,
-    )
+    process.run([script_path], cwd=cwd, stdout=stdout, stderr=stderr, check=True)

craft_parts/utils/process.py

@@ -0,0 +1,201 @@
+# -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*-
+#
+# Copyright 2024 Canonical Ltd.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License version 3 as published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Utilities for executing subprocesses and handling their stdout and stderr streams."""
+
+import os
+import select
+import subprocess
+import sys
+from collections.abc import Generator, Sequence
+from contextlib import closing, contextmanager
+from dataclasses import dataclass
+from io import BytesIO
+from pathlib import Path
+from typing import IO, TextIO, cast
+
+Command = str | Path | Sequence[str | Path]
+Stream = TextIO | int | None
+
+_BUF_SIZE = 4096
+
+# Compatibility with subprocess.DEVNULL
+DEVNULL = subprocess.DEVNULL
+
+
+@dataclass
+class ProcessResult:
+    """Describes the outcome of a process."""
+
+    returncode: int
+    stdout: bytes
+    stderr: bytes
+    combined: bytes
+    command: Command
+
+    def check_returncode(self) -> None:
+        """Raise an exception if the process returned non-zero."""
+        if self.returncode != 0:
+            raise ProcessError(self)
+
+
+class _ProcessStream:
+    def __init__(self, read_fd: int, write_fd: int) -> None:
+        self.read_fd = read_fd
+        self.write_fd = write_fd
+
+        self._linebuf = bytearray()
+        self._streambuf = b""
+
+    @property
+    def singular(self) -> bytes:
+        return self._streambuf
+
+    def process(self) -> bytes:
+        """Forward any data from ``self.read_fd`` to ``self.write_fd`` and return a copy of it."""
+        data = os.read(self.read_fd, _BUF_SIZE)
+        i = data.rfind(b"\n")
+        if i >= 0:
+            self._linebuf.extend(data[: i + 1])
+            self._streambuf += self._linebuf
+            os.write(self.write_fd, self._linebuf)
+            self._linebuf.clear()
+            self._linebuf.extend(data[i + 1 :])
+            return data
+        self._linebuf.extend(data)
+        return b""
+
+
+def run(
+    command: Command,
+    *,
+    cwd: Path | None = None,
+    stdout: Stream = None,
+    stderr: Stream = None,
+    check: bool = True,
+) -> ProcessResult:
+    """Execute a subprocess and collect its output.
+
+    This function collects the stdout and stderr streams as separate
+    accounts and a singular, combined account.
+
+    :param command: Command to execute.
+    :type Command:
+    :param cwd: Path to execute in.
+    :type Path | None:
+    :param stdout: Handle to a fd or I/O stream to treat as stdout. None defaults
+        to ``sys.stdout``, and process.DEVNULL can be passed for no printing.
+    :type Stream:
+    :param stderr: Handle to a fd or I/O stream to treat as stderr. None defaults
+        to ``sys.stderr``, and process.DEVNULL can be passed for no printing.
+    :type Stream:
+    :param check: If True, a ProcessError exception will be raised if ``command``
+        returns a non-zero return code.
+    :type bool:
+
+    :raises ProcessError: If process exits with a non-zero return code.
+    :raises OSError: If the specified executable is not found.
+
+    :return: A description of the process' outcome.
+    :rtype: ProcessResult
+    """
+    proc = subprocess.Popen(
+        command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, cwd=cwd
+    )
+
+    with (
+        _select_stream(stdout, sys.stdout.fileno()) as out_fd,
+        _select_stream(stderr, sys.stderr.fileno()) as err_fd,
+    ):
+        # stdout and stderr are guaranteed not `None` because we called with `subprocess.PIPE`
+        proc_stdout = cast(IO[bytes], proc.stdout).fileno()
+        proc_stderr = cast(IO[bytes], proc.stderr).fileno()
+
+        os.set_blocking(proc_stdout, False)
+        os.set_blocking(proc_stderr, False)
+
+        out_handler = _ProcessStream(proc_stdout, out_fd)
+        err_handler = _ProcessStream(proc_stderr, err_fd)
+
+        with closing(BytesIO()) as combined_io:
+            while True:
+                r, _, _ = select.select([proc_stdout, proc_stderr], [], [])
+
+                try:
+                    if proc_stdout in r:
+                        combined_io.write(out_handler.process())
+
+                    if proc_stderr in r:
+                        combined_io.write(err_handler.process())
+
+                except BlockingIOError:
+                    pass
+
+                if proc.poll() is not None:
+                    combined = combined_io.getvalue()
+                    break
+
+    result = ProcessResult(
+        proc.returncode,
+        out_handler.singular,
+        err_handler.singular,
+        combined,
+        command,
+    )
+
+    if check and result.returncode != 0:
+        raise ProcessError(result)
+
+    return result
+
+
+@contextmanager
+def _select_stream(stream: Stream, default: int) -> Generator[int]:
+    """Select and return an appropriate raw file descriptor.
+
+    Based on the input, this function returns a raw integer file descriptor according
+    to what is expected from the ``run()`` function in this same module.
+
+    If determining the file handle involves opening our own, this generator handles
+    closing it afterwards.
+    """
+    s: int
+    close = False
+    if stream == DEVNULL:
+        s = os.open(os.devnull, os.O_WRONLY)
+        close = True
+    elif isinstance(stream, int):
+        s = stream
+    elif stream is None:
+        s = default
+    else:
+        s = stream.fileno()
+
+    try:
+        yield s
+    finally:
+        if close:
+            os.close(s)
+
+
+@dataclass
+class ProcessError(Exception):
+    """Simple error for failed processes.
+
+    Generally raised if the return code of a process is non-zero.
+    """
+
+    result: ProcessResult