Skip to content

fix(robustness): replace risky production unwrap() with typed error handling - #3465

Closed
jewoos2921 wants to merge 1 commit into
block:mainfrom
jewoos2921:fix/prod-unwrap-removal
Closed

fix(robustness): replace risky production unwrap() with typed error handling#3465
jewoos2921 wants to merge 1 commit into
block:mainfrom
jewoos2921:fix/prod-unwrap-removal

Conversation

@jewoos2921

Copy link
Copy Markdown

Summary

Removes unwrap() calls from production code paths where runtime panics could be triggered by external input or platform-specific conditions, as part of a codebase audit following AGENTS.md's no-unwrap-in-production policy.

Changes

🔴 cas_publish.rs — real panic risk fixed

  • pack_path.to_str().unwrap()ok_or_else(CasError)?
  • Risk: Non-UTF-8 temp directory paths (e.g. Windows usernames with non-ASCII characters) would panic the relay during git index-pack. Now returns a typed CasError instead.

🟡 validation.rs — invariant documentation (9 sites)

  • try_into().unwrap() in PNG/WebP/MP4/GIF metadata validation → expect() with documented invariant
  • Each site is guarded by an explicit bounds check earlier in the same function, so the unwrap never fires in practice. expect() makes the safety invariant explicit and produces a clear diagnostic if the assumption ever breaks during future refactoring.

🟡 agents.rs — invariant documentation

  • events.next().unwrap()expect() with invariant comment
  • Already guarded by an is_empty() early-return above; made the safety explicit.

What was NOT changed

The remaining ~18 production unwrap() calls are all in categories where failure is genuinely impossible (e.g. "*".parse().unwrap() for a static header value, Response::builder().body(Body::from(...)).unwrap() where the body type is infallible, SHA-256 digest slicing). Changing these would add noise without improving safety.

Note: side_effects.rs had two risky unwrap() calls (role string parsing, actor member lookup) that were also targeted, but main already refactored that code to eliminate them.

Verification

  • cargo check — compiles
  • cargo clippy -- -D warnings — no warnings
  • cargo fmt --check — formatted
  • cargo test -p buzz-media --lib — 103/103 pass
  • cargo test -p buzz-cli --lib — pass

…andling

Remove unwrap() calls from production paths where runtime panics could be
triggered by external input or platform-specific conditions:

- cas_publish.rs: pack_path.to_str().unwrap() → ok_or_else(CasError)?
  Non-UTF-8 temp paths (e.g. Windows usernames with non-ASCII chars)
  would panic the relay during git index-pack.

- validation.rs: 9x try_into().unwrap() in PNG/WebP/MP4/GIF metadata
  validation → expect() with documented invariant.
  While logically safe after bounds checks, expect() makes the invariant
  explicit and produces a clear diagnostic if the assumption ever breaks.

- agents.rs: events.next().unwrap() → expect() with invariant comment.
  Already guarded by an is_empty() check above; made the safety explicit.

Note: side_effects.rs role_str.parse().unwrap() and actor_member.unwrap()
were also addressed but main already refactored this code to remove them.
@jewoos2921
jewoos2921 requested a review from a team as a code owner July 29, 2026 04:40
@jewoos2921 jewoos2921 closed this by deleting the head repository Jul 29, 2026
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