Report an unreadable input, rather than tracebacking out of the CLI - #73
Merged
Conversation
Four reads could raise a type their caller did not catch, so a directory, an unreadable file or a non-UTF-8 one exited 1 with a traceback where the contract promises exit 2 and a document: _existing_trace tested exists() while the handlers catch only TraceReadError, and two read_text calls sat outside their decoders' except (UnicodeDecodeError is a ValueError, so neither would ever have caught it). The config case is the worst of them, because grapharc.toml is picked up implicitly from the working directory. plan --approve also printed its park notice ahead of the JSON document, which made the whole output unparseable on the flag most likely to be driven unattended; the notice is now silent in JSON mode and unchanged for a human. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #63. Fixes #62.
#63 — four reads that could raise a type nobody caught
trace/metrics/viz <directory> --jsonIsADirectoryErrortraceback{"ok": false, "error": "unreadable trace file: adir: Is a directory"}trace <unreadable file>PermissionErrortracebackrun <non-UTF-8 file> --jsonUnicodeDecodeErrortracebackdemo/run/planwith a binarygrapharc.tomlUnicodeDecodeErrortracebackConfigErrornaming the fileThree root causes, all the same shape — a read whose failure mode the surrounding handler does not cover:
_existing_trace(cli/main.py) testedpath.exists(), and the three reader handlers catch onlyTraceReadError, so every otherOSErrorwent past both. It now opens the file where the failure is still reportable.cli/graphrun.pyandcli/config.pyeach hadread_text(encoding="utf-8")outside theexceptthat catches their decoder.UnicodeDecodeErroris aValueError, so neitherJSONDecodeErrornorTOMLDecodeErrorwas ever going to catch it.replay/diffalready handled this correctly, which is what marked the rest as an oversight rather than a decision. The config case matters most:grapharc.tomlis picked up implicitly from the working directory, so a stray binary file there broke every configurable command.#62 —
plan --approve --jsonemitted unparseable outputThe park notice printed to stdout ahead of the document, so
json.load(stdout)raised. It is now silent in JSON mode (matchingcli/live.py, which already guards its announce) and unchanged in text mode — a human still getswaiting for approval (up to 3s) — answer with: grapharc approve <dir>.Tests
Six new tests, each confirmed red against the stashed pre-fix source:
test_a_directory_where_a_trace_belongs_{is_a_report_not_a_traceback,fails_as_one_json_document}, parametrized overtrace/metrics/viztest_run_reports_a_graph_file_that_is_not_utf8test_a_config_that_is_not_utf8_names_the_filetest_plan_approve_in_json_mode_emits_one_document— assertsjson.loads(stdout)succeeds and stderr is emptytest_plan_approve_in_text_mode_still_announces_how_to_answer— a guard so silencing JSON mode cannot silence humansVerification
pytest→ 1785 passed, 12 deselected.ruff check grapharc tests→ clean. No flaky timing failures in this run.🤖 Generated with Claude Code