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

"TypeError: No constructor defined" discards exception context #3432

Closed
object-Object opened this issue Sep 5, 2023 · 2 comments · Fixed by #3455
Closed

"TypeError: No constructor defined" discards exception context #3432

object-Object opened this issue Sep 5, 2023 · 2 comments · Fixed by #3455
Labels

Comments

@object-Object
Copy link

Bug Description

PyO3 raises TypeError when you try to construct a #[pyclass] without a constructor. The raised exception doesn't have a value for the __context__ attribute, which breaks exception chaining. This is especially problematic when implementing a custom exception from Rust.

/// Default new implementation
unsafe extern "C" fn no_constructor_defined(
_subtype: *mut ffi::PyTypeObject,
_args: *mut ffi::PyObject,
_kwds: *mut ffi::PyObject,
) -> *mut ffi::PyObject {
trampoline(|_| {
Err(crate::exceptions::PyTypeError::new_err(
"No constructor defined",
))
})
}

Steps to Reproduce

PyO3 module:

use pyo3::exceptions::PyException;
use pyo3::prelude::*;

#[pyclass(extends=PyException)]
pub struct RustException {}

#[pymodule]
fn pyo3_init_bug(_py: Python, m: &PyModule) -> PyResult<()> {
    m.add_class::<RustException>()?;
    Ok(())
}

Test cases:

from pyo3_init_bug import RustException
import pytest
from traceback import format_exception


class PythonException(Exception):
    def __new__(cls):
        raise TypeError("No constructor defined")


@pytest.mark.parametrize("use_rust_exception", [True, False])
def test_broken_exception(use_rust_exception: bool):
    with pytest.raises(Exception) as exc_info:
        try:
            raise ValueError("Original message")
        except Exception:
            if use_rust_exception:
                raise RustException
            else:
                raise PythonException

    print("".join(format_exception(exc_info.value)))
    assert exc_info.value.__context__ is not None

Only the RustException test case fails.

Backtrace

(venv) PS C:\Users\redacted\pyo3_init_bug> pytest -rP                
=================================================================== test session starts ====================================================================
platform win32 -- Python 3.11.0, pytest-7.4.1, pluggy-1.3.0
rootdir: C:\Users\redacted\pyo3_init_bug
collected 2 items

test_broken_exception.py F.                                                                                                                           [100%]

========================================================================= FAILURES ========================================================================= 
_______________________________________________________________ test_broken_exception[True] ________________________________________________________________ 

use_rust_exception = True

    @pytest.mark.parametrize("use_rust_exception", [True, False])
    def test_broken_exception(use_rust_exception: bool):
        with pytest.raises(Exception) as exc_info:
            try:
                raise ValueError("Original message")
            except Exception:
                if use_rust_exception:
                    raise RustException
                else:
                    raise PythonException

        print("".join(format_exception(exc_info.value)))
>       assert exc_info.value.__context__ is not None
E       AssertionError: assert None is not None
E        +  where None = TypeError('No constructor defined').__context__
E        +    where TypeError('No constructor defined') = <ExceptionInfo TypeError('No constructor defined') tblen=1>.value

test_broken_exception.py:23: AssertionError
------------------------------------------------------------------- Captured stdout call ------------------------------------------------------------------- 
Traceback (most recent call last):
  File "C:\Users\redacted\pyo3_init_bug\test_broken_exception.py", line 18, in test_broken_exception
    raise RustException
TypeError: No constructor defined

========================================================================== PASSES ========================================================================== 
_______________________________________________________________ test_broken_exception[False] _______________________________________________________________ 
------------------------------------------------------------------- Captured stdout call ------------------------------------------------------------------- 
Traceback (most recent call last):
  File "C:\Users\redacted\pyo3_init_bug\test_broken_exception.py", line 15, in test_broken_exception
    raise ValueError("Original message")
ValueError: Original message

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "C:\Users\redacted\pyo3_init_bug\test_broken_exception.py", line 20, in test_broken_exception
    raise PythonException
  File "C:\Users\redacted\pyo3_init_bug\test_broken_exception.py", line 8, in __new__
    raise TypeError("No constructor defined")
TypeError: No constructor defined

=============================================================== 1 failed, 1 passed in 0.16s ================================================================

Your operating system and version

Windows 11

Your Python version (python --version)

Python 3.11.0

Your Rust version (rustc --version)

rustc 1.70.0 (90c541806 2023-05-31)

Your PyO3 version

0.19.0

How did you install python? Did you use a virtualenv?

Standard Windows installer. I used a venv.

Additional Info

I ran into this issue with Pydantic's ValidationError type, which doesn't have a constructor. This issue has more details on that particular use case: pypa/hatch#959

@mejrs
Copy link
Member

mejrs commented Sep 5, 2023

We could fix this by setting the Py_TPFLAGS_DISALLOW_INSTANTIATION flag on RustException's type object, with which it will be handled like this:

Traceback (most recent call last):
  File "c:\Users\bruno\Python\Misc\test9.py", line 9, in test_broken_exception
    raise ValueError("Original message")
ValueError: Original message

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "c:\Users\bruno\Python\Misc\test9.py", line 12, in test_broken_exception
    raise RustException
TypeError: cannot create 'builtins.RustException' instances

Unfortunately this only works on Python 3.10 and up :(.

@davidhewitt
Copy link
Member

Potentially we can use that on 3.10 and up and reverse engineer a similar implementation for the older pythons? I think moving to a solution managed by the runtime is much cleaner overall.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants