agronholm / exceptiongroup

Backport of PEP 654 (exception groups)
Other
42 stars 20 forks source link

test_catch_handler_raises regressed on 3.11.4 #64

Closed mweinelt closed 1 year ago

mweinelt commented 1 year ago

We are seeing a test regression on 1.1.1 after upgrading Python from 3.11.3 to 3.11.4.


exceptiongroup> __________________________ test_catch_handler_raises ___________________________
exceptiongroup>   + Exception Group Traceback (most recent call last):
exceptiongroup>   |   File "/build/source/tests/test_catch_py311.py", line 127, in test_catch_handler_raises
exceptiongroup>   |     raise ExceptionGroup("booboo", [ValueError("bar")])
exceptiongroup>   | ExceptionGroup: booboo (1 sub-exception)
exceptiongroup>   +-+---------------- 1 ----------------
exceptiongroup>     | ValueError: bar
exceptiongroup>     +------------------------------------
exceptiongroup> 
exceptiongroup> During handling of the above exception, another exception occurred:
exceptiongroup> tests/test_catch_py311.py:129: in test_catch_handler_raises
exceptiongroup>     raise RuntimeError("new")
exceptiongroup> E   RuntimeError: new
agronholm commented 1 year ago

@Zac-HD I think this might warrant a major version bump. Thoughts? CPython did this in a patch release, but they break backwards compat all the time anyway.

Zac-HD commented 1 year ago

That's https://github.com/python/cpython/issues/103590, right?

I think it's OK for the backport to treat this as a minor release, since we're "just" restoring compatibility with the CPython behavior.

However, I think that this maybe shouldn't have been changed upstream - the previous behavior was good and if the PEP diverged we should ignore the PEP. APIs which sometimes but not always raise ExceptionGroup are really hard to call with proper error handling... I'll try to do some experiments and post something tomorrow.

Zac-HD commented 1 year ago

Welp, playing around with some alternatives the old and new behaviors upstream seem about equivalent to me, since except* makes it much easier to handle naked-or-grouped exceptions. I still prefer the old always-a-group behavior, which couldn't lead to surprises from rarely-grouped exceptions you didn't realize could happen at the same time, but it doesn't seem worth arguing about that now.

Let's release as a minor version and move on.

agronholm commented 1 year ago

Let's release as a minor version and move on.

I actually made a patch release. Hopefully not an issue.

mcepl commented 1 year ago

Unfortunately, it doesn't seem to be fixed. Even with 1.1.2 I get this with Python 3.11.4 (packaging this module for openSUSE/Factory and it broke our staging area for 3.11.4 upgrade):

