Skip to content

fix(pg-cli): replace unwrap()/panic with user-friendly errors#162

Merged
rubenhensen merged 2 commits intomainfrom
fix/pg-cli-unwrap-156
May 5, 2026
Merged

fix(pg-cli): replace unwrap()/panic with user-friendly errors#162
rubenhensen merged 2 commits intomainfrom
fix/pg-cli-unwrap-156

Conversation

@dobby-coder
Copy link
Copy Markdown
Contributor

@dobby-coder dobby-coder Bot commented May 5, 2026

Closes #156.

Summary

  • exec() in encrypt and decrypt now return anyhow::Result<()>, and main prints Error: ... with a full context chain on failure (exit code 1) instead of crashing with a panic backtrace.
  • Client::new validates the PKG URL up front; create_url returns a ClientError rather than panicking on Url::parse / Url::join. ClientError gains an InvalidUrl variant plus Display / source impls so the context chain renders the underlying url::ParseError.
  • The explicit panic! in decrypt.rs for input filenames not ending in .enc becomes a returned error. The check is factored into a small strip_enc_extension helper so it is unit-testable.
  • All File::open / File::create, metadata(), file_name(), progress-bar template construction, and the public-signing-key extraction paths now use anyhow context for human-readable messages.
  • print_qr returns Result and propagates QR serialization errors.

Behaviour is otherwise unchanged: same CLI surface, same network calls, same on-disk artifact format.

Tests

Unit tests added against the new helpers (the behavioural pieces that were extractable without a live PKG):

  • client::tests::rejects_invalid_baseurlClient::new("not a url") returns InvalidUrl instead of panicking later.
  • client::tests::accepts_valid_baseurl_and_joins_pathscreate_url joins endpoints correctly.
  • client::tests::create_url_joins_with_trailing_slash — joining is robust to a trailing-slash baseurl.
  • decrypt::tests::strips_enc_suffix, rejects_input_without_enc_suffix, rejects_empty_input — the .enc filename check now returns a friendly error and is exercised on both happy and rejection paths.

Also smoke-tested manually:

$ pg-cli enc /tmp/x.txt --identity 'not json' --pub-sign-id '[]'
Error: failed to parse identity JSON: expected ident at line 1 column 2
input was: not json

$ pg-cli enc /tmp/x.txt --identity '{}' --pub-sign-id '[]' --pkg 'not a url'
Error: failed to create PKG client for 'not a url': invalid PKG URL 'not a url': relative URL without a base

Reviewer quickstart

git fetch origin && git checkout fix/pg-cli-unwrap-156
cargo build -p pg-cli
cargo test  --manifest-path pg-cli/Cargo.toml --all-features
cargo fmt   --manifest-path pg-cli/Cargo.toml --all -- --check

Closes #156.

The pg-cli crate used unwrap() throughout non-test paths, so common
failure modes (missing input file, invalid PKG URL, malformed JSON
identity, encryption failure) crashed with raw stack traces instead
of helpful messages.

- exec() in encrypt and decrypt now return anyhow::Result<()>
- main returns ExitCode and prints "Error: ..." with full context
  chain, then exits 1
- Client::new validates the baseurl up front; create_url returns a
  ClientError instead of panicking on Url::parse / join
- ClientError gains InvalidUrl + Display/source impls so anyhow's
  context chain renders the underlying url::ParseError
- print_qr returns Result and propagates serialization errors
- decrypt's explicit panic for non-.enc input becomes a returned
  error; the check is extracted into a small helper for testing
- File open/create, metadata, file_name, progress-bar template, and
  signing-key extraction all use anyhow context for explanations
- Adds unit tests for the new helpers (strip_enc_extension,
  Client::new url validation, create_url join behaviour)

Behaviour is otherwise unchanged: same CLI surface, same network
calls, same artifact format.
@dobby-coder dobby-coder Bot requested a review from rubenhensen May 5, 2026 14:03
@dobby-coder dobby-coder Bot marked this pull request as ready for review May 5, 2026 14:11
@rubenhensen
Copy link
Copy Markdown
Contributor

fix conflicts

@dobby-coder
Copy link
Copy Markdown
Contributor Author

dobby-coder Bot commented May 5, 2026

Dobby has seen master rubenhensen's request! Dobby will fix the conflicts on this PR right away!

Resolved conflicts in pg-cli/Cargo.toml and pg-cli/src/client.rs:
- Adopted main's switch from lazy_static to std::sync::LazyLock (#158).
- Kept this branch's anyhow + url dependencies and ClientError/Display impls.
- Kept both sets of tests: the new URL validation tests from this branch
  plus main's headers_contain_client_version test.
@dobby-coder
Copy link
Copy Markdown
Contributor Author

dobby-coder Bot commented May 5, 2026

Conflicts resolved in 2a36f0e.

Files conflicted: pg-cli/Cargo.toml, pg-cli/src/client.rs

Resolution:

  • Adopted main's switch from lazy_static to std::sync::LazyLock (from Replace deprecated lazy_static with std::sync::LazyLock #158). Dropped the lazy_static = "1.4.0" dep this branch had added; kept the anyhow and url deps this PR introduces.
  • Kept both use std::fmt; (needed by this branch's ClientError Display impl) and use std::sync::LazyLock; (from main).
  • Merged both test sets: the three new URL-validation tests from this branch (rejects_invalid_baseurl, accepts_valid_baseurl_and_joins_paths, create_url_joins_with_trailing_slash) plus main's headers_contain_client_version.

Verified locally:

  • cargo build -p pg-cli
  • cargo test --manifest-path pg-cli/Cargo.toml --all-features → 7 passed, 0 failed ✅
  • cargo fmt --all -- --check
  • git diff origin/main --stat → 226 insertions across 7 files, so the PR is not a no-op against main.

Reviewer quickstart unchanged:

git fetch origin && git checkout fix/pg-cli-unwrap-156
cargo build -p pg-cli
cargo test  --manifest-path pg-cli/Cargo.toml --all-features

@rubenhensen rubenhensen merged commit 8e7c775 into main May 5, 2026
20 checks passed
@rubenhensen rubenhensen deleted the fix/pg-cli-unwrap-156 branch May 5, 2026 17:46
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.

pg-cli: pervasive unwrap() calls make CLI crash with unhelpful panic messages

1 participant