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.s...@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 <iritkatr...@googlemail.com>
> wrote:
>
>> On Tue, Feb 23, 2021 at 3:49 PM Damian Shaw <damian.peter.s...@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-le...@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/
>
_______________________________________________
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@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/SAKCTYYKXY6XSEN7YOEKIX37VAJL6XMC/
Code of Conduct: http://python.org/psf/codeofconduct/
