Skip to content

fix: actionable errors for malformed --data instead of a raw traceback - #32

Merged
igor-ctrl merged 2 commits into
mainfrom
fix/data-json-errors
Aug 11, 2026
Merged

fix: actionable errors for malformed --data instead of a raw traceback#32
igor-ctrl merged 2 commits into
mainfrom
fix/data-json-errors

Conversation

@igor-ctrl

Copy link
Copy Markdown
Owner

Summary

  • post, patch, and action each carried their own copy of an unguarded json.loads() for --data/-d. A shell-mangled inline literal (PowerShell strips quotes from {"a": 1} routinely) or a bare file path passed without the @ prefix surfaced a ~25-line raw json.JSONDecodeError traceback instead of telling the user what to fix.
  • Extracted the shared parsing logic into bcli_cli._data_arg.parse_data_argument() (the codebase already had a pattern of single-purpose _*.py helper modules shared across commands — _safety.py, _url_resolve.py, _envelope_wrap.py — so this follows suit rather than leaving three drifting copies).
  • Every failure now raises typer.BadParameter (never a bare JSONDecodeError): the message includes the JSON error's line/column/reason and a short excerpt of what was received (truncated to ~80 chars, never the whole payload), plus a one-line hint when the input looks like a filesystem path (missing @) or a shell-mangled literal (starts with { but has no quotes, or has unquoted keys).
  • A malformed @file now names the file in its error. A missing @file keeps the existing "File not found" message unchanged.

Changes

  • src/bcli_cli/_data_arg.py — new shared helper, parse_data_argument().
  • src/bcli_cli/commands/{post,patch,action}_cmd.py — use the shared helper, removed the three duplicated _parse_data() definitions.
  • tests/test_cli/test_data_arg.py — new, full heuristic matrix for the shared helper.
  • tests/test_cli/test_post_cmd.py, tests/test_cli/test_patch_cmd.py — new, command-level coverage (these two commands had no dedicated test file before).
  • tests/test_cli/test_action_cmd.py — extended TestDataHandling with the same error-path cases.
  • CHANGELOG.md[0.8.1] entry.
  • pyproject.toml / uv.lock — version bump to 0.8.1 (src/bcli/_version.py reads from package metadata at runtime, so it needed no change).
  • docs/command-reference.md, docs/write-operations.md — documented that @ is required (a bare path is parsed as JSON, not silently treated as a file) and the PowerShell quote-stripping gotcha.

Test plan

  • uv run --extra dev python -m pytest tests/ -q — 1131 passed, 5 skipped (pre-existing, unrelated), 3 warnings (pre-existing, unrelated to this change).
  • uv run ruff check src/ tests/ — all checks passed.
  • Manually exercised all target scenarios against parse_data_argument() directly (valid inline, valid @file, shell-mangled with/without quotes, bare existing file path, Windows drive path, path-like-but-missing, missing @file, malformed @file, generic garbage) — every failure path raises typer.BadParameter with the expected hint, none leak a raw JSONDecodeError.

post, patch, and action each had their own unguarded json.loads() on
--data/-d, so a shell-mangled inline literal (PowerShell strips quotes
routinely) or a bare file path passed without the @ prefix surfaced a
raw json.JSONDecodeError traceback instead of telling the user what to
fix. Extract the parsing into bcli_cli._data_arg.parse_data_argument
and raise typer.BadParameter on every failure, with the JSON error's
line/column/reason, a short excerpt of the input, and a one-line hint
when the input looks like a filesystem path or a shell-mangled literal.
@igor-ctrl
igor-ctrl force-pushed the fix/data-json-errors branch from f6af0b6 to 39b283c Compare August 11, 2026 21:22
The path heuristic called Path(data).is_file() on the raw argument. On Linux
any argument over 255 bytes raises OSError(ENAMETOOLONG) and an embedded NUL
raises ValueError, so a long malformed payload — the kind most likely to be
malformed in the first place — escaped as a raw traceback: precisely the
failure this module was added to remove. macOS does not raise, so it passed
locally and only surfaced on CI.

Both filesystem probes are now guarded: the heuristic treats an unusable name
as "not a path", and the @file branch reports "File not found" rather than
propagating OSError. A file that exists but can't be read now reports
"Could not read <path>: <reason>" instead of an OSError traceback.

Regression tests cover long, long-and-JSON-ish, long-path-shaped, NUL-bearing
payloads and a long @path. They pass trivially on macOS and are meaningful on
Linux/CI, which is where the defect lives.
@igor-ctrl
igor-ctrl merged commit d213215 into main Aug 11, 2026
3 checks passed
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.

1 participant