refactor: error handling to use exceptions (#1719)

* Refactor error handling to use exceptions cibuildwheel has up until now handled most errors by printing an error message to sys.stderr and calling sys.exit. Others were handled using Logger.error, depending on the context. We also had return codes, but these weren't explicitly defined anywhere. This makes that convention more explicit and codified. Now to halt the program, the correct thing to do is to throw a cibuildwheel.errors.FatalError exception - that is caught in main() and printed before exiting. The existing behaviour was kept - if an error occurs within a build step (probably something to do with the build itself), the Logger.error() method is used. Outside of a build step (e.g. a misconfiguration), the behaviour is still to print 'cibuildwheel: <message>' I also took the opportunity to add a debugging option `--debug-traceback` (and `CIBW_DEBUG_TRACEBACK`), which you can enable to see a full traceback on errors. (I've deactivated the flake8-errmsg lint rule, as it was throwing loads of errors and these error messages aren't generally seen in a traceback context) * add noqa rule * Apply suggestions from code review Co-authored-by: Henry Schreiner <[email protected]> * Return to flake8-errmsg conformance * Code review suggestions * Subclass Exception rather than SystemExit * apply error handling to new code and fix merge issues * Apply review suggestion * fix: merge issue * Update cibuildwheel/errors.py --------- Co-authored-by: Henry Schreiner <[email protected]> Co-authored-by: mayeut <[email protected]>
pypa · Jun 10, 2024 · bf817c6 · bf817c6
1 parent 384c8d5
commit bf817c6
Show file tree

Hide file tree

Showing 12 changed files with 207 additions and 142 deletions.
diff --git a/cibuildwheel/__main__.py b/cibuildwheel/__main__.py
@@ -1,11 +1,13 @@
 from __future__ import annotations
 
 import argparse
+import dataclasses
 import os
 import shutil
 import sys
 import tarfile
 import textwrap
+import traceback
 import typing
 from collections.abc import Iterable, Sequence, Set
 from pathlib import Path
@@ -18,6 +20,7 @@
 import cibuildwheel.pyodide
 import cibuildwheel.util
 import cibuildwheel.windows
+from cibuildwheel import errors
 from cibuildwheel._compat.typing import assert_never
 from cibuildwheel.architecture import Architecture, allowed_architectures_check
 from cibuildwheel.logger import log
@@ -31,10 +34,38 @@
     chdir,
     detect_ci_provider,
     fix_ansi_codes_for_github_actions,
+    strtobool,
 )
 
 
+@dataclasses.dataclass
+class GlobalOptions:
+    print_traceback_on_error: bool = True  # decides what happens when errors are hit.
+
+
 def main() -> None:
