Rework exception handling

Let's always use regular exceptions and stop outputting stack traces
on exceptions by default. To get a stacktrace, users can run mkosi with
--debug run and we will output the usual stacktrace.

diff --git a/mkosi/__init__.py b/mkosi/__init__.py
index f79facdb6730ffff6917e25e67310e88fe70650f..a764c3f334b3cbdddd84021cfd30aedcead822cb 100644 (file)
--- a/mkosi/__init__.py
+++ b/mkosi/__init__.py
@@ -42,14 +42,7 @@ from mkosi.backend import (
     tmp_dir,
 )
 from mkosi.install import add_dropin_config_from_resource, copy_path, flock
-from mkosi.log import (
-    ARG_DEBUG,
-    MkosiException,
-    MkosiNotSupportedException,
-    MkosiPrinter,
-    die,
-    warn,
-)
+from mkosi.log import ARG_DEBUG, MkosiPrinter, die, warn
 from mkosi.manifest import GenericVersion, Manifest
 from mkosi.mounts import dissect_and_mount, mount_overlay, scandir_recursive
 from mkosi.pager import page
@@ -1130,10 +1123,10 @@ def load_args(args: argparse.Namespace) -> MkosiConfig:
         OutputFormat.tar,
         OutputFormat.cpio,
     ):
-        die("Directory, subvolume, tar, cpio, and plain squashfs images cannot be booted in qemu.", MkosiNotSupportedException)
+        die("Directory, subvolume, tar, cpio, and plain squashfs images cannot be booted in qemu.")
 
     if shutil.which("bsdtar") and args.distribution == Distribution.openmandriva and args.tar_strip_selinux_context:
-        die("Sorry, bsdtar on OpenMandriva is incompatible with --tar-strip-selinux-context", MkosiNotSupportedException)
+        die("Sorry, bsdtar on OpenMandriva is incompatible with --tar-strip-selinux-context")
 
     if args.cache_dir:
         args.cache_dir = args.cache_dir / f"{args.distribution}~{args.release}"
@@ -1224,13 +1217,13 @@ def load_args(args: argparse.Namespace) -> MkosiConfig:
     if args.verb in (Verb.shell, Verb.boot):
         opname = "acquire shell" if args.verb == Verb.shell else "boot"
         if args.output_format in (OutputFormat.tar, OutputFormat.cpio):
-            die(f"Sorry, can't {opname} with a {args.output_format} archive.", MkosiNotSupportedException)
+            die(f"Sorry, can't {opname} with a {args.output_format} archive.")
         if should_compress_output(args):
-            die(f"Sorry, can't {opname} with a compressed image.", MkosiNotSupportedException)
+            die(f"Sorry, can't {opname} with a compressed image.")
 
     if args.verb == Verb.qemu:
         if not args.output_format == OutputFormat.disk:
-            die("Sorry, can't boot non-disk images with qemu.", MkosiNotSupportedException)
+            die("Sorry, can't boot non-disk images with qemu.")
 
     if args.repo_dirs and not (is_dnf_distribution(args.distribution) or args.distribution == Distribution.arch):
         die("--repo-dir is only supported on DNF based distributions and Arch")
@@ -1930,15 +1923,6 @@ def check_root() -> None:
         die("Must be invoked as root.")
 
 
-@contextlib.contextmanager
-def suppress_stacktrace() -> Iterator[None]:
-    try:
-        yield
-    except subprocess.CalledProcessError as e:
-        # MkosiException is silenced in main() so it doesn't print a stacktrace.
-        raise MkosiException() from e
-
-
 def machine_name(config: Union[MkosiConfig, argparse.Namespace]) -> str:
     return config.image_id or config.output.with_suffix("").name.partition("_")[0]
 
@@ -2480,15 +2464,14 @@ def run_verb(config: MkosiConfig) -> None:
             if config.auto_bump:
                 bump_image_version(config)
 
-        with suppress_stacktrace():
-            if config.verb in (Verb.shell, Verb.boot):
-                run_shell(config)
+        if config.verb in (Verb.shell, Verb.boot):
+            run_shell(config)
 
