jpmschweitzerandClaude Opus 5
49fa6ada95
Every non-zero exit names the command that would fix it, and still exits non-zero. Both halves matter; the second is the one that gets lost, because a tool that explains itself beautifully and exits 0 looks MORE correct while having silently disabled its own gate. core/errors.py holds ReachError(message, fix=) and @handle_errors. core/logging.py holds @logged, emitting through console rather than a second sink — one output path, so there is nothing to drift. core/command.py composes them, and the order is load-bearing: handle_errors wraps logged, so the logger sees the original exception. Inverted, every failure would be recorded as "SystemExit" and the log would say nothing about what went wrong while looking like it worked. core/ raises SystemExit, not typer.Exit. A service must be callable from a test, another service, or a future second front end, and an exception type that only makes sense inside a CLI leaks the transport into every layer. The check router is retrofitted off its hand-rolled verdict-and-exit pattern — exactly the boilerplate this removes — and test_check_parity.py passes unchanged across the retrofit. That test predates the decorators and pins exit codes against the old script, so it is independent evidence, not a test tuned to match new behaviour. Unknown domains and unknown verbs now enumerate what exists instead of only saying no. That needed a shared group class, which collided with "no typer outside main.py and router.py" — resolved by sharpening the invariant rather than breaking it, since its purpose is that a SERVICE never knows it was called from a CLI. Transport now lives in main.py, router.py and core/cli.py; never in service.py, schemas.py or helpers.py. The upside is that cli.domain() carries the settings that were previously per-router decisions, including the load-bearing rich_markup_mode=None that one forgetful domain could have undone. test_conformance.py makes five invariants executable, AST-based rather than grep. Scoped to the package, not the 123 legacy scripts — and deliberately so: as T-1250 moves each script into domains/, it lands inside the scope and the rules start applying automatically, so the test's reach grows with the migration. Proven to fail before being trusted: removing @command and removing a fix= each produced a failure naming the file, the line and the reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
111 lines
4.1 KiB
Python
111 lines
4.1 KiB
Python
"""Failures that teach, as a decorator rather than call-site discipline (D-263).
|
|
|
|
The contract: **every non-zero exit prints the command that would fix it, and
|
|
still exits non-zero.** Both halves matter, and the second is the one that gets
|
|
lost. A tool that explains itself beautifully and exits 0 has silently disabled
|
|
its own gate — and the explanation makes it look *more* correct, not less, which
|
|
is why this is a decorator with a test behind it and not a convention.
|
|
|
|
No typer or click import here. `core/` is transport substrate: a service must be
|
|
callable from a test, another service, or a future second front end, and an
|
|
exception type that only makes sense inside a CLI would leak the transport into
|
|
every layer. Exit is raised as a plain `SystemExit`, which click passes through
|
|
untouched.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import functools
|
|
from collections.abc import Callable, Iterable
|
|
from typing import Any, TypeVar
|
|
|
|
from tooling.core import console
|
|
|
|
F = TypeVar("F", bound=Callable[..., Any])
|
|
|
|
|
|
class ReachError(Exception):
|
|
"""A failure the caller can act on.
|
|
|
|
`fix` is not optional in spirit — it is the whole point. If you cannot name
|
|
a next command, you probably do not understand the failure well enough to
|
|
report it yet, and a message that only says "no" is the thing this exists to
|
|
replace.
|
|
"""
|
|
|
|
def __init__(self, message: str, *, fix: str | None = None, exit_code: int = 1) -> None:
|
|
super().__init__(message)
|
|
self.message = message
|
|
self.fix = fix
|
|
# Non-zero by construction. A ReachError carrying exit_code=0 would be a
|
|
# contradiction — and exactly the silent-gate failure described above.
|
|
self.exit_code = exit_code if exit_code != 0 else 1
|
|
|
|
|
|
def unknown_choice(kind: str, given: str, accepted: Iterable[str]) -> ReachError:
|
|
"""Reject a value from a known finite set, naming the whole set.
|
|
|
|
Whenever the accepted values are knowable, print them. This is the specific
|
|
gap D-263 measured in pql — an invalid ticket status rejected without naming
|
|
the six valid ones — which leaves the caller grepping source to guess.
|
|
"""
|
|
options = sorted(accepted)
|
|
listed = ", ".join(options) if options else "(none available)"
|
|
return ReachError(
|
|
f"unknown {kind}: {given!r}",
|
|
fix=f"choose one of: {listed}",
|
|
exit_code=2,
|
|
)
|
|
|
|
|
|
def handle_errors(func: F) -> F:
|
|
"""Render a failure through `console`, then exit with its code.
|
|
|
|
Deliberately catches nothing it cannot improve on. `SystemExit` passes
|
|
through — a decision to exit has already been made and re-reporting it would
|
|
double the output.
|
|
"""
|
|
|
|
@functools.wraps(func)
|
|
def wrapper(*args: Any, **kwargs: Any) -> Any:
|
|
try:
|
|
return func(*args, **kwargs)
|
|
except ReachError as exc:
|
|
# The verdict prints ONCE, LAST, after whatever the command streamed.
|
|
# A remedy emitted mid-stream at line 400 of 900 is technically
|
|
# printed and practically invisible.
|
|
console.verdict(exc.message, ok=False, fix=exc.fix)
|
|
raise SystemExit(exc.exit_code) from exc
|
|
except SystemExit:
|
|
raise
|
|
except Exception as exc:
|
|
_report_unexpected(exc)
|
|
raise SystemExit(1) from exc
|
|
|
|
return wrapper # type: ignore[return-value]
|
|
|
|
|
|
def _report_unexpected(exc: Exception) -> None:
|
|
"""An exception nobody anticipated still exits non-zero and still says something.
|
|
|
|
The traceback goes behind `--verbose` rather than at a user who cannot act on
|
|
it; the one-line form names the flag that reveals it, so the next step is
|
|
always visible even when the failure was not foreseen.
|
|
"""
|
|
if console.is_verbose():
|
|
import traceback
|
|
|
|
console.event(traceback.format_exc().rstrip(), level="error")
|
|
console.verdict(
|
|
f"unexpected {type(exc).__name__}: {exc}",
|
|
ok=False,
|
|
fix="the traceback above is the whole story — this is a bug in reach, not in your input",
|
|
)
|
|
return
|
|
|
|
console.verdict(
|
|
f"unexpected {type(exc).__name__}: {exc}",
|
|
ok=False,
|
|
fix="re-run with --verbose for the traceback",
|
|
)
|