Better error handling#65
Conversation
WalkthroughThe changes in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Rating
participant TagParser
User->>Rating: Request to parse tags
Rating->>TagParser: Call from_tags()
TagParser->>TagParser: Check for tag key
alt Tag key exists
TagParser->>TagParser: Check for tag value
alt Tag value exists
TagParser-->>Rating: Return parsed data
else Tag value missing
TagParser-->>Rating: Return error for missing tag value
end
else Tag key missing
TagParser-->>Rating: Return error for missing tag key
end
Rating-->>User: Respond with parsed data or error
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/rating.rs (1)
84-95: Consider further error handling improvementsWhile the current error handling is good, consider these enhancements:
- Create custom error types instead of using anyhow directly
- Include the tag index in error messages for better debugging
- Add validation for parsed values (e.g., ensure ratings are within min/max bounds)
Example implementation:
#[derive(Debug, thiserror::Error)] pub enum RatingError { #[error("Missing tag key at index {0}")] MissingKey(usize), #[error("Missing tag value at index {0}")] MissingValue(usize), #[error("Invalid rating value: {0} (must be between {1} and {2})")] InvalidRating(u8, u8, u8), } // Then in the code: let key = t.first() .ok_or_else(|| RatingError::MissingKey(idx))?; let value = t.get(1) .ok_or_else(|| RatingError::MissingValue(idx))?; // After parsing last_rating: if last_rating < min_rate || last_rating > max_rate { return Err(RatingError::InvalidRating(last_rating, min_rate, max_rate).into()); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/rating.rs(1 hunks)
🔇 Additional comments (1)
src/rating.rs (1)
84-89: Improved error handling looks good!
The replacement of unwrap() with ok_or_else() provides better error handling and prevents potential panics. The error messages are clear and descriptive.
The current implementation has multiple unwrap() calls that could panic at runtime with malformed data. This is particularly risky when dealing with external Nostr data.
Summary by CodeRabbit