diff --git a/grapharc/cli/main.py b/grapharc/cli/main.py index 63339ff..a511211 100644 --- a/grapharc/cli/main.py +++ b/grapharc/cli/main.py @@ -47,7 +47,7 @@ from grapharc.cli.output import EXIT_FAILED, EXIT_OK, emit, fail from grapharc.observe.metrics import summarize, to_mermaid from grapharc.observe.replay import ReplayError -from grapharc.observe.trace import TraceRecorder +from grapharc.observe.trace import TraceReadError, TraceRecorder EXAMPLES = ( "stage0", @@ -480,7 +480,13 @@ def _cmd_trace(args: argparse.Namespace) -> int: recorder = _existing_trace(args.path, command="trace", as_json=args.json) if isinstance(recorder, int): return recorder - events = recorder.read_events(args.run_id) + try: + events = recorder.read_events(args.run_id) + except TraceReadError as exc: + # A bad line — a truncated write, a hand edit — is the "unreadable + # trace" the exit-code contract names, not a traceback. Same for + # `metrics` and `viz` below: all three read this file. + return fail(str(exc), as_json=args.json, command="trace") payload: dict[str, Any] = { "ok": True, "command": "trace", @@ -510,7 +516,10 @@ def _cmd_metrics(args: argparse.Namespace) -> int: recorder = _existing_trace(args.path, command="metrics", as_json=args.json) if isinstance(recorder, int): return recorder - metrics = summarize(recorder, args.run_id) + try: + metrics = summarize(recorder, args.run_id) + except TraceReadError as exc: + return fail(str(exc), as_json=args.json, command="metrics") if metrics is None: return fail( f"no events for run {args.run_id!r} in {args.path}", @@ -533,6 +542,8 @@ def _cmd_viz(args: argparse.Namespace) -> int: return recorder try: mermaid = to_mermaid(recorder, args.run_id) + except TraceReadError as exc: + return fail(str(exc), as_json=args.json, command="viz") except ReplayError as exc: # Every other reading command answers this as a document; `viz` used to # let it out as a traceback with empty stdout, which breaks the CLI's own diff --git a/grapharc/observe/__init__.py b/grapharc/observe/__init__.py index c58403a..1046851 100644 --- a/grapharc/observe/__init__.py +++ b/grapharc/observe/__init__.py @@ -40,7 +40,7 @@ replay, replay_thread, ) -from grapharc.observe.trace import TraceEvent, TraceRecorder, load_events +from grapharc.observe.trace import TraceEvent, TraceReadError, TraceRecorder, load_events __all__ = [ "ListSpanExporter", @@ -61,6 +61,7 @@ "SpanExporter", "ThreadCost", "TraceEvent", + "TraceReadError", "TraceRecorder", "attribute", "attribute_thread", diff --git a/grapharc/observe/trace.py b/grapharc/observe/trace.py index 9ef46e4..2fb24cb 100644 --- a/grapharc/observe/trace.py +++ b/grapharc/observe/trace.py @@ -20,11 +20,29 @@ from pathlib import Path from typing import Any -from pydantic import BaseModel +from pydantic import BaseModel, ValidationError _MAX_VALUE_CHARS = 2000 +class TraceReadError(Exception): + """A line in a trace file is not a `TraceEvent`. + + Raised instead of skipped on purpose: a partially-read audit trail + presented as complete would be worse than a refusal. The message names the + file and the 1-based line so the one bad line — a process killed mid-write, + a hand edit — can be found without a pydantic field listing. + """ + + def __init__(self, path: Path, line_number: int, cause: Exception) -> None: + super().__init__( + f"unreadable trace file: {path}: line {line_number} is not a trace event" + ) + self.path = path + self.line_number = line_number + self.cause = cause + + def _jsonable(value: Any) -> Any: """Best-effort conversion to something json.dumps accepts, truncating long text.""" if isinstance(value, BaseModel): @@ -186,10 +204,13 @@ def read_events(self, run_id: str | None = None) -> list[TraceEvent]: return [] events = [] with self.path.open(encoding="utf-8") as f: - for line in f: + for line_number, line in enumerate(f, start=1): if not line.strip(): continue - ev = TraceEvent.model_validate_json(line) + try: + ev = TraceEvent.model_validate_json(line) + except ValidationError as exc: + raise TraceReadError(self.path, line_number, exc) from exc if run_id is None or ev.run_id == run_id: events.append(ev) return events @@ -207,4 +228,4 @@ def load_events( return recorder.read_events(run_id) -__all__ = ["TraceEvent", "TraceRecorder", "load_events"] +__all__ = ["TraceEvent", "TraceReadError", "TraceRecorder", "load_events"] diff --git a/tests/test_cli.py b/tests/test_cli.py index 00fcbf1..50f0326 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -336,6 +336,43 @@ def test_viz_renders_the_executed_path(two_runs, capsys): assert "load" in payload["mermaid"] +# Every reading command, with the arguments it needs beyond the path. `metrics` +# and `viz` never reach the run id: the file refuses before any run is looked up. +READERS = [["trace"], ["metrics", "r1"], ["viz", "r1"]] + + +def _bad_trace(tmp_path) -> Path: + """A trace whose second line is not an event — a process killed mid-write.""" + trace = TraceRecorder(tmp_path / "bad.jsonl") + trace.event(run_id="r1", graph="g", node="load", phase="start", step=1) + with trace.path.open("a", encoding="utf-8") as f: + f.write('{"not": "a trace event"}\n') + return trace.path + + +@pytest.mark.parametrize("argv", READERS, ids=lambda argv: argv[0]) +def test_a_malformed_trace_is_a_report_not_a_traceback(argv, tmp_path, capsys): + """The contract in `output.py` names "an unreadable trace" as exit 2.""" + bad = _bad_trace(tmp_path) + code, out, err = call([argv[0], str(bad), *argv[1:]], capsys) + assert code == 2 + assert out == "" + assert f"error: unreadable trace file: {bad}: line 2 is not a trace event\n" == err + + +@pytest.mark.parametrize("argv", READERS, ids=lambda argv: argv[0]) +def test_a_malformed_trace_fails_as_one_json_document(argv, tmp_path, capsys): + bad = _bad_trace(tmp_path) + code, payload, err = call_json([argv[0], str(bad), *argv[1:]], capsys) + assert code == 2 + assert payload == { + "ok": False, + "command": argv[0], + "error": f"unreadable trace file: {bad}: line 2 is not a trace event", + } + assert err == "" + + # -- models -------------------------------------------------------------------