fix: Add handling error in python rules #12
Conversation
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage. @@ Coverage Diff @@
## main #12 +/- ##
==========================================
- Coverage 71.61% 69.67% -1.94%
==========================================
Files 17 17
Lines 1335 1372 +37
==========================================
Hits 956 956
- Misses 379 416 +37
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR improves error handling in the Python rules processing system by replacing panic-prone ? operators with explicit error handling that logs issues and continues processing remaining rules. This prevents a single failing rule from stopping the entire rule processing pipeline.
Key Changes:
- Replaced
?operator withmatchexpressions for all PyO3 operations (getattr, call1, extract) - Added descriptive error messages for each failure point (getting functions, executing functions)
- Changed behavior from immediate propagation to logging errors and continuing with next rule
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let match_func = match module.getattr("match") { | ||
| Ok(func) => func, | ||
| Err(e) => { | ||
| eprintln!( | ||
| "{}{}{}", | ||
| "Failed to get 'match' function from rule '".yellow(), | ||
| rule_path.display(), | ||
| "': ".yellow(), | ||
| ); | ||
| eprintln!("{e}"); | ||
| continue; | ||
| } | ||
| }; | ||
| let fix_func = match module.getattr("fix") { | ||
| Ok(func) => func, | ||
| Err(e) => { | ||
| eprintln!( | ||
| "{}{}{}", | ||
| "Failed to get 'fix' function from rule '".yellow(), | ||
| rule_path.display(), | ||
| "': ".yellow(), | ||
| ); | ||
| eprintln!("{e}"); | ||
| continue; | ||
| } | ||
| }; | ||
| if match_func.is_callable() && fix_func.is_callable() { | ||
| if match_func | ||
| let is_match = match match_func | ||
| .call1(( | ||
| command.command(), | ||
| command.output().stdout(), | ||
| command.output().stderr(), | ||
| ))? | ||
| .extract::<bool>()? | ||
| )) | ||
| .and_then(|result| result.extract::<bool>()) | ||
| { | ||
| let fixed_command: String = fix_func | ||
| Ok(result) => result, | ||
| Err(e) => { | ||
| eprintln!( | ||
| "{}{}{}", | ||
| "Failed to execute 'match' function in rule '".yellow(), | ||
| rule_path.display(), | ||
| "': ".yellow(), | ||
| ); | ||
| eprintln!("{e}"); | ||
| continue; | ||
| } | ||
| }; | ||
| if is_match { | ||
| let fixed_command: String = match fix_func | ||
| .call1(( | ||
| command.command(), | ||
| command.output().stdout(), | ||
| command.output().stderr(), | ||
| ))? | ||
| .extract()?; | ||
| )) | ||
| .and_then(|result| result.extract()) | ||
| { | ||
| Ok(cmd) => cmd, | ||
| Err(e) => { | ||
| eprintln!( | ||
| "{}{}{}", | ||
| "Failed to execute 'fix' function in rule '".yellow(), | ||
| rule_path.display(), | ||
| "': ".yellow(), | ||
| ); | ||
| eprintln!("{e}"); | ||
| continue; | ||
| } | ||
| }; |
There was a problem hiding this comment.
The new error handling logic for Python rule processing lacks test coverage. Consider adding unit tests that verify:
- Error handling when
getattr("match")fails - Error handling when
getattr("fix")fails - Error handling when
match_func.call1()fails - Error handling when
fix_func.call1()fails - Verification that the loop continues processing other rules after an error
These tests would help ensure the error handling behaves correctly and prevent regressions. Other modules in src/fix/ (like rust.rs and individual rule files) have comprehensive test coverage that could serve as examples.
Description
fix #9
Type of Change
Testing
Checklist