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

Fix #63 #66, supersede #67: Wrap Python exceptions into custom InternalPythonError #79

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

qwenger
Copy link
Collaborator

@qwenger qwenger commented Apr 8, 2022

Fix #63, fix #66.

Supersedes #67. As proposed over there, this stores Python exception into a JS object rather than globally.

On the QuickJS side, the object uses as an opaque pointer to store the Python exception.
On the JS side, the object has constructor InternalPythonError that extends InternalError and has the message property set to Python call failed.
On the Python side, the exception type is JSPythonException, a subclass of JSException. Its __cause__ is set to the exception originally triggered, which is a pretty clean way to keep track of it.

The code has a few "not so nice" areas, but for which I did not find anything nicer:

  • The finalizer (that decref's the stored Python exception when destroying the wrapping InternalPythonError) has to do a special check of the thread state, because it is both called from inside prepare...end_call_js and prepare...end_call_python.
  • A custom constructor for InternalPythonError was needed, because I did not see a way to use the parent constructor (InternalError).
  • To get a proper stack for the JS error, a standard InternalError is first raised, then catched directly and its stack property transferred over to the InternalPythonError.
  • Similarly, the Python exception is fetched and restored to set its cause (but that's pretty standard).

…ns into custom InternalPythonError object extending InternalError.
@qwenger
Copy link
Collaborator Author

qwenger commented Apr 8, 2022

The check_memory.py script does not notice any issue (other than the spurious ones from threading, which also show up in master).

@qwenger
Copy link
Collaborator Author

qwenger commented Apr 9, 2022

One question is also whether we would like to expose the JS Internal(Python)Error via the JS(Python)Exception on the Python side, e.g. as an Object bound to JS(Python)Exception.raw.

Or better yet, make JSException subclass of Object and directly encapsulate the JS error inside it?

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.

Segfault in awaited Python call More verbose exceptions in callables
1 participant