-            if config.verb == Verb.qemu:
-                run_qemu(config)
+        if config.verb == Verb.qemu:
+            run_qemu(config)
 
-            if config.verb == Verb.ssh:
-                run_ssh(config)
+        if config.verb == Verb.ssh:
+            run_ssh(config)
 
         if config.verb == Verb.serve:
             run_serve(config)

diff --git a/mkosi/__main__.py b/mkosi/__main__.py
index 52dddac6714a3f57bdd053c6dc67b7452648a36a..8ca7d53a848c758693d8511f4c9399dc310efbfc 100644 (file)
--- a/mkosi/__main__.py
+++ b/mkosi/__main__.py
@@ -3,13 +3,13 @@
 
 import contextlib
 import os
+import subprocess
 import sys
 from collections.abc import Iterator
-from subprocess import CalledProcessError
 
 from mkosi import load_args, run_verb
 from mkosi.config import MkosiConfigParser
-from mkosi.log import MkosiException, die
+from mkosi.log import ARG_DEBUG, MkosiPrinter, die
 from mkosi.run import excepthook
 
 
@@ -17,10 +17,28 @@ from mkosi.run import excepthook
 def propagate_failed_return() -> Iterator[None]:
     try:
         yield
-    except MkosiException as e:
-        cause = e.__cause__
-        if cause and isinstance(cause, CalledProcessError):
-            sys.exit(cause.returncode)
+    except SystemExit as e:
+        if ARG_DEBUG:
+            raise e
+
+        sys.exit(e.code)
+    except KeyboardInterrupt as e:
+        if ARG_DEBUG:
+            raise e
+
+        MkosiPrinter.error("Interrupted")
+        sys.exit(1)
+    except subprocess.CalledProcessError as e:
+        if ARG_DEBUG:
+            raise e
+
+        # We always log when subprocess.CalledProcessError is raised, so we don't log again here.
+        sys.exit(e.returncode)
+    except Exception as e:
+        if ARG_DEBUG:
+            raise e
+
+        MkosiPrinter.error("Error: mkosi failed because of an exception, rerun mkosi with --debug run to get more information")
         sys.exit(1)
 
 

diff --git a/mkosi/backend.py b/mkosi/backend.py
index 8f5c1adb4ce0f960d1fb5355a48bc265f21bf32a..ad3c7a1c852a7f8149b41c2b4e35d87211505641 100644 (file)
--- a/mkosi/backend.py
+++ b/mkosi/backend.py
@@ -21,7 +21,7 @@ from pathlib import Path
 from typing import Any, Callable, Optional, TypeVar, Union
 
 from mkosi.distributions import DistributionInstaller
-from mkosi.log import MkosiException, die
+from mkosi.log import die
 
 T = TypeVar("T")
 V = TypeVar("V")
