Raise original exceptions

[mertunsall]

This PR modifies the exception handling to raise original exceptions instead of wrapping them.

[claude]

Claude finished @mertunsall's task —— View job

---

Code Review Complete

I've reviewed the PR that modifies exception handling in the bubus/models.py file. The changes look good overall and address a common pain point in exception handling.

Todo List:

• Check for repository guidelines (CLAUDE.md)
• Analyze the changes in bubus/models.py
• Provide comprehensive code review feedback
• Update comment with final review

Review Feedback

✅ Code Quality & Best Practices

The changes follow good exception handling practices by preserving the original exception type and structured data. The code is well-commented and the logic is clear.

✅ Functionality & Logic

The change correctly:

• Extracts the original error from failing_result.error or failing_result.result
• Logs the handler context for debugging (using logger.debug)
• Re-raises the original exception when it's a BaseException
• Falls back to wrapping in Exception for non-exception errors

⚠️ Potential Considerations

1. Exception Context Loss: While the original exception type is preserved, the stack trace context from the event handler execution might be lost. Consider if this could make debugging harder in some scenarios.

2. Logging Level: The error context is logged at DEBUG level (line 512). Consider if WARNING or ERROR might be more appropriate for production debugging, as debug logs are often disabled.

3. Fallback Case: The fallback case (line 518-519) converts non-exception errors to Exception(str(original_error)). This should indeed be rare in practice, but the string conversion might lose information if original_error has special formatting methods.

✅ Security

No security concerns identified. The changes don't introduce any new attack vectors.

✅ Performance

Minimal performance impact. The additional isinstance() check and logging are negligible overhead, and this only occurs in error paths.

❓ Test Coverage

I don't see test changes in this PR. Consider adding tests to verify:

• Original exceptions are preserved (type and attributes)
• Logging behavior works correctly
• Fallback case handles non-exception errors properly

📍 Code Location Reference

The changes are in bubus/models.py:507-520 in the event_results_filtered method.

Overall Assessment

This is a solid improvement that makes debugging easier by preserving original exception types. The implementation is clean and handles edge cases appropriately. Consider the logging level and test coverage suggestions above.

[mertunsall]

bugbot run

[cursor]

Skipping Bugbot: Bugbot is disabled for this repository