Cyphrme / Cyphrme/Cyphr

Audit unjustified runtime panics in production code (.unwrap()/.expect())

Open
#37 0 comments 0 reactions 0 assignees View on GitHub
tech-debt
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.