Skip to content

gh-99553: add tests for ExceptionGroup wrapping #99615

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 11, 2023
Merged

gh-99553: add tests for ExceptionGroup wrapping #99615

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4b9e01a
gh-99553: clarify doc and add tests
Zac-HD Nov 20, 2022
29b9f27
Update Doc/library/exceptions.rst
Zac-HD Nov 20, 2022
07bdfea
Update Doc/library/exceptions.rst
Zac-HD Nov 21, 2022
9bdb9c6
undo docs changes
hauntsaninja Apr 11, 2023
774b0b8
Merge branch 'main' into zac/custom-exceptiongroup-docs-tests
hauntsaninja Apr 11, 2023
043d002
Merge branch 'main' into zac/custom-exceptiongroup-docs-tests
hauntsaninja Apr 11, 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
7 changes: 4 additions & 3 deletions Doc/library/exceptions.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -965,9 +965,10 @@ their subgroups based on the types of the contained exceptions.
def derive(self, excs):
return Errors(excs, self.exit_code)

Like :exc:`ExceptionGroup`, any subclass of :exc:`BaseExceptionGroup` which
is also a subclass of :exc:`Exception` can only wrap instances of
:exc:`Exception`.
A subclass of :exc:`BaseExceptionGroup` that extends :exc:`Exception`
cannot wrap :exc:`BaseException`\ s, as is the case with :exc:`ExceptionGroup`.
However, there is no further restriction: a :exc:`BaseExceptionGroup` subclass
which extends :exc:`KeyError` could wrap any :exc:`Exception`.
Copy link
Member

Choose a reason for hiding this comment

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

I think this is a strange thing to say in a doc, because why would you expect it to not wrap any Exception?

Maybe instead of the second sentence we could just explain the motivation for the restriction: “This ensures that wrapped BaseExceptions are not caught by ‘except Exception’.”

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why would you expect it to not wrap any Exception?

Suppose we have class MyEG(BaseExceptionGroup, X):, where except X: will catch such a group because it extends X. We disallow wrapping BaseExceptions in an ExceptionGroup because they would not be caught by except Exception: if unwrapped.

However this logic is not applied consistently: if X is e.g. KeyError, then except KeyError: can indeed catch MyEG("", [ValueError()]). I find this inconsistency surprising, which is why I commented on #99572 and then opened this PR!

(I'm not trying to reopen the design discussion, just trying to explain my perspective)

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I see. Well yes, the Exception case is different - people use ‘expect Exception’ as ‘catch everything except fatal errors’.


.. versionadded:: 3.11

Expand Down
14 changes: 14 additions & 0 deletions Lib/test/test_exception_group.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,20 @@ class MyEG(BaseExceptionGroup, ValueError):
with self.assertRaisesRegex(TypeError, msg):
MyEG("eg", [ValueError(12), KeyboardInterrupt(42)])

def test_EG_and_specific_subclass_can_wrap_any_nonbase_exception(self):
class MyEG(ExceptionGroup, ValueError):
pass

# The restriction is specific to Exception, not "the other base class"
MyEG("eg", [ValueError(12), Exception()])

def test_BEG_and_specific_subclass_can_wrap_any_nonbase_exception(self):
class MyEG(BaseExceptionGroup, ValueError):
pass

# The restriction is specific to Exception, not "the other base class"
MyEG("eg", [ValueError(12), Exception()])


def test_BEG_subclass_wraps_anything(self):
class MyBEG(BaseExceptionGroup):
Expand Down