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

Conversation

agronholm
Copy link
Owner

Fixes #70.

@coveralls
Copy link

coveralls commented Jul 16, 2023

Pull Request Test Coverage Report for Build 5810639052

  • 12 of 12 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.03%) to 98.669%

Totals Coverage Status
Change from base Build 5795493105: 0.03%
Covered Lines: 519
Relevant Lines: 526

💛 - Coveralls

@@ -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.

@agronholm agronholm merged commit 8b8791b into main Aug 9, 2023
10 checks passed
@agronholm agronholm deleted the fix-bare-raise branch August 9, 2023 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bare raise in catch behaves differently to bare raise in native except*
3 participants