Skip to content
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

Add support for setting and retrieving exception cause #1679

Merged
merged 1 commit into from Jun 24, 2021

Conversation

messense
Copy link
Member

No description provided.

@messense messense marked this pull request as ready for review June 14, 2021 13:37
Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

👍 I'm in favour of adding this. I have a few design choices I want to ask about:

  • possibly merge set_cause / clear_cause - see below
  • also, these APIs could potentially work with &PyBaseException instead of Self. Would be slightly closer to the underlying C APIs, though might in practice just force users to call .into_instance(py).

I think the answers mostly depend on how you expect users to want to use these APIs? I assume you might have a use case which prompted you to add them.

src/err/mod.rs Outdated Show resolved Hide resolved
@messense
Copy link
Member Author

messense commented Jun 23, 2021

I do have a use case for this, consider that a Rust extension module that accepts and calls a user supplied Python callback, the Python callback can raise many kind of exceptions, but the Rust extension module will wrap them as an unified exception for example RuntimeError, it'd be nice to set the exception cause to whatever exception the Python callback throws so it's easier to debug.

https://github.com/messense/rjsonnet-py/blob/1d57d47863fa47a3bf1bef5562374fa4467731a2/src/lib.rs#L193-L213

@messense messense changed the title Add support for settings and retrieving exception cause Add support for setting and retrieving exception cause Jun 23, 2021
Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

👍 thanks for tweaking, I think we can add this!

@davidhewitt davidhewitt merged commit a02353c into PyO3:main Jun 24, 2021
@messense messense deleted the error-cause branch June 24, 2021 07:00
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.

None yet

2 participants