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

Improve Python <-> .NET exception integration #1134

Merged
merged 42 commits into from Jun 1, 2021

Conversation

lostmsu
Copy link
Member

@lostmsu lostmsu commented May 5, 2020

What does this implement/fix? Explain your changes.

  1. .NET exceptions bubbled through Python stack back into .NET are preserved (before: PythonException was thrown instead).
  2. Codecs can now encode and decode exceptions
  3. .NET InnerException property is populated from Python's __cause__ attribute.
  4. Python exception stack is added to .NET exception stack
  5. Internal: simplified memory management in PythonException class. Instead of working with raw pointers, uses PyObject instances.

Breaking

  1. Traceback, PyVal and PyType are now stored and returned as PyObjects.
  2. PythonException.Restore no longer nulls out PythonException instance.
  3. PythonException instances are no longer disposable. You can still dispose .Type, .Value and .Traceback, but it is not recommended, as they may be shared with other instances.

Does this close any currently open issues?

#893, #1098

Checklist

Check all those that are applicable and complete.

  • Make sure to include one or more tests for your change
  • If an enhancement PR, please create docs and at best an example
  • Add yourself to AUTHORS
  • Updated the CHANGELOG

lostmsu added 10 commits May 13, 2020 14:03
…ET interop

New method ThrowLastAsClrException should be used everywhere instead of obsolete PythonException constructor. It automatically restores .NET exceptions, and applies codecs for Python exceptions.

Traceback, PyVal and PyType are now stored and returned as PyObjects.

PythonException now has InnerException set from its cause (e.g. __cause__ in Python, if any).

PythonException.Restore no longer clears the exception instance.

All helper methods were removed from public API surface.
The new type indicates parameters of C API functions,
that steal references to passed objects.
Removed private fields, apart from ones returned by `PyErr_Fetch`.
Corresponding property values are now generated on demand.

Added FetchCurrent*Raw for internal consumption.

`PythonException.Type` is now of type `PyType`.

Use C API functions `PyException_GetCause` and `PyException_GetTraceback` instead of trying to read via attributes by name.

`PythonException` instances are no longer disposable. You can still dispose `.Type`, `.Value` and `.Traceback`, but it is not recommended, as they may be shared with other instances.
@lostmsu
Copy link
Member Author

lostmsu commented Apr 9, 2021

Damn CI failures do not reproduce locally :/

@lostmsu lostmsu marked this pull request as ready for review May 23, 2021 09:01
@lostmsu lostmsu requested a review from filmor May 23, 2021 09:02
@lostmsu lostmsu force-pushed the PR/ExceptionsImprovement branch 2 times, most recently from f812ec7 to 651d145 Compare May 29, 2021 22:24
@lostmsu lostmsu merged commit 7d8f754 into pythonnet:master Jun 1, 2021
@Felk
Copy link

Felk commented Jun 13, 2021

Hello, given that Exceptions is not public anymore, what is the recommended way of catching specific python exceptions? Previously I used this:

catch (PythonException ex) when (ex.PyType == Exceptions.KeyError)

@lostmsu
Copy link
Member Author

lostmsu commented Jun 14, 2021

@Felk You can get KeyError from builtins module.

@Felk
Copy link

Felk commented Jun 14, 2021

Thank you, this is how I got it to work now:

dynamic builtins = Py.Import("builtins");
try
{
    // ...
}
catch (PythonException ex) when (ex.Type.Equals(builtins.KeyError))
{
    // ...
}

@lostmsu
Copy link
Member Author

lostmsu commented Jun 14, 2021

@Felk comparing type handles will be slightly faster

@Felk
Copy link

Felk commented Jun 14, 2021

@lostmsu can you give me an example of what you mean?

@lostmsu
Copy link
Member Author

lostmsu commented Jun 14, 2021

@Felk ex.Type.Handle == builtins.KeyError.Handle. But in you scenario that should not matter, cause generally exception handling code won't be performance critical part.

If you really cared, you'd also cache builtins.KeyError into a variable of type PyObject to avoid dynamic dispatch.

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

3 participants