diff --git a/CHANGELOG.md b/CHANGELOG.md index 49e82fe..a8643aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,12 +23,16 @@ and versions are tracked in the repo-root `VERSION` file. ### Fixed +- Surface expected output-format and optional-dependency failures as actionable + usage errors at the process boundary, including a stable JSON error code. + - Bind an allowlisted Python extension to its discovered entry-point source and fail closed if its distribution identity changes before loading. - Preserve run bundles while their lease is held through terminal metadata writing and cleanup; recheck lease state before deletion. + - Detect JSON capture without running Click callbacks, callable defaults, type converters, or close hooks a second time; respect option-value arity so a payload equal to `--json` remains human output. diff --git a/docs/output-contracts.md b/docs/output-contracts.md index 684ed5d..5fe5fd2 100644 --- a/docs/output-contracts.md +++ b/docs/output-contracts.md @@ -4,6 +4,9 @@ and `ndjson` formats. Install `base-cli[yaml]` before selecting `yaml`; the other formats are available from the core package. The requested `text` format is presentation-aware: it renders a table on a TTY and tab-delimited rows when stdout is redirected or piped. +When an output format or its optional dependency is invalid, `run_app()` reports +an actionable usage error (exit code `2`); JSON mode uses the stable +`output_format_error` envelope code. Delimited output is intentionally automation-friendly: diff --git a/lib/python/base_cli/_lifecycle.py b/lib/python/base_cli/_lifecycle.py index 9a89e49..b2dd040 100644 --- a/lib/python/base_cli/_lifecycle.py +++ b/lib/python/base_cli/_lifecycle.py @@ -11,6 +11,7 @@ from .context import Context from .exit_codes import ExitCode from .history import compact_optional_path, format_timestamp, status_for_exit_code +from .output import OutputFormatError @dataclass(frozen=True) @@ -129,6 +130,8 @@ def outcome_from_exception(click: Any, exc: BaseException) -> InvocationOutcome: return InvocationOutcome("interrupted", "aborted", ExitCode.INTERRUPTED) if isinstance(exc, EOFError): return InvocationOutcome("aborted", "error", ExitCode.FAILURE) + if isinstance(exc, OutputFormatError): + return InvocationOutcome("output_format_error", "error", ExitCode.USAGE_ERROR) if isinstance(exc, click.Abort): if isinstance(exc.__cause__, KeyboardInterrupt): return InvocationOutcome("interrupted", "aborted", ExitCode.INTERRUPTED) diff --git a/lib/python/base_cli/_run.py b/lib/python/base_cli/_run.py index c87cd05..d115337 100644 --- a/lib/python/base_cli/_run.py +++ b/lib/python/base_cli/_run.py @@ -29,6 +29,7 @@ from .exit_codes import ExitCode from .json_contracts import dumps_envelope, error_envelope, success_envelope from .lifecycle_options import LifecycleOption, LifecycleOptions +from .output import OutputFormatError from .redaction import option_aliases_from_decls _MAX_JSON_CAPTURE_BYTES = 8 * 1_048_576 @@ -311,6 +312,15 @@ def run_app( _emit_json_error(state, outcome, str(exc), output_capture) return outcome.exit_code raise + except OutputFormatError as exc: + if reraise_unexpected: + raise + outcome = InvocationOutcome("output_format_error", "error", ExitCode.USAGE_ERROR) + if state.json_output: + _emit_json_error(state, outcome, str(exc), output_capture) + else: + print(f"Error: {exc}", file=sys.stderr) + return outcome.exit_code except Exception as exc: if reraise_unexpected: raise diff --git a/tests/test_app_run.py b/tests/test_app_run.py index b542ce6..46dc976 100644 --- a/tests/test_app_run.py +++ b/tests/test_app_run.py @@ -11,6 +11,8 @@ from unittest import mock import base_cli +from base_cli._lifecycle import outcome_from_exception +from base_cli.output import OutputFormatError def generic_app(**kwargs: object) -> base_cli.App: @@ -18,6 +20,30 @@ def generic_app(**kwargs: object) -> base_cli.App: class RunAppTests(unittest.TestCase): + @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") + def test_output_format_errors_are_usage_outcomes_and_can_be_reraised(self) -> None: + import click + + outcome = outcome_from_exception(click, OutputFormatError("unsupported format")) + self.assertEqual((outcome.kind, outcome.exit_code), ("output_format_error", 2)) + + app = base_cli.App(name="output-format-reraise", log_to_file=False) + + @app.command() + def main(ctx: base_cli.Context) -> None: + del ctx + raise OutputFormatError("unsupported format") + + with ( + tempfile.TemporaryDirectory() as tmpdir, + mock.patch.dict( + os.environ, + {"HOME": tmpdir, "BASE_CLI_CACHE_DIR": str(Path(tmpdir) / ".cache")}, + ), + ): + with self.assertRaises(OutputFormatError): + base_cli.run_app(app, [], reraise_unexpected=True) + @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") def test_malformed_click_exit_code_before_context_is_an_unexpected_error(self) -> None: import click diff --git a/tests/test_optional_yaml_dependency.py b/tests/test_optional_yaml_dependency.py index 9e5b1d7..5ae832a 100644 --- a/tests/test_optional_yaml_dependency.py +++ b/tests/test_optional_yaml_dependency.py @@ -1,6 +1,7 @@ from __future__ import annotations import io +import json import sys import tempfile import unittest @@ -25,6 +26,39 @@ def test_yaml_output_explains_optional_install_when_yaml_is_missing(self) -> Non stream=stream, ) + def test_run_app_reports_missing_yaml_as_actionable_usage_error(self) -> None: + app = base_cli.App( + name="optional-yaml-output", + log_to_file=False, + lifecycle_options=base_cli.LifecycleOptions(json=base_cli.LifecycleOption("--json")), + ) + + @app.command() + def main(ctx: base_cli.Context) -> None: + del ctx + render_records( + ({"name": "value"},), + requested_format="yaml", + columns=(("NAME", "name"),), + ) + + for args in ([], ["--json"]): + with self.subTest(json=args == ["--json"]), tempfile.TemporaryDirectory() as home: + with mock.patch.dict(sys.modules, {"yaml": None}): + result = base_cli.testing.invoke(app, args, home=Path(home)) + + self.assertEqual(result.exit_code, base_cli.ExitCode.USAGE_ERROR) + if args: + payload = json.loads(result.stdout) + self.assertEqual(payload["code"], "output_format_error") + self.assertEqual(payload["details"]["exit_code"], base_cli.ExitCode.USAGE_ERROR) + self.assertIn("base-cli[yaml]", payload["message"]) + self.assertEqual(result.stderr, "") + else: + self.assertEqual(result.stdout, "") + self.assertIn("Error: PyYAML is required", result.stderr) + self.assertIn("base-cli[yaml]", result.stderr) + def test_yaml_config_explains_optional_install_when_yaml_is_missing(self) -> None: with tempfile.TemporaryDirectory() as tmpdir: path = Path(tmpdir) / "config.yaml"