Audit unjustified runtime panics in production code (.unwrap()/.expect())
- Dominant language
- Rust
- Stars
- 3
- Forks
- 0
- Avg merge
- 1d 42m
- Merged PRs (30d)
- 11
Description
Standing bar: production code shouldn't panic at runtime without an explicit, well-justified reason — "handling errors properly is harder" doesn't meet that bar. For a server handling live requests, an unhandled panic in a request path is an availability concern, not just a code-quality one.
Surfaced while reviewing a new method on `CloneableLog` (`rs/cyphr/src/commit_root.rs`) that follows an existing pattern: 10 other methods on the same struct call `.expect("commit tree mutex poisoned")` on a poisoned `Mutex` rather than propagating a storage error. That specific pattern has a stated rationale — a poisoned mutex means a prior call already panicked mid-mutation, an unrecoverable state, so panicking again rather than plumbing a synthetic error is arguably intentional, not a shortcut. Whether that rationale actually holds up is worth a second look, but it's not the main point here.
The main point: this is one instance of a much broader pattern. A quick count of `.unwrap()`/`.expect()` in production (non-test) source across the workspace:
- `cyphr`: 342
- `cyphr-blob-fjall`: 49
- `cyphr-storage`: 13
- `cyphr-index-sqlite`: 8
- `cyphr-server`: 3
Almost none of these have been individually reviewed for whether they're a genuinely justified invariant (e.g. "this literally cannot fail given the preceding check") versus a convenience that should propagate a real, typed error instead. Worth a dedicated audit pass distinguishing the two, rather than a blanket rule.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with rs/cyphr/src/commit_root.rs and review the existing CloneableLog mutex expectations, then inventory .unwrap() and .expect() calls in non-test source across cyphr, cyphr-blob-fjall, cyphr-storage, cyphr-index-sqlite, and cyphr-server. Classify each occurrence as a justified invariant or a convenience that should propagate a typed error; done means the production call sites have been reviewed and the required changes are identified or made.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend, security
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100