+    global_options = GlobalOptions()
+    try:
+        main_inner(global_options)
+    except errors.FatalError as e:
+        message = e.args[0]
+        if log.step_active:
+            log.step_end_with_error(message)
+        else:
+            print(f"cibuildwheel: {message}", file=sys.stderr)
+
+        if global_options.print_traceback_on_error:
+            traceback.print_exc(file=sys.stderr)
+
+        sys.exit(e.return_code)
+
+
+def main_inner(global_options: GlobalOptions) -> None:
+    """
+    `main_inner` is the same as `main`, but it raises FatalError exceptions
+    rather than exiting directly.
+    """
+
     parser = argparse.ArgumentParser(
         description="Build wheels for all the platforms.",
         epilog="""
@@ -132,8 +163,17 @@ def main() -> None:
         help="Enable pre-release Python versions if available.",
     )
 
+    parser.add_argument(
+        "--debug-traceback",
+        action="store_true",
+        default=strtobool(os.environ.get("CIBW_DEBUG_TRACEBACK", "0")),
+        help="Print a full traceback for all errors",
+    )
+
     args = CommandLineArguments(**vars(parser.parse_args()))
 
+    global_options.print_traceback_on_error = args.debug_traceback
+
     args.package_dir = args.package_dir.resolve()
 
     # This are always relative to the base directory, even in SDist builds
@@ -179,11 +219,8 @@ def _compute_platform_only(only: str) -> PlatformName:
         return "windows"
     if "pyodide_" in only:
         return "pyodide"
-    print(
-        f"Invalid --only='{only}', must be a build selector with a known platform",
-        file=sys.stderr,
-    )
-    sys.exit(2)
+    msg = f"Invalid --only='{only}', must be a build selector with a known platform"
+    raise errors.ConfigurationError(msg)
 
 
 def _compute_platform_auto() -> PlatformName:
@@ -194,34 +231,27 @@ def _compute_platform_auto() -> PlatformName:
     elif sys.platform == "win32":
         return "windows"
     else:
-        print(
+        msg = (
             'cibuildwheel: Unable to detect platform from "sys.platform". cibuildwheel doesn\'t '
             "support building wheels for this platform. You might be able to build for a different "
-            "platform using the --platform argument. Check --help output for more information.",
-            file=sys.stderr,
+            "platform using the --platform argument. Check --help output for more information."
         )
-        sys.exit(2)
+        raise errors.ConfigurationError(msg)
 
 
 def _compute_platform(args: CommandLineArguments) -> PlatformName:
     platform_option_value = args.platform or os.environ.get("CIBW_PLATFORM", "auto")
 
     if args.only and args.platform is not None:
-        print(
-            "--platform cannot be specified with --only, it is computed from --only",
-            file=sys.stderr,
-        )
-        sys.exit(2)
+        msg = "--platform cannot be specified with --only, it is computed from --only"
+        raise errors.ConfigurationError(msg)
     if args.only and args.archs is not None:
-        print(
-            "--arch cannot be specified with --only, it is computed from --only",
-            file=sys.stderr,
-        )
-        sys.exit(2)
+        msg = "--arch cannot be specified with --only, it is computed from --only"
+        raise errors.ConfigurationError(msg)
 
     if platform_option_value not in PLATFORMS | {"auto"}:
-        print(f"cibuildwheel: Unsupported platform: {platform_option_value}", file=sys.stderr)
-        sys.exit(2)
+        msg = f"Unsupported platform: {platform_option_value}"
+        raise errors.ConfigurationError(msg)
 
     if args.only:
         return _compute_platform_only(args.only)
@@ -268,9 +298,8 @@ def build_in_directory(args: CommandLineArguments) -> None:
 
     if not any(package_dir.joinpath(name).exists() for name in package_files):
         names = ", ".join(sorted(package_files, reverse=True))
-        msg = f"cibuildwheel: Could not find any of {{{names}}} at root of package"
-        print(msg, file=sys.stderr)
-        sys.exit(2)
+        msg = f"Could not find any of {{{names}}} at root of package"
+        raise errors.ConfigurationError(msg)
 
     platform_module = get_platform_module(platform)
     identifiers = get_build_identifiers(
@@ -301,16 +330,14 @@ def build_in_directory(args: CommandLineArguments) -> None:
         options.check_for_invalid_configuration(identifiers)
         allowed_architectures_check(platform, options.globals.architectures)
     except ValueError as err:
-        print("cibuildwheel:", *err.args, file=sys.stderr)
-        sys.exit(4)
+        raise errors.DeprecationError(*err.args) from err
 
     if not identifiers:
-        print(
-            f"cibuildwheel: No build identifiers selected: {options.globals.build_selector}",
-            file=sys.stderr,
-        )
-        if not args.allow_empty:
-            sys.exit(3)
+        message = f"No build identifiers selected: {options.globals.build_selector}"
+        if args.allow_empty:
+            print(f"cibuildwheel: {message}", file=sys.stderr)
+        else:
+            raise errors.NothingToDoError(message)
 
     output_dir = options.globals.output_dir
 
@@ -365,7 +392,9 @@ def print_preamble(platform: str, options: Options, identifiers: Sequence[str])
 
 
 def get_build_identifiers(
-    platform_module: PlatformModule, build_selector: BuildSelector, architectures: Set[Architecture]
+    platform_module: PlatformModule,
+    build_selector: BuildSelector,
+    architectures: Set[Architecture],
 ) -> list[str]:
     python_configurations = platform_module.get_python_configurations(build_selector, architectures)
     return [config.identifier for config in python_configurations]

diff --git a/cibuildwheel/errors.py b/cibuildwheel/errors.py
@@ -0,0 +1,27 @@
+"""
+Errors that can cause the build to fail. Each subclass of FatalError has
+a different return code, by defining them all here, we can ensure that they're
+semantically clear and unique.
+"""
+
+
+class FatalError(BaseException):
+    """
+    Raising an error of this type will cause the message to be printed to
+    stderr and the process to be terminated. Within cibuildwheel, raising this
+    exception produces a better error message, and optional traceback.
+    """
+
+    return_code: int = 1
+
+
+class ConfigurationError(FatalError):
+    return_code = 2
+
+
+class NothingToDoError(FatalError):
+    return_code = 3
+
+
+class DeprecationError(FatalError):
+    return_code = 4
diff --git a/cibuildwheel/linux.py b/cibuildwheel/linux.py
@@ -10,6 +10,7 @@
 
 from packaging.version import Version
 
+from . import errors
 from ._compat.typing import assert_never
 from .architecture import Architecture
 from .logger import log
@@ -147,11 +148,7 @@ def check_all_python_exist(
             exist = False
     if not exist:
         message = "\n".join(messages)
-        print(
-            f"cibuildwheel:\n{message}",
-            file=sys.stderr,
-        )
-        sys.exit(1)
+        raise errors.FatalError(message)
 
 
 def build_in_container(
@@ -225,28 +222,19 @@ def build_in_container(
         # check config python is still on PATH
         which_python = container.call(["which", "python"], env=env, capture_output=True).strip()
         if PurePosixPath(which_python) != python_bin / "python":
-            print(
-                "cibuildwheel: python available on PATH doesn't match our installed instance. If you have modified PATH, ensure that you don't overwrite cibuildwheel's entry or insert python above it.",
-                file=sys.stderr,
-            )
-            sys.exit(1)
+            msg = "python available on PATH doesn't match our installed instance. If you have modified PATH, ensure that you don't overwrite cibuildwheel's entry or insert python above it."
+            raise errors.FatalError(msg)
 
         if use_uv:
             which_uv = container.call(["which", "uv"], env=env, capture_output=True).strip()
             if not which_uv:
-                print(
-                    "cibuildwheel: uv not found on PATH. You must use a supported manylinux or musllinux environment with uv.",
-                    file=sys.stderr,
-                )
-                sys.exit(1)
+                msg = "uv not found on PATH. You must use a supported manylinux or musllinux environment with uv."
+                raise errors.FatalError(msg)
         else:
             which_pip = container.call(["which", "pip"], env=env, capture_output=True).strip()
             if PurePosixPath(which_pip) != python_bin / "pip":
-                print(
-                    "cibuildwheel: pip available on PATH doesn't match our installed instance. If you have modified PATH, ensure that you don't overwrite cibuildwheel's entry or insert pip above it.",
-                    file=sys.stderr,
-                )
-                sys.exit(1)
+                msg = "pip available on PATH doesn't match our installed instance. If you have modified PATH, ensure that you don't overwrite cibuildwheel's entry or insert pip above it."
+                raise errors.FatalError(msg)
 
         compatible_wheel = find_compatible_wheel(built_wheels, config.identifier)
         if compatible_wheel:
@@ -446,21 +434,18 @@ def build(options: Options, tmp_path: Path) -> None:  # noqa: ARG001
                 check=True,
                 stdout=subprocess.DEVNULL,
             )
-        except subprocess.CalledProcessError:
-            print(
-                unwrap(
-                    f"""
-                    cibuildwheel: {build_step.container_engine.name} not found. An
-                    OCI exe like Docker or Podman is required to run Linux builds.
-                    If you're building on Travis CI, add `services: [docker]` to
-                    your .travis.yml. If you're building on Circle CI in Linux,
-                    add a `setup_remote_docker` step to your .circleci/config.yml.
-                    If you're building on Cirrus CI, use `docker_builder` task.
-                    """
-                ),
-                file=sys.stderr,
+        except subprocess.CalledProcessError as error:
+            msg = unwrap(
+                f"""
+                cibuildwheel: {build_step.container_engine.name} not found. An
+                OCI exe like Docker or Podman is required to run Linux builds.
+                If you're building on Travis CI, add `services: [docker]` to
+                your .travis.yml. If you're building on Circle CI in Linux,
+                add a `setup_remote_docker` step to your .circleci/config.yml.
+                If you're building on Cirrus CI, use `docker_builder` task.
+                """
             )
-            sys.exit(2)
+            raise errors.ConfigurationError(msg) from error
 
         try:
             ids_to_build = [x.identifier for x in build_step.platform_configs]
@@ -483,11 +468,9 @@ def build(options: Options, tmp_path: Path) -> None:  # noqa: ARG001
                 )
 
         except subprocess.CalledProcessError as error:
-            log.step_end_with_error(
-                f"Command {error.cmd} failed with code {error.returncode}. {error.stdout}"
-            )
             troubleshoot(options, error)
-            sys.exit(1)
+            msg = f"Command {error.cmd} failed with code {error.returncode}. {error.stdout or ''}"
+            raise errors.FatalError(msg) from error
 
 
 def _matches_prepared_command(error_cmd: Sequence[str], command_template: str) -> bool:
@@ -506,7 +489,6 @@ def troubleshoot(options: Options, error: Exception) -> None:
         )  # TODO allow matching of overrides too?
     ):
         # the wheel build step or the repair step failed
-        print("Checking for common errors...")
         so_files = list(options.globals.package_dir.glob("**/*.so"))
 
         if so_files:

diff --git a/cibuildwheel/logger.py b/cibuildwheel/logger.py
@@ -152,6 +152,10 @@ def error(self, error: BaseException | str) -> None:
             c = self.colors
             print(f"{c.bright_red}Error{c.end}: {error}\n", file=sys.stderr)
 
+    @property
+    def step_active(self) -> bool:
+        return self.step_start_time is not None
+
     def _start_fold_group(self, name: str) -> None:
         self._end_fold_group()
         self.active_fold_group_name = name

diff --git a/cibuildwheel/macos.py b/cibuildwheel/macos.py
@@ -16,6 +16,7 @@
 from filelock import FileLock
 from packaging.version import Version
 
+from . import errors
 from ._compat.typing import assert_never
 from .architecture import Architecture
 from .environment import ParsedEnvironment
@@ -150,14 +151,13 @@ def install_cpython(tmp: Path, version: str, url: str, free_threading: bool) ->
             if detect_ci_provider() is None:
                 # if running locally, we don't want to install CPython with sudo
                 # let the user know & provide a link to the installer
-                print(
+                msg = (
                     f"Error: CPython {version} is not installed.\n"
                     "cibuildwheel will not perform system-wide installs when running outside of CI.\n"
                     f"To build locally, install CPython {version} on this machine, or, disable this version of Python using CIBW_SKIP=cp{version.replace('.', '')}-macosx_*\n"
-                    f"\nDownload link: {url}",
-                    file=sys.stderr,
+                    f"\nDownload link: {url}"
                 )
-                raise SystemExit(1)
+                raise errors.FatalError(msg)
             pkg_path = tmp / "Python.pkg"
             # download the pkg
             download(url, pkg_path)
@@ -279,22 +279,16 @@ def setup_python(
         call("pip", "--version", env=env)
         which_pip = call("which", "pip", env=env, capture_stdout=True).strip()
         if which_pip != str(venv_bin_path / "pip"):
-            print(
-                "cibuildwheel: pip available on PATH doesn't match our installed instance. If you have modified PATH, ensure that you don't overwrite cibuildwheel's entry or insert pip above it.",
-                file=sys.stderr,
-            )
-            sys.exit(1)
+            msg = "cibuildwheel: pip available on PATH doesn't match our installed instance. If you have modified PATH, ensure that you don't overwrite cibuildwheel's entry or insert pip above it."
+            raise errors.FatalError(msg)
 
     # check what Python version we're on
     call("which", "python", env=env)
     call("python", "--version", env=env)
     which_python = call("which", "python", env=env, capture_stdout=True).strip()
     if which_python != str(venv_bin_path / "python"):
-        print(
-            "cibuildwheel: python available on PATH doesn't match our installed instance. If you have modified PATH, ensure that you don't overwrite cibuildwheel's entry or insert python above it.",
-            file=sys.stderr,
-        )
-        sys.exit(1)
+        msg = "cibuildwheel: python available on PATH doesn't match our installed instance. If you have modified PATH, ensure that you don't overwrite cibuildwheel's entry or insert python above it."
+        raise errors.FatalError(msg)
 
     config_is_arm64 = python_configuration.identifier.endswith("arm64")
     config_is_universal2 = python_configuration.identifier.endswith("universal2")
@@ -756,7 +750,5 @@ def build(options: Options, tmp_path: Path) -> None:
 
             log.build_end()
     except subprocess.CalledProcessError as error:
-        log.step_end_with_error(
-            f"Command {error.cmd} failed with code {error.returncode}. {error.stdout}"
-        )
-        sys.exit(1)
+        msg = f"Command {error.cmd} failed with code {error.returncode}. {error.stdout or ''}"
+        raise errors.FatalError(msg) from error