-
Notifications
You must be signed in to change notification settings - Fork 924
Fix error propagation rule for Python's C API #2019
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
🎉 All Contributor License Agreements have been signed. Ready to merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a Python C API exception rule violation that was causing SystemError when callback functions raised exceptions. The fix properly transfers exception ownership between different Python execution contexts by storing and restoring exceptions during callback execution.
- Implements proper exception transfer using PyErr_Fetch/PyErr_Restore to maintain exception ownership
- Adds exception storage fields to CallState structure for context preservation
- Updates all callback functions to capture exceptions before crashing the call state
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/confluent_kafka/src/confluent_kafka.h | Adds exception storage fields to CallState structure |
src/confluent_kafka/src/confluent_kafka.c | Implements exception transfer logic in callbacks and CallState functions |
src/confluent_kafka/src/Producer.c | Updates delivery callback to capture exceptions before crashing |
src/confluent_kafka/src/Consumer.c | Updates rebalance callback to capture exceptions before crashing |
tests/test_Producer.py | Adds test verifying no SystemError occurs when delivery callback raises exception |
tests/test_Consumer.py | Adds test verifying no SystemError occurs when error callback raises exception |
Comments suppressed due to low confidence (2)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great description of the issue and clean fix!
Looks like some whitespace issue are making the linter fail |
What
This PR fixes issue : #865
Couple of Python C voilation rule we want to understand:
Previous Workflow (Broken)
New Workflow (Fixed)
Checklist
References
https://docs.python.org/3/c-api/exceptions.html
Test & Review