Raising ExceptionGroup when leaving ExitStack in case of exceptions in callbacks

Ideas
1

Now Python >= 3.11 has ExceptionGroup, what do you think of raising such an exception
when leaving an ExitStack in case of errors in callbacks ?

This would allow someone to get all exceptions, not only the last one.

Example of current implementation:

from contextlib import ExitStack
def fail1():
    raise RuntimeError("fail1")
def fail2():
    raise RuntimeError("fail2")
with ExitStack() as es:
  es.callback(fail1)
  es.callback(fail2)

Results in:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/matias/miniconda3/envs/bec/lib/python3.11/contextlib.py", line 603, in __exit__
    raise exc_details[1]
  File "/home/matias/miniconda3/envs/bec/lib/python3.11/contextlib.py", line 587, in __exit__
    if cb(*exc_details):
       ^^^^^^^^^^^^^^^^
  File "/home/matias/miniconda3/envs/bec/lib/python3.11/contextlib.py", line 469, in _exit_wrapper
    callback(*args, **kwds)
  File "<stdin>", line 2, in fail1
RuntimeError: fail1

There is no way to know an error occured while calling fail2

With the proposed change, the code above would result in:

 + Exception Group Traceback (most recent call last):
  |   File "<stdin>", line 1, in <module>
  |   File "/home/matias/miniconda3/envs/bec/lib/python3.11/contextlib.py", line 607, in __exit__
  |     raise ExceptionGroup("Exceptions occured in ExitStack callbacks", excs)
  | ExceptionGroup: Exceptions occured in ExitStack callbacks (2 sub-exceptions)
  +-+---------------- 1 ----------------
    | Traceback (most recent call last):
    |   File "/home/matias/miniconda3/envs/bec/lib/python3.11/contextlib.py", line 587, in __exit__
    |     if cb(*exc_details):
    |        ^^^^^^^^^^^^^^^^
    |   File "/home/matias/miniconda3/envs/bec/lib/python3.11/contextlib.py", line 469, in _exit_wrapper
    |     callback(*args, **kwds)
    |   File "<stdin>", line 2, in fail2
    | RuntimeError: fail2
    +---------------- 2 ----------------
    | Traceback (most recent call last):
    |   File "/home/matias/miniconda3/envs/bec/lib/python3.11/contextlib.py", line 587, in __exit__
    |     if cb(*exc_details):
    |        ^^^^^^^^^^^^^^^^
    |   File "/home/matias/miniconda3/envs/bec/lib/python3.11/contextlib.py", line 469, in _exit_wrapper
    |     callback(*args, **kwds)
    |   File "<stdin>", line 2, in fail1
    | RuntimeError: fail1
    +------------------------------------

I find it much better. What do you think ?

2

This is backwards incompatible, but I think adding this behind an option is a good idea. Not sure if there is an existing deprecation behavior for changing the default of such an option.

3

Thanks for the quick comment !

I agree it is backwards incompatible, however for 1 exception only I think it is ok to raise the exception like today - so, for Python <3.11 it could be “one exception” always, like now, and for Python >= 3.11 then it could be one exception or an ExceptionGroup, which is just a normal exception after all… So is it really a problem ? I would prefer that compared to adding an option (just my opinion)

4

… an ExceptionGroup, which is just a normal exception after all… So is it really a problem ?

It is a problem - except RuntimeError would stop working for your example above.

5

Maybe it should be available as a flag?

6

Hm, if it’s currently really always a RuntimeError, it might be fine to turn it into a class RuntimErrorGroup(RuntimeError, ExceptionGroup). It’s technically still a change, but I don’t think inspecting RuntimeError is a very common situation.

7

The RuntimeErrors mentioned here are all from user code. ExitStack just re-raises the last exception it encounters which could be anything.

However, there’s a bug here: ExitStack tries very hard to set the __context__ of the exception correctly to report all of the exceptions, but it’s been broken since sometime after Python 3.8:

