Skip to content

gh-103791: Make contextlib.suppress also act on exceptions within an ExceptionGroup #103792

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Apr 24, 2023
+73 −22
Merged

gh-103791: Make contextlib.suppress also act on exceptions within an ExceptionGroup #103792

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7bc3c15
gh-103791: Make contextlib.suppress also act on exceptions within an …
ambv Apr 24, 2023
08fa06c
Remove unnecessary `print()` calls
ambv Apr 24, 2023
be30df6
Fix docstring typo
ambv Apr 24, 2023
acf95f6
Don't chain with the old exception: split already has the correct TB
ambv Apr 24, 2023
503d9f8
Documentation and Blurb
ambv Apr 24, 2023
a43ffe4
Apply suggestions from code review
ambv Apr 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Don't chain with the old exception: split already has the correct TB
  • Loading branch information
@ambv
ambv committed Apr 24, 2023
commit acf95f6d3b25a62dd7933264d4dbeeb2fcab368f
2 changes: 1 addition & 1 deletion Lib/contextlib.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ def __exit__(self, exctype, excinst, exctb):
match, rest = excinst.split(self._exceptions)
if rest is None:
return True
raise rest from excinst
raise rest
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't ideal as it makes the exception group's own traceback include the def __exit__(...) frame. For example:

  File "/Volumes/RAMDisk/cpython/Lib/test/test_contextlib.py", line 1220, in test_exception_groups
    with suppress(ValueError):
  File "/Volumes/RAMDisk/cpython/Lib/contextlib.py", line 454, in __exit__
    raise rest from excinst
  File "/Volumes/RAMDisk/cpython/Lib/test/test_contextlib.py", line 1221, in test_exception_groups
    raise eg_all()

in case of the newly added test.

This isn't a big problem because:

  • the group's member exceptions contain pristine tracebacks, only the group itself contains extra frames; and
  • the raise of the original group is still included in the traceback because we used .split() to create the new object.

Ideally, we wouldn't need this. However, the API of __exit__ makes it impossible to replace the ExceptionGroup instance with another one, while ExceptionGroup itself makes its exceptions read-only.

return False


Expand Down