Skip to content

Conversation

@Lil-Duckling-22
Copy link

@Lil-Duckling-22 Lil-Duckling-22 commented Oct 23, 2025

Replaced all .unwrap() calls in regex compilation and JSON parsing with proper error handling using ? operator and custom error types. Added JsonParseError variant to handle JSON parsing failures gracefully. This prevents potential panics and improves error reporting for invalid regex patterns or malformed JSON configurations.

Summary by CodeRabbit

  • Bug Fixes
    • Improved error handling and reporting for JSON parsing failures.
    • Enhanced error propagation throughout data extraction functions for more reliable and graceful operation.

Replaced all .unwrap() calls in regex compilation and JSON parsing with proper error handling using ? operator and custom error types. Added JsonParseError variant to handle JSON parsing failures gracefully. This prevents potential panics and improves error reporting for invalid regex patterns or malformed JSON configurations.
@coderabbitai
Copy link

coderabbitai bot commented Oct 23, 2025

Walkthrough

Error handling improvements for JSON parsing in the extract_substrs module. Replaced unwrap-based parsing with explicit error handling, introducing a new JsonParseError variant to the ExtractSubstrssError enum, and propagating JSON failures through the normal error channel across ten extraction functions.

Changes

Cohort / File(s) Summary
Error handling and JSON parsing
packages/apis/src/extract_substrs.rs
Added JsonParseError(String) variant to ExtractSubstrssError enum. Replaced all serde_json::from_str(...).unwrap() calls with explicit parsing that maps JSON errors to the new error variant. Updated ten extraction functions (extract_email_addr_idxes, extract_email_domain_idxes, extract_from_all_idxes, extract_from_addr_idxes, extract_to_all_idxes, extract_to_addr_idxes, extract_subject_all_idxes, extract_body_hash_idxes, extract_timestamp_idxes, extract_message_id_idxes) to use explicit JSON parsing with proper error propagation via the ? operator. Refined regex construction flow with direct iteration over parts and proper error handling during compilation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

The changes are straightforward and follow a consistent pattern: replacing panics with proper error handling across multiple function calls. The modifications are localized to a single file with repetitive, homogeneous updates that facilitate predictable review.

Poem

🐰 A fuzzy fix hops into place,
No more unwraps in this race!
JSON errors now flow with grace,
Through proper paths we embraceβ€"
ErrorType’s our safety space! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "fix: replace unwrap() with proper error handling in regex compilation" accurately describes a central change in the pull request: replacing unwrap() calls with proper error propagation using the ? operator. The title is specific, clear, and technical, making it understandable to reviewers scanning the history. While the changeset also addresses JSON parsing error handling (introducing a new JsonParseError variant), the instructions acknowledge that titles don't need to cover every detail of the changeset. The title captures the primary approach and intent of the changes without being misleading or overly vague.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/apis/src/extract_substrs.rs (1)

65-66: Fix typo in comment and consider rewording.

Minor typo: "seperate" should be "separate".

Additionally, consider updating the comment to reflect that while failure is unlikely after validating individual parts, using ? provides defensive error handling for edge cases that might arise from the regex transformations.

Apply this diff to fix the typo:

-    // This should be impossible to fail, since we tested the seperate regex parts before.
+    // This should be impossible to fail, since we tested the separate regex parts before.

Or update the comment to be more accurate:

-    // This should be impossible to fail, since we tested the seperate regex parts before.
+    // Should not fail since individual parts were validated, but handle error defensively.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4fb1bda and 27d5322.

📒 Files selected for processing (1)
  • packages/apis/src/extract_substrs.rs (4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/apis/src/extract_substrs.rs (1)
packages/apis/src/wasm.rs (1)
  • serde_json (173-173)
🔇 Additional comments (3)
packages/apis/src/extract_substrs.rs (3)

39-40: LGTM! Well-defined error variant.

The new JsonParseError variant properly integrates with the existing error enum and will provide clear error messages when JSON parsing fails.


59-62: Good cleanup: removing unused enumeration.

The loop no longer needs the index since it's only building the regex string. This simplification improves readability.


108-110: Excellent: consistent error handling across all extraction functions.

All ten functions now properly handle JSON parsing errors using a consistent pattern. The error mapping preserves the original error details via to_string(), and the ? operator correctly propagates failures through the call stack.

Also applies to: 117-119, 133-135, 142-144, 149-151, 156-158, 165-167, 174-176, 183-185, 192-194

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