Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ 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.
- Preserve explicit application identities losslessly while using
collision-resistant, path-safe runtime namespace components.
- Give `BatteriesIncludedConfigLoader.cli_name` a documented identity role by
Expand Down
3 changes: 3 additions & 0 deletions docs/output-contracts.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
3 changes: 3 additions & 0 deletions lib/python/base_cli/_lifecycle.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -125,6 +126,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)
Expand Down
10 changes: 10 additions & 0 deletions lib/python/base_cli/_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,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
Expand Down Expand Up @@ -211,6 +212,15 @@ def run_app(
_emit_json_error(state, outcome, str(exc), output_capture)
return outcome.exit_code
raise
except OutputFormatError as exc:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consistency: the new except OutputFormatError branch never checks reraise_unexpected, unlike every other exception branch in run_app() that offers it as a debugging/testing escape hatch (e.g. base_cli.testing.invoke(app, args, reraise_unexpected=True)). Callers relying on that pattern to inspect the raw exception/traceback can't do so for this new error path.

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
Expand Down
26 changes: 26 additions & 0 deletions tests/test_app_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,39 @@
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:
return base_cli.App(profile=base_cli.CliProfile.generic(), **kwargs)


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
Expand Down
34 changes: 34 additions & 0 deletions tests/test_optional_yaml_dependency.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

import io
import json
import sys
import tempfile
import unittest
Expand All @@ -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"
Expand Down
Loading