$ python3.9 exitstack_exc_test.py 
3.9.16
Traceback (most recent call last):
  File "/home/zach/code/github.com/python/cpython/exitstack_exc_test.py", line 10, in <module>
    es.callback(fail2)
  File "/usr/local/lib/python3.9/contextlib.py", line 532, in __exit__
    raise exc_details[1]
  File "/usr/local/lib/python3.9/contextlib.py", line 517, in __exit__
    if cb(*exc_details):
  File "/usr/local/lib/python3.9/contextlib.py", line 405, in _exit_wrapper
    callback(*args, **kwds)
  File "/home/zach/code/github.com/python/cpython/exitstack_exc_test.py", line 5, in fail1
    raise RuntimeError("fail1")
RuntimeError: fail1
$ python3.8 exitstack_exc_test.py 
3.8.14
Traceback (most recent call last):
  File "/usr/local/lib/python3.8/contextlib.py", line 510, in __exit__
    if cb(*exc_details):
  File "/usr/local/lib/python3.8/contextlib.py", line 382, in _exit_wrapper
    callback(*args, **kwds)
  File "exitstack_exc_test.py", line 7, in fail2
    raise RuntimeError("fail2")
RuntimeError: fail2

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "exitstack_exc_test.py", line 10, in <module>
    es.callback(fail2)
  File "/usr/local/lib/python3.8/contextlib.py", line 525, in __exit__
    raise exc_details[1]
  File "/usr/local/lib/python3.8/contextlib.py", line 510, in __exit__
    if cb(*exc_details):
  File "/usr/local/lib/python3.8/contextlib.py", line 382, in _exit_wrapper
    callback(*args, **kwds)
  File "exitstack_exc_test.py", line 5, in fail1
    raise RuntimeError("fail1")
RuntimeError: fail1
8

Turns out the bug is already reported as gh-102201.

9

Thanks @zware for having had a look and finding out it is a bug.

However, the 3.8 implementation gives a traceback saying “During handling of the above
exception, another exception occured” , which is not really what happens in the case of
multiple failing callbacks in ExitStack.

Indeed, the different failures are independent ; I find it a perfect use case of an
ExceptionGroup (to my understanding of ExceptionGroup, which was initially proposed
thought from “async”).

To be honest I didn’t understand the __context__ manipulation in ExitStack code. I would then propose to add a boolean optional keyword argument to ExitStack(), to indicate
how errors should be propagated, e.g.:

with ExitStack(raise_exceptiongroup=True) as es:
    ...

The code above would always raise an exception group in case of error, reporting all errors. If not set (by default), could be current behaviour (last exception). I think it would address comment by @iritkatriel

I think the ExitStack could be simplified to not change __context__.

10

Well, that’s kind of a matter of perspective :slight_smile:. ExitStack is attempting to act just like a series of nested with statements, where the 3.8 ExitStack behavior is indeed what you get from more recent Pythons:

class badcontext:
    def __init__(self, i):
        self.i = i

    def __enter__(self):
        return self

    def __exit__(self, *exc_details):
        raise RuntimeError(f'fail{self.i}')

with badcontext(1):
    with badcontext(2):
        pass

$ python3.9 tmp/exitstack_test.py 
Traceback (most recent call last):
  File "/home/zach/tmp/exitstack_test.py", line 13, in <module>
    pass
  File "/home/zach/tmp/exitstack_test.py", line 9, in __exit__
    raise RuntimeError(f'fail{self.i}')
RuntimeError: fail2

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/zach/tmp/exitstack_test.py", line 13, in <module>
    pass
  File "/home/zach/tmp/exitstack_test.py", line 9, in __exit__
    raise RuntimeError(f'fail{self.i}')
RuntimeError: fail1

Also note that the exception from the first-called callback is in fact passed into the next callback, so the outer exception really is called in the context of the inner one.

All that said, I think I could support adding an API to get an ExceptionGroup back from ExitStack (or a subclass for that purpose). I don’t think it could ever become the default, though, as that would completely break the correlation to a series of nested with statements; you wouldn’t be able to rewrite an ExitStack that raises ExceptionGroup as nested withs without also rewriting the context managers used.