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

Raise the "original" exception from Interpreter.run()? #43

Open
ericsnowcurrently opened this issue Sep 10, 2018 · 0 comments
Open

Raise the "original" exception from Interpreter.run()? #43

ericsnowcurrently opened this issue Sep 10, 2018 · 0 comments
Assignees
Labels
complexity: medium moderately hard to figure out P2 low priority size: medium requires a moderate amount of changes type: enhancement X [cross-interpreter] e.g. channels

Comments

@ericsnowcurrently
Copy link
Owner

ericsnowcurrently commented Sep 10, 2018

(Note that this is more of a future enhancement than a blocker for PEP 554, etc.)

from PEP 554:

Regarding uncaught exceptions in Interpreter.run(), we noted that they are "effectively" propagated into the code where run() was called. To prevent leaking exceptions (and tracebacks) between interpreters, we create a surrogate of the exception and its traceback (see traceback.TracebackException), set it to __cause__ on a new RunFailedError, and raise that.

Raising (a proxy of) the exception directly is problematic since it's harder to distinguish between an error in the run() call and an uncaught exception from the subinterpreter.


Currently uncaught exceptions from Interpreter.run() are stringified and then wrapped in a RunFailedError before being raised in the calling interpreter. With that you can always know if an uncaught exception came from a subinterpreter. On top of that, in #17 we're addressing the issue of lost information for the propagated exception (e.g. type, attrs, traceback, __cause__). Once that is resolved the boundary between the two interpreters will be clear and the traceback informative about the course of events.

At that point, however, you still always have to deal with RunFailedError if you otherwise don't care that the exception came from another interpreter (which I expect most folks won't). This leads to annoying boilerplate code. In #17 the example demonstrates some of the clunkiness.

We can address this, but must remember there's sometimes value to knowing that an exception came out of another interpreter. So we shouldn't just throw that information away either. That said, there's perhaps even more value to not requiring boilerplate code.

Here are some options for handling the original exception more easily:

A. raise the propagated exception directly in the calling interpreter
B. like A, but set __cause__ on the exception to the RunFailedError
C. like B, but set __propagated__ instead
D. raise RunFailedError and re-raise original via RunFailedError.raise() (or .reraise())
E. provide a context manager that re-raises the "original" exception

Contrast the example in #17 with the alternatives presented above:

A:

>>> interp.run(script)
Traceback (most recent call last):
  File "<stdin>", line 3
    raise SpamError('eggs')
  File "<stdin>", line 7
    interp.run(script)
SpamError: eggs

You don't have to treat the unhandled exception specially, but you lose context. Perhaps you could get it back by walking the traceback looking for the point that interp.run() is called (there could even be a helper for that). However, at the very least the traceback won't be nearly as useful.

B:

>>> interp.run(script)
Traceback (most recent call last):
  File "<stdin>", line 3
    raise SpamError('eggs')
SpamError: eggs

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "<stdin>", line 7
    interp.run(script)
RunFailedError: SpamError: eggs

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "<stdin>", line 3
    raise SpamError('eggs')
SpamError: eggs
>>> try:
...     interp.run(script)  # or anywhere else up the call chain...
... except Exception as exc:
...     if exc.__cause__ is None or type(exc.__cause__) is not RunFailedError:
...         raise
...     # handle it

Then you get the best of both worlds (at the expense of an inversion of the exception chain):

C:

Like B, but doesn't reverse the meaning of __cause__. However, either the traceback machinery would have to know how to deal with __propagated__ or we lose the information.

D:

...

E:

...

@ericsnowcurrently ericsnowcurrently added X [isolation] improved interpreter isolation X [cross-interpreter] e.g. channels P1 medium priority type: enhancement size: medium requires a moderate amount of changes complexity: medium moderately hard to figure out and removed X [isolation] improved interpreter isolation labels Sep 10, 2018
@ericsnowcurrently ericsnowcurrently self-assigned this May 7, 2019
@ericsnowcurrently ericsnowcurrently added P2 low priority and removed P1 medium priority labels May 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complexity: medium moderately hard to figure out P2 low priority size: medium requires a moderate amount of changes type: enhancement X [cross-interpreter] e.g. channels
Projects
None yet
Development

No branches or pull requests

1 participant