This pep sounded kinda scary to me, so I wanted to try it out.

The reference implementation appears to have some bugs in it (around reraise star) , so it's not entirely clear what the expected behavior is supposed to be, but by checking out ffc493b5 I got some OK results.

I had a look at the tempfile example given under the Motivations section, and wanted to see how this would work (as it's the area of code I'm most familiar with).

Would the proposed tempfile change look something like this? (functionally, if not stylistically):

--- a/Lib/tempfile.py
+++ b/Lib/tempfile.py
@@ -819,8 +819,14 @@ def __repr__(self):
     def __enter__(self):
         return self.name
 
-    def __exit__(self, exc, value, tb):
-        self.cleanup()
+    def __exit__(self, exc_cls, exc_value, tb):
+        try:
+            self.cleanup()
+        except Exception as clean_exc:
+            if exc_value is not None:
+                raise ExceptionGroup('Exception occurred during cleanup', [exc_value, clean_exc])
+            else:
+                raise
 
     def cleanup(self):
         if self._finalizer.detach():

If so, then the following code fails to catch the ZeroDivisionError (there is an uncaught exception raised):

    import tempfile, os, pathlib

    def do_some_stuff():
        with tempfile.TemporaryDirectory() as td:
            os.rmdir(td)
            pathlib.Path(td).write_text("Surprise!")
            1/0

    if __name__ == '__main__':
        try:
            do_some_stuff()
        except Exception:
            print("Something went wrong")
        else:
            print("No error")

The output I get:
"""
Traceback (most recent call last):
  File "/home/sstagg/tmp/fuzztest/cpython/td.py", line 7, in do_some_stuff
    1/0
ZeroDivisionError: division by zero

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/sstagg/tmp/fuzztest/cpython/Lib/tempfile.py", line 824, in __exit__
    self.cleanup()
  File "/home/sstagg/tmp/fuzztest/cpython/Lib/tempfile.py", line 833, in cleanup
    self._rmtree(self.name)
  File "/home/sstagg/tmp/fuzztest/cpython/Lib/tempfile.py", line 809, in _rmtree
    _shutil.rmtree(name, onerror=onerror)
  File "/home/sstagg/tmp/fuzztest/cpython/Lib/shutil.py", line 718, in rmtree
    _rmtree_safe_fd(fd, path, onerror)
  File "/home/sstagg/tmp/fuzztest/cpython/Lib/shutil.py", line 631, in _rmtree_safe_fd
    onerror(os.scandir, path, sys.exc_info())
  File "/home/sstagg/tmp/fuzztest/cpython/Lib/shutil.py", line 627, in _rmtree_safe_fd
    with os.scandir(topfd) as scandir_it:
NotADirectoryError: [Errno 20] Not a directory: '/tmp/tmpeedn6r0n'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/sstagg/tmp/fuzztest/cpython/td.py", line 12, in <module>
    do_some_stuff()
  File "/home/sstagg/tmp/fuzztest/cpython/td.py", line 7, in do_some_stuff
    1/0
  File "/home/sstagg/tmp/fuzztest/cpython/Lib/tempfile.py", line 827, in __exit__
    raise ExceptionGroup('Exception occurred during cleanup', [exc_value, clean_exc])
ExceptionGroup: Exception occurred during cleanup
"""

===

So, to catch and handle errors raised from TemporaryDirectory safely, the try-except has to be wrapped in a try-*except block?:


    if __name__ == '__main__':
        try:
            try:
                do_some_stuff()
            except Exception:
                print("Fail Site 1")
        except *NotADirectoryError:
            print("Fail Site 2")
        except *Exception:
            print("Fail Site 3")

In this situation, Sites 2 and 3 are called, but if there is no problem during cleanup then Site 1 is called?

If, instead, the ExceptionGroup is explicitly handled:

    if __name__ == '__main__':
        try:
            do_some_stuff()
        except (Exception, ExceptionGroup):
            print("Fail Site 1")

Then this actually works quite nicely for the 'except Exception' scenario, but is much more complex if you want to catch and handle specific types of exceptions:

    if __name__ == '__main__':
        try:
            do_some_stuff()
        except ExceptionGroup as exc_group:
            zd_errors, others = exc_group(lambda e: isinstance(e, ZeroDivisionError))
            if zd_errors:
                print("Fail Site 1")
            if others:
                raise others
        except ZeroDivisionError:
            print("Fail Site 2")

If the idea is that tempfile.TemporaryDirectory *always* raises an ExceptionGroup (even if there was no cleanup exception) then any code that calls anything that might eventually call TemporaryDirectory will have to be aware that a different type of exception could appear that normal handling doesn't catch (or for /every/ usage of TemporaryDirectory be immediately wrapped in try-*except handling code).

It feels like ever letting an ExceptionGroup unwind more than 1 or two stack frames is super dangerous, as it directly requires all code up the stack to consider this situation and handle it separately from 'normal' exceptions.

It's quite possible that I've completely mis-understood, or skipped over something critical here.  If so, apologies!

Steve


On Tue, Feb 23, 2021 at 8:04 PM Damian Shaw <damian.peter.shaw@gmail.com> wrote:
Hi Irit,

Catching exceptions like this is an extremely common pattern in the real world, e.g. this pattern has over 5 million GitHub matches: https://github.com/search?l=&q=%22except+Exception%22+language%3APython&type=code

How common it is aside there are also many valid use cases for this pattern, e.g. logging a final unhandled exception of a script, catching exceptions inside an orchestration framework, exploring code where the list of exceptions is unknown, etc.

A description from the PEP on how to handle this kind of pattern, especially for code that must support multiple versions of Python, would be extremely helpful.

Damian

On Tue, Feb 23, 2021 at 2:35 PM Irit Katriel <iritkatriel@googlemail.com> wrote:
On Tue, Feb 23, 2021 at 3:49 PM Damian Shaw <damian.peter.shaw@gmail.com> wrote:

Firstly, if I have a library which supports multiple versions of Python and I need to catch all standard exceptions, what is considered the best practise after this PEP is introduced?

Currently I might have code like this right now:
try:
    ... # Code
except Exception as e:
    ... # Logic to handle exception


Hi Damian, 

Catching all exceptions in this way is not a common pattern. Typically you have an idea what kind of exceptions you expect to get from an operation, and which of those exceptions you are interested in handling. Most of your try/except blocks will be targeted, like handling KeyError from d[k], an operation that will never raise an ExceptionGroup.

ExceptionGroups will only be raised by specific APIs which will advertise themselves as such. We don't expect that there will be a need for blanket migrations of "except T" to "except (T, ExceptionGroup)" to "except *T".

Irit


_______________________________________________
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-leave@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at https://mail.python.org/archives/list/python-dev@python.org/message/XAPO475TVFICD77M3VRLC7OKA5O7WTOT/
Code of Conduct: http://python.org/psf/codeofconduct/