[   20s] =================================== FAILURES ===================================
[   20s] __________________________ test_catch_handler_raises ___________________________
[   20s]   + Exception Group Traceback (most recent call last):
[   20s]   |   File "/usr/lib/python3.11/site-packages/_pytest/runner.py", line 341, in from_call
[   20s]   |     result: Optional[TResult] = func()
[   20s]   |                                 ^^^^^^
[   20s]   |   File "/usr/lib/python3.11/site-packages/_pytest/runner.py", line 262, in <lambda>
[   20s]   |     lambda: ihook(item=item, **kwds), when=when, reraise=reraise
[   20s]   |             ^^^^^^^^^^^^^^^^^^^^^^^^
[   20s]   |   File "/usr/lib/python3.11/site-packages/pluggy/_hooks.py", line 265, in __call__
[   20s]   |     return self._hookexec(self.name, self.get_hookimpls(), kwargs, firstresult)
[   20s]   |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[   20s]   |   File "/usr/lib/python3.11/site-packages/pluggy/_manager.py", line 80, in _hookexec
[   20s]   |     return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
[   20s]   |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[   20s]   |   File "/usr/lib/python3.11/site-packages/pluggy/_callers.py", line 60, in _multicall
[   20s]   |     return outcome.get_result()
[   20s]   |            ^^^^^^^^^^^^^^^^^^^^
[   20s]   |   File "/usr/lib/python3.11/site-packages/pluggy/_result.py", line 60, in get_result
[   20s]   |     raise ex[1].with_traceback(ex[2])
[   20s]   |   File "/usr/lib/python3.11/site-packages/pluggy/_callers.py", line 39, in _multicall
[   20s]   |     res = hook_impl.function(*args)
[   20s]   |           ^^^^^^^^^^^^^^^^^^^^^^^^^
[   20s]   |   File "/usr/lib/python3.11/site-packages/_pytest/runner.py", line 177, in pytest_runtest_call
[   20s]   |     raise e
[   20s]   |   File "/usr/lib/python3.11/site-packages/_pytest/runner.py", line 169, in pytest_runtest_call
[   20s]   |     item.runtest()
[   20s]   |   File "/usr/lib/python3.11/site-packages/_pytest/python.py", line 1799, in runtest
[   20s]   |     self.ihook.pytest_pyfunc_call(pyfuncitem=self)
[   20s]   |   File "/usr/lib/python3.11/site-packages/pluggy/_hooks.py", line 265, in __call__
[   20s]   |     return self._hookexec(self.name, self.get_hookimpls(), kwargs, firstresult)
[   20s]   |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[   20s]   |   File "/usr/lib/python3.11/site-packages/pluggy/_manager.py", line 80, in _hookexec
[   20s]   |     return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
[   20s]   |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[   20s]   |   File "/usr/lib/python3.11/site-packages/pluggy/_callers.py", line 60, in _multicall
[   20s]   |     return outcome.get_result()
[   20s]   |            ^^^^^^^^^^^^^^^^^^^^
[   20s]   |   File "/usr/lib/python3.11/site-packages/pluggy/_result.py", line 60, in get_result
[   20s]   |     raise ex[1].with_traceback(ex[2])
[   20s]   |   File "/usr/lib/python3.11/site-packages/pluggy/_callers.py", line 39, in _multicall
[   20s]   |     res = hook_impl.function(*args)
[   20s]   |           ^^^^^^^^^^^^^^^^^^^^^^^^^
[   20s]   |   File "/usr/lib/python3.11/site-packages/_pytest/python.py", line 194, in pytest_pyfunc_call
[   20s]   |     result = testfunction(**testargs)
[   20s]   |              ^^^^^^^^^^^^^^^^^^^^^^^^
[   20s]   | ExceptionGroup:  (1 sub-exception)
[   20s]   +-+---------------- 1 ----------------
[   20s]     | Exception Group Traceback (most recent call last):
[   20s]     |   File "/home/abuild/rpmbuild/BUILD/exceptiongroup-1.1.2/tests/test_catch_py311.py", line 127, in test_catch_handler_raises
[   20s]     |     raise ExceptionGroup("booboo", [ValueError("bar")])
[   20s]     | ExceptionGroup: booboo (1 sub-exception)
[   20s]     +-+---------------- 1 ----------------
[   20s]       | ValueError: bar
[   20s]       +------------------------------------
[   20s]     | 
[   20s]     | During handling of the above exception, another exception occurred:
[   20s]     | 
[   20s]     | Traceback (most recent call last):
[   20s]     |   File "/home/abuild/rpmbuild/BUILD/exceptiongroup-1.1.2/tests/test_catch_py311.py", line 129, in test_catch_handler_raises
[   20s]     |     raise RuntimeError("new")
[   20s]     | RuntimeError: new
[   20s] =========================== short test summary info ============================
[   20s] SKIPPED [2] tests/test_formatting.py:221: No patching is done on Python >= 3.11
[   20s] SKIPPED [2] tests/test_formatting.py:341: No patching is done on Python >= 3.11
[   20s] SKIPPED [2] tests/test_formatting.py:368: No patching is done on Python >= 3.11
[   20s] SKIPPED [1] tests/test_formatting.py:417: No patching is done on Python >= 3.11
[   20s] SKIPPED [2] tests/test_formatting.py:462: only works if NameError exposes the missing name
[   20s] SKIPPED [2] tests/test_formatting.py:486: only works if AttributeError exposes the missing name
[   20s] SKIPPED [1] tests/test_formatting.py:511: No patching is done on Python >= 3.11
[   20s] =================== 1 failed, 79 passed, 12 skipped in 0.35s ===================
[   20s] error: Bad exit status from /var/tmp/rpm-tmp.9O89y6 (%check)

Complete build log with all details about packages used and steps taken to reproduce.

agronholm commented 1 year ago

So why is it failing for you, but working fine on GitHub CI and for me locally?

agronholm commented 1 year ago

I should point out that the test that fails for you does not even involve code from this project at all, it just verifies CPython behavior.

agronholm commented 1 year ago

Your log indicates that the test was run on Python 3.11.3, not 3.11.4.

agronholm commented 1 year ago

I've changed the test to be skipped if sys.version is lower than 3.11.4 now. That should resolve the issue you're seeing.

mcepl commented 1 year ago

Your log indicates that the test was run on Python 3.11.3, not 3.11.4.

You are right, we have still 3.11.3 in Factory, it is exactly this failure which blocks 3.11.4 from getting through the staging area. I will try https://github.com/agronholm/exceptiongroup/commit/452ba0946347b4e0df950763213f162704bc1eed