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

EXPERIMENT: pybind11_tests INTENTIONAL BREAKAGE #3965

Closed
wants to merge 1 commit into from

Conversation

rwgk
Copy link
Collaborator

@rwgk rwgk commented May 21, 2022

Description

Is raise_from in PYBIND11_CATCH_INIT_EXCEPTIONS working as intended?

Suggested changelog entry:

@rwgk
Copy link
Collaborator Author

rwgk commented May 21, 2022

For easy future reference, direct link to the CI results:

https://github.com/pybind/pybind11/actions/runs/2363503482

I downloaded the CI log archive:

https://github.com/pybind/pybind11/suites/6603861876/logs?attempt=1 (logs_20001.zip)

After unzip:

$ grep -r initialization.failed | wc -l
96
$ grep -r pybind11_tests.INTENTIONAL.BREAKAGE

I.e. raise_from in PYBIND11_CATCH_INIT_EXCEPTIONS is not working as intended.

The root cause error message is lost.

@Skylion007 @henryiii @virtuald

I believe for Python >= 3.3, this was unfortunately broken by PR #2112, we all missed it.

Randomly picked example:

2022-05-21T15:45:09.2238950Z   ------ pybind11_tests.cp39-win32.pyd file size: 17401344 (no change)
2022-05-21T15:45:10.2242864Z   ImportError while loading conftest 'D:\a\pybind11\pybind11\tests\conftest.py'.
2022-05-21T15:45:10.2243626Z   ..\..\tests\conftest.py:16: in <module>
2022-05-21T15:45:10.2244045Z       import pybind11_tests  # noqa: F401
2022-05-21T15:45:10.2244580Z   E   ImportError: initialization failed
2022-05-21T15:45:10.2245495Z C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\MSBuild\Microsoft\VC\v160\Microsoft.CppCommon.targets(241,5): error MSB8066: Custom build for 'D:\a\pybind11\pybind11\build\CMakeFiles\36552307feba98ee6f1ba2d538cfd480\pytest.rule;D:\a\pyb

rwgk pushed a commit to rwgk/pybind11 that referenced this pull request May 21, 2022
@Skylion007
Copy link
Collaborator

Skylion007 commented May 21, 2022

To confirm, is it that raise_from(err_already_set, exc_type, message) is broken? We are sure that pytest actually displays the cause portion of the exception object, right? If that is the case, we should add some tests to test_exceptions.py . This suprises me since the nested exception tests seem to show that the other overload (that doesn't use err_already_set) works, I guess we should make it a bit more thorough in our tests.

@rwgk
Copy link
Collaborator Author

rwgk commented May 21, 2022

To confirm, is it that raise_from(err_already_set, exc_type, message) is broken?

I don't know which part is failing. My guess is that it is somehow specific to importing a pybind11 extension.

I guess we should make it a bit more thorough in our tests.

Yes, I'm thinking that, too. Something like a new raise_error_on_import.cpp which we import from test_exceptions.py, then check the exception to be sure it has outer and inner.

@rwgk
Copy link
Collaborator Author

rwgk commented May 30, 2022

Summary:

  • The culprit is pytest!
  • For production situations, this is not a problem.
  • Closing this PR: I don't have a great idea for alerting others to this trap before they fall into it.

Example outputs below:

  • When running with pytest, only the outer exception is shown.

  • When importing the exact same (broken) extension from an interactive Python interpreter the inner and outer exception are shown.

Seems to be exactly like this bug from 10+ years ago:

pytest-dev/pytest#138

I verified conclusively (interactively) that this is still the case with pytest 7.1.2 (latest released).

Once you know it's easy enough to deal with it, but if you don't, it really is a bad trap.

platform linux -- Python 3.9.12, pytest-6.2.3, py-1.10.0, pluggy-0.13.1 -- /usr/bin/python3
Running tests in directory "/usr/local/google/home/rwgk/forked/pybind11/tests":
ImportError while loading conftest '/usr/local/google/home/rwgk/forked/pybind11/tests/conftest.py'.
conftest.py:16: in <module>
    import pybind11_tests  # noqa: F401
E   ImportError: initialization failed
$

$ cd lib
$ python3
Python 3.9.12 (main, Mar 24 2022, 13:02:21)
[GCC 11.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import pybind11_tests
ValueError: pybind11_tests INTENTIONAL BREAKAGE

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: initialization failed
>>>

@rwgk rwgk closed this May 30, 2022
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.

2 participants