Skip to content

Replace anyhow with custom error types using thiserror#1192

Merged
kixelated merged 3 commits into
mainfrom
claude/moq-token-thiserror-SN5wz
Apr 2, 2026
Merged

Replace anyhow with custom error types using thiserror#1192
kixelated merged 3 commits into
mainfrom
claude/moq-token-thiserror-SN5wz

Conversation

@kixelated

Copy link
Copy Markdown
Collaborator

This PR replaces the anyhow crate with a custom error type hierarchy using thiserror, providing more structured and type-safe error handling throughout the moq-token crate.

Summary

Migrated from generic anyhow::Error to a custom Error enum with a specialized KeyError variant for cryptographic operations. This improves error semantics, enables better error handling patterns, and reduces external dependencies.

Key Changes

  • New error module (error.rs): Introduced KeyError enum for key-specific errors and top-level Error enum that encompasses all error types in the crate

    • KeyError variants cover algorithm mismatches, missing keys, unsupported operations, and coordinate validation
    • Error enum aggregates KeyError and integrates with external error types (JSON, IO, Base64, JWT, RSA, etc.)
    • Defined Result<T> type alias as std::result::Result<T, Error>
  • Updated key.rs: Replaced all anyhow::Result with crate::Result and converted error handling:

    • Replaced bail!() and anyhow::anyhow!() with explicit KeyError variants
    • Simplified error propagation using ? operator instead of match expressions
    • Removed unnecessary Context trait usage, using ok_or() for Option conversions
  • Updated generate.rs: Migrated to custom error types and simplified RSA key generation error handling

  • Updated set.rs: Replaced anyhow::Result with crate::Result and removed verbose error wrapping in to_public_set()

  • Updated algorithm.rs: Changed FromStr::Err type from anyhow::Error to Error with proper variant usage

  • Updated claims.rs: Replaced anyhow::bail!() with crate::Error::UselessToken

  • Updated Cargo.toml: Replaced anyhow = "1" dependency with thiserror = "2"

Notable Implementation Details

  • Error types use #[from] attributes for automatic conversion from underlying error types
  • Preserved all error context and messages through descriptive error variants
  • Maintained backward compatibility with public API return types (still return Result<T>)
  • Conditional compilation for jwks-loader feature errors

https://claude.ai/code/session_01AaDSYmsVmbhVA6iSFJ6yNa

Replace anyhow::Result with a structured Error enum using thiserror.
This gives consumers typed errors they can match on instead of opaque
anyhow errors.

- Add Error enum with #[from] for all external error types (serde_json,
  base64, io, jsonwebtoken, elliptic-curve, rsa, aws-lc-rs, reqwest)
- Add KeyError enum for key-specific errors (invalid algorithm/curve,
  missing private key, unsupported operations, key not found, etc.)
- Export crate::Result<T> type alias
- Update all public API methods to return crate::Result<T>
- All 88 existing tests pass unchanged

https://claude.ai/code/session_01AaDSYmsVmbhVA6iSFJ6yNa
@coderabbitai

coderabbitai Bot commented Apr 2, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: dfdb1d75-5d62-4b52-b9d2-763e1bd7c8ec

📥 Commits

Reviewing files that changed from the base of the PR and between 467b326 and b1519c8.

📒 Files selected for processing (5)
  • rs/moq-token/Cargo.toml
  • rs/moq-token/src/claims.rs
  • rs/moq-token/src/generate.rs
  • rs/moq-token/src/key.rs
  • rs/moq-token/src/set.rs
🚧 Files skipped from review as they are similar to previous changes (5)
  • rs/moq-token/Cargo.toml
  • rs/moq-token/src/claims.rs
  • rs/moq-token/src/generate.rs
  • rs/moq-token/src/key.rs
  • rs/moq-token/src/set.rs

Walkthrough

The pull request replaces anyhow with thiserror in the crate manifest and introduces a new error module that defines KeyError, Error, and a Result<T> alias. Numerous public APIs and internal helpers across modules (algorithm, claims, generate, key, set) were changed from returning anyhow::Result to crate::Result, and error construction/propagation was refactored to return explicit Error/KeyError variants instead of using anyhow-based bail/context. lib.rs now re-exports the new error types.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 21.43% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main change: replacing anyhow with custom error types using thiserror. It clearly identifies the primary refactoring objective of the PR.
Description check ✅ Passed The description comprehensively explains the migration from anyhow to custom error types, detailing the new error module structure, changes across each affected file, and implementation details. It is directly related to the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/moq-token-thiserror-SN5wz
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch claude/moq-token-thiserror-SN5wz

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@rs/moq-token/src/generate.rs`:
- Around line 79-81: The code accesses RSA CRT fields via key.dp(), key.dq(),
and key.qinv() and uses expect(), which will panic because those Options are
only populated after precompute(); call key.precompute().map_err(|e| …)? (or
otherwise invoke the rsa crate's precompute method) before reading dp/dq/qinv in
generate.rs, then replace the expect() calls with proper error propagation
(e.g., .ok_or_else(|| YourError::MissingCrtField("dp"))? ) so the function
returns a Result on failure instead of panicking.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 24048dc1-112f-4110-89e8-06482215109c

📥 Commits

Reviewing files that changed from the base of the PR and between bb81dce and 467b326.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (8)
  • rs/moq-token/Cargo.toml
  • rs/moq-token/src/algorithm.rs
  • rs/moq-token/src/claims.rs
  • rs/moq-token/src/error.rs
  • rs/moq-token/src/generate.rs
  • rs/moq-token/src/key.rs
  • rs/moq-token/src/lib.rs
  • rs/moq-token/src/set.rs

Comment thread rs/moq-token/src/generate.rs Outdated
kixelated and others added 2 commits April 2, 2026 14:03
…error-SN5wz

# Conflicts:
#	rs/moq-token/src/key.rs
- Resolve merge conflict in key.rs: keep simplified error handling from
  PR while adding guest/guest_sub/guest_pub fields from main
- Address CodeRabbit review: replace expect() with proper error handling
  for RSA CRT fields (dp/dq/qinv) by calling precompute() first
- Apply cargo-sort and formatter fixes

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@kixelated kixelated merged commit f05a2fd into main Apr 2, 2026
2 checks passed
@kixelated kixelated deleted the claude/moq-token-thiserror-SN5wz branch April 2, 2026 22:13
This was referenced Apr 2, 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.

2 participants