Skip to content

fix: report output format failures as usage errors - #363

Open
codeforester wants to merge 2 commits into
mainfrom
bug/343-20260918-output-format-errors
Open

codeforester wants to merge 2 commits into
mainfrom
bug/343-20260918-output-format-errors

Conversation

@codeforester

Copy link
Copy Markdown
Contributor

Summary

  • Classify public OutputFormatError failures at the run_app process boundary as exit-code 2 usage errors.
  • Preserve safe actionable dependency/format messages in human output and use output_format_error in JSON mode.
  • Add process-boundary tests for optional YAML dependency failures and document the contract.

Closes #343

Validation

  • uv run --extra dev --extra typer --extra quality python -m pytest tests/test_optional_yaml_dependency.py tests/test_json_contracts.py -q
  • Ruff check and format check for changed Python files
  • git diff --check

_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.

@codeforester

Copy link
Copy Markdown
Contributor Author

Additional finding (file not touched by this diff, so not postable as an inline comment):

lib/python/base_cli/_lifecycle.py:150outcome_from_exception() has no case for OutputFormatError, so the new usage-error classification this PR adds in run_app() isn't mirrored in run history/telemetry/diagnostics. Reproduced: a command raising OutputFormatError (e.g. missing PyYAML) correctly returns exit code 2 to the shell/JSON caller, but the persisted run.json lifecycle bundle records "outcome": "unexpected_error", "exit_code": 1, "status": "error" — a real on-disk divergence from what was actually returned — and _record_lifecycle_diagnostic() logs it as an 'Unexpected command exception' with a full traceback, mislabeling a clean, documented usage error as an internal crash. Any tooling that inspects run history/telemetry (e.g. basectl runs list) will disagree with the actual exit code. Recommend adding an OutputFormatError case to outcome_from_exception() alongside this fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: expose actionable output-format errors at the process boundary

1 participant