Skip to content
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.

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

Fixed bare raise and exception chaining when a handler raises an exception #71

Merged
merged 5 commits into from
Aug 9, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
4 changes: 4 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ This library adheres to `Semantic Versioning 2.0 <http://semver.org/>`_.
- `catch()` now raises a `TypeError` if passed an async exception handler instead of
just giving a `RuntimeWarning` about the coroutine never being awaited. (#66, PR by
John Litborn)
- Fixed plain ``raise`` statement in an exception handler callback to work like a
``raise`` in an ``except*`` block
- Fixed new exception group not being chained to the original exception when raising an
exception group from exceptions raised in handler callbacks

**1.1.2**

Expand Down
7 changes: 5 additions & 2 deletions src/exceptiongroup/_catch.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def __exit__(
elif unhandled is None:
return True
else:
raise unhandled from None
raise unhandled from exc

return False

Expand All @@ -50,7 +50,10 @@ def handle_exception(self, exc: BaseException) -> BaseException | None:
matched, excgroup = excgroup.split(exc_types)
if matched:
try:
result = handler(matched)
try:
raise matched
except BaseExceptionGroup:
result = handler(matched)
except BaseException as new_exc:
new_exceptions.append(new_exc)
else:
Expand Down
15 changes: 15 additions & 0 deletions tests/test_catch.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,21 @@ def handler(exc):
raise ExceptionGroup("booboo", [ValueError("bar")])


def test_catch_handler_reraises():
def handler(exc):
raise

Choose a reason for hiding this comment

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

Can you add a test with raise Exception and make sure the __context__ is set to the split exception group

Copy link
Owner Author

Choose a reason for hiding this comment

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

The __context__ value is the same as __cause__ on py3.11 though?

Copy link
Owner Author

Choose a reason for hiding this comment

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

So it seems that in the backport, __context__ is set to the split exception group which doesn't match the py3.11 test which I just augmented.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I made sure that the back-port works the same way as py3.11 does.

Copy link
Owner Author

Choose a reason for hiding this comment

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

As usual, I misread your comment. I'll make sure to add such a test.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Well, this was interesting. Contrary to my expectations, the exception group in __context__ on py3.11 was a copy of the original, so assert exc.value.__context__ is excgrp failed.

Copy link
Owner Author

@agronholm agronholm Jul 22, 2023

Choose a reason for hiding this comment

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

I'm not sure that it's essential to copy the exception group. I stopped just short of doing that, as didn't think it's worth the effort to change the context to a copy.


with pytest.raises(ExceptionGroup) as exc:
with catch({(ValueError,): handler, (RuntimeError,): lambda eg: None}):
original_exc = ExceptionGroup("booboo", [ValueError("bar"), RuntimeError()])
raise original_exc

assert len(exc.value.exceptions) == 1
assert isinstance(exc.value.exceptions[0], ValueError)
assert str(exc.value.exceptions[0]) == "bar"
assert exc.value.__cause__ is original_exc


def test_catch_subclass():
lookup_errors = []
with catch({LookupError: lookup_errors.append}):
Expand Down