@@ -428,7 +428,7 @@ def patch_file(filepath: Path, line_rewriter: Callable[[str], str]) -> None:
 def safe_tar_extract(tar: tarfile.TarFile, path: Path=Path("."), *, numeric_owner: bool=False) -> None:
     """Extract a tar without CVE-2007-4559.
 
-    Throws a MkosiException if a member of the tar resolves to a path that would
+    Throws an exception if a member of the tar resolves to a path that would
     be outside of the passed in target path.
 
     Omits the member argument from TarFile.extractall, since we don't need it at
@@ -446,7 +446,7 @@ def safe_tar_extract(tar: tarfile.TarFile, path: Path=Path("."), *, numeric_owne
                 target.resolve().relative_to(path)
                 members += [member]
         except ValueError as e:
-            raise MkosiException(f"Attempted path traversal in tar file {tar.name!r}") from e
+            die(f"Attempted path traversal in tar file {tar.name!r}", e)
 
     tar.extractall(path, members=members, numeric_owner=numeric_owner)
 

diff --git a/mkosi/distributions/gentoo.py b/mkosi/distributions/gentoo.py
index 3a389b536d69a5c512db735865080f6586ea6517..30a699c150b1dc9dc3a3304a3f16aeb0919510e5 100644 (file)
--- a/mkosi/distributions/gentoo.py
+++ b/mkosi/distributions/gentoo.py
@@ -12,7 +12,7 @@ from textwrap import dedent
 from mkosi.backend import MkosiState, safe_tar_extract
 from mkosi.distributions import DistributionInstaller
 from mkosi.install import copy_path, flock
-from mkosi.log import ARG_DEBUG, MkosiException, MkosiPrinter, complete_step, die
+from mkosi.log import ARG_DEBUG, MkosiPrinter, complete_step, die
 from mkosi.remove import unlink_try_hard
 from mkosi.run import run_workspace_command
 
@@ -139,7 +139,7 @@ class Gentoo:
         except ImportError as e:
             MkosiPrinter.warn(NEED_PORTAGE_MSG)
             MkosiPrinter.info(PORTAGE_INSTALL_INSTRUCTIONS)
-            raise MkosiException(e)
+            raise e
 
         return dict(profile_path=PROFILE_PATH,
                     custom_profile_path=CUSTOM_PROFILE_PATH,

diff --git a/mkosi/log.py b/mkosi/log.py
index 9101976e6627448d5e21dc4bc1de322d905c32d6..9db56f77e63946734c2e0987d0e6ea3e4a398527 100644 (file)
--- a/mkosi/log.py
+++ b/mkosi/log.py
@@ -6,17 +6,9 @@ from typing import Any, Iterator, NoReturn, Optional
 ARG_DEBUG: set[str] = set()
 
 
-class MkosiException(Exception):
-    """Leads to sys.exit"""
-
-
-class MkosiNotSupportedException(MkosiException):
-    """Leads to sys.exit when an invalid combination of parsed arguments happens"""
-
-
-def die(message: str, exception: type[MkosiException] = MkosiException) -> NoReturn:
+def die(message: str, exception: Optional[Exception] = None) -> NoReturn:
     MkosiPrinter.error(f"Error: {message}")
-    raise exception(message)
+    raise exception or RuntimeError(message)
 
 
 def warn(message: str) -> None:

diff --git a/mkosi/run.py b/mkosi/run.py
index 6fcb9dc5ece56ab2fb334180b5c5969e8be4cccf..c00ffa1519cc67987edcc62218a1880ba1d504f4 100644 (file)
--- a/mkosi/run.py
+++ b/mkosi/run.py
@@ -187,8 +187,7 @@ def fork_and_wait(target: Callable[[], T]) -> T:
 
     return result
 
-
-def run(
+def _run(
     cmdline: Sequence[PathString],
     check: bool = True,
     stdout: _FILE = None,
@@ -222,8 +221,22 @@ def run(
     try:
         return subprocess.run(cmdline, check=check, stdout=stdout, stderr=stderr, env=env, **kwargs,
                               preexec_fn=foreground, close_fds=False)
-    except FileNotFoundError:
-        die(f"{cmdline[0]} not found in PATH.")
+    except FileNotFoundError as e:
+        die(f"{cmdline[0]} not found in PATH.", e)
+
+
+def run(
+    cmdline: Sequence[PathString],
+    check: bool = True,
+    stdout: _FILE = None,
+    stderr: _FILE = None,
+    env: Mapping[str, PathString] = {},
+    **kwargs: Any,
+) -> CompletedProcess:
+    try:
+        return _run(cmdline, check, stdout, stderr, env, **kwargs)
+    except subprocess.CalledProcessError as e:
+        die(f"\"{shlex.join(str(s) for s in cmdline)}\" returned non-zero exit code {e.returncode}.", e)
 
 
 def spawn(
@@ -245,6 +258,8 @@ def spawn(
         return subprocess.Popen(cmdline, stdout=stdout, stderr=stderr, **kwargs, preexec_fn=foreground)
     except FileNotFoundError:
         die(f"{cmdline[0]} not found in PATH.")
+    except subprocess.CalledProcessError as e:
+        die(f"\"{shlex.join(str(s) for s in cmdline)}\" returned non-zero exit code {e.returncode}.", e)
 
 
 def run_with_apivfs(
@@ -290,13 +305,12 @@ def run_with_apivfs(
     template = f"chmod 1777 {state.root / 'tmp'} {state.root / 'var/tmp'} {state.root / 'dev/shm'} && exec {{}} || exit $?"
 
     try:
-        return run([*cmdline, template.format(shlex.join(str(s) for s in cmd))],
+        return _run([*cmdline, template.format(shlex.join(str(s) for s in cmd))],
                    text=True, stdout=stdout, stderr=stderr, env=env)
     except subprocess.CalledProcessError as e:
         if "run" in ARG_DEBUG:
-            run([*cmdline, template.format("sh")], check=False, env=env)
-        die(f"\"{shlex.join(str(s) for s in cmd)}\" returned non-zero exit code {e.returncode}.")
-
+            _run([*cmdline, template.format("sh")], check=False, env=env)
+        raise e
 
 def run_workspace_command(
     state: MkosiState,
@@ -348,12 +362,12 @@ def run_workspace_command(
     template = "chmod 1777 /tmp /var/tmp /dev/shm && PATH=$PATH:/usr/bin:/usr/sbin exec {} || exit $?"
 
     try:
-        return run([*cmdline, template.format(shlex.join(str(s) for s in cmd))],
+        return _run([*cmdline, template.format(shlex.join(str(s) for s in cmd))],
                    text=True, stdout=stdout, env=env)
     except subprocess.CalledProcessError as e:
         if "run" in ARG_DEBUG:
-            run([*cmdline, template.format("sh")], check=False, env=env)
-        die(f"\"{shlex.join(str(s) for s in cmd)}\" returned non-zero exit code {e.returncode}.")
+            _run([*cmdline, template.format("sh")], check=False, env=env)
+        raise e
     finally:
         if state.workspace.joinpath("resolv.conf").is_symlink():
             resolve.unlink(missing_ok=True)

diff --git a/tests/test_backend.py b/tests/test_backend.py
index be8b966bd279b305fefb124aa4758b8451074f0b..70108ed6d58695fdc43fd30d3b18e27857218032 100644 (file)
--- a/tests/test_backend.py
+++ b/tests/test_backend.py
@@ -14,8 +14,6 @@ from mkosi.backend import (
     set_umask,
     strip_suffixes,
 )
-from mkosi.log import MkosiException
-
 
 def test_distribution() -> None:
     assert Distribution.fedora.package_type == PackageType.rpm
@@ -55,7 +53,7 @@ def test_safe_tar_extract(tmp_path: Path) -> None:
     assert (safe_target / name).exists()
 
     evil_target = tmp_path / "evil_target"
-    with pytest.raises(MkosiException):
+    with pytest.raises(ValueError):
         with tarfile.TarFile.open(evil_tar) as t:
             safe_tar_extract(t, evil_target)
     assert not (evil_target / name).exists()

diff --git a/tests/test_parse_load_args.py b/tests/test_parse_load_args.py
index d20523102c50f2534845a855e2b8aec647189430..1328b8053d8515a0a6e2f23f092e4a95b6aa3124 100644 (file)
--- a/tests/test_parse_load_args.py
+++ b/tests/test_parse_load_args.py
@@ -11,7 +11,6 @@ import pytest
 
 import mkosi
 from mkosi.backend import Distribution, MkosiConfig, Verb
-from mkosi.log import MkosiException
 from mkosi.config import MkosiConfigParser
 
 
@@ -77,13 +76,13 @@ def test_parse_config_files_filter() -> None:
 
 def test_shell_boot() -> None:
     with cd_temp_dir():
-        with pytest.raises(MkosiException, match=".boot.*tar"):
+        with pytest.raises(RuntimeError, match=".boot.*tar"):
             parse(["--format", "tar", "boot"])
 
-        with pytest.raises(MkosiException, match=".boot.*cpio"):
+        with pytest.raises(RuntimeError, match=".boot.*cpio"):
             parse(["--format", "cpio", "boot"])
 
-        with pytest.raises(MkosiException, match=".boot.*compressed" ):
+        with pytest.raises(RuntimeError, match=".boot.*compressed" ):
             parse(["--format", "disk", "--compress-output=yes", "boot"])