ValueError and ExtraField exceptions don't specify where they occurred in nested classes

[vedantpuri]

• cattrs version: 22.1.0.dev0
• Python version: 3.7.5
• Operating System: macOS Monterey (12.2.1)

Description

This is referring to the _exceptions attribute on some of the exceptions raised in cattrs when structuring

I was trying to use cattrs inbuilt exceptions for structuring nested attrs classes. I noticed that in this case, KeyError and TypeError exceptions are nested to exactly where the issue occurred, which is great!

But I was expecting the same type of formatting to show up for Value Errors, which is not the case, they don't specify where the issue was caused. Similar issue happens for when there are extra Fields (Also, I think there should be a separate Exception class for that, but I'll make a new issue for that — #237 )

Also, should _exceptions be made public ?

What I Did

POC of issue:

import cattrs
from cattrs.errors import ClassValidationError
from attrs import define, field
from attrs.validators import lt

@define
class _InnerInnerClass:
    f_inner_inner: int


@define
class _InnerClass:
    f_inner_1: int
    f_inner_2: int = field(validator=lt(2))
    inner_inner_props: _InnerInnerClass


@define
class ComposedClass:
    inner_props: _InnerClass
    f: str


def make_my_class(payload, payload_kind):
    print(f"PAYLOAD_TYPE={payload_kind}")
    c = cattrs.GenConverter(forbid_extra_keys=True)
    try:
        c.structure(payload, ComposedClass)
    except ClassValidationError as ex:
        print(ex.__dict__, end="\n\n")
        return
    print("Success", end="\n\n")


if __name__ == "__main__":
    success_payload = {
        "f": "outer_most_string",
        "inner_props": {
            "f_inner_1": 10,
            "f_inner_2": 1,
            "inner_inner_props": {"f_inner_inner": 99},
        },
    }
    make_my_class(success_payload, "SUCCESS")

    value_error_payload = {
        "f": "outer_most_string",
        "inner_props": {
            "f_inner_1": 10,
            "f_inner_2": 12,  # Bad value
            "inner_inner_props": {"f_inner_inner": 99},
        },
    }
    make_my_class(value_error_payload, "BAD_VALUE")


    type_error_payload = {
        "f": "outer_most_string",
        "inner_props": {
            "f_inner_1": 10,
            "f_inner_2": None,  # Bad type
            "inner_inner_props": {"f_inner_inner": 99},
        },
    }
    make_my_class(type_error_payload, "BAD_TYPE")

    # Missing/Bad Key
    key_error_payload = {
        "f": "outer_most_string",
        "inner_props": {
            "f_inner_1": 10,
            "f_inner_2_bad_key": 12,
            "inner_inner_props": {"f_inner_inner": 99},
        },
    }
    make_my_class(key_error_payload, "BAD_KEY")

    extra_fields_payload = {
        "f": "outer_most_string",
        "inner_props": {
            "f_inner_1": 10,
            "f_inner_2": 1,
            "f_extra": 900,
            "inner_inner_props": {"f_inner_inner": 99},
        },
    }
    make_my_class(extra_fields_payload, "EXTRA_FIELDS")

Output:

$ python src/playground.py 
PAYLOAD_TYPE=SUCCESS
Success

PAYLOAD_TYPE=BAD_VALUE
{'cl': <class '__main__.ComposedClass'>, '_message': 'While structuring ComposedClass', '_exceptions': [ValueError("'f_inner_2' must be < 2: 12")], '__note__': None}

PAYLOAD_TYPE=BAD_TYPE
{'cl': <class '__main__.ComposedClass'>, '_message': 'While structuring ComposedClass', '_exceptions': [ClassValidationError('While structuring _InnerClass', [TypeError("int() argument must be a string, a bytes-like object or a number, not 'NoneType'")])], '__note__': None}

PAYLOAD_TYPE=BAD_KEY
{'cl': <class '__main__.ComposedClass'>, '_message': 'While structuring ComposedClass', '_exceptions': [ClassValidationError('While structuring _InnerClass', [KeyError('f_inner_2')])], '__note__': None}

PAYLOAD_TYPE=EXTRA_FIELDS
{'cl': <class '__main__.ComposedClass'>, '_message': 'While structuring ComposedClass', '_exceptions': [Exception('Extra fields in constructor for _InnerClass: f_extra')], '__note__': None}

[Tinche]

You make some good points. Let me give you some context first.

These new exceptions inherit from ExceptionGroups, which are a new Python thing (see https://peps.python.org/pep-0654/#specification). So we can't really do anything about the _exceptions, I think. The __note__ attributes are from another PEP (https://peps.python.org/pep-0678/), which hasn't been accepted yet.

I agree a special exception for extra fields would be good, we should add that.

The differences in exceptions raised you're seeing are because some of these are raised by cattrs, and some are raised by validators in the class itself. That said, I think it'd be easy to change this, and more consistent. I can look into this.

[Tinche]

I've added a fix to the main branch that should help with your BAD_VALUE test.

[vedantpuri]

Thats awesome, thanks @Tinche for looking into this, Excited to use the updates!!

[Tinche]

Can you redo your tests with the latest version? See if it's better for you.

[vedantpuri]

@Tinche
Sorry for the late response, I just saw this. I ran through some more test cases and it definitely looks better. A couple of things:

• When constructing an inner class and passing a bad key, we throw both key error and extra field error. eg:

key_error_payload = {
        "f": "outer_most_string",
        "inner_props": {
            "f_inner_1": 10,
            "f_inner_2_bad_key": 12,
            "inner_inner_props": {"f_inner_inner": 99},
        },
    }
    make_my_class(key_error_payload, "BAD_KEY_INNER")

Results in:

PAYLOAD_TYPE=BAD_KEY_INNER
{'cl': <class '__main__.ComposedClass'>, '_message': 'While structuring ComposedClass', '_exceptions': [ClassValidationError('While structuring _InnerClass', [KeyError('f_inner_2'), ForbiddenExtraKeysError('Extra fields in constructor for _InnerClass: f_inner_2_bad_key')])], '__note__': None}

Do you think we should throw only one exception ? I am honestly on the fence for this

• Secondly I appreciate you checking in on whether it works for our use case, and with this new implementation it certainly makes things better. The only missing thing I can think of is probably also tacking on the attribute name in the nesting of class construction where this failed. In the example in the description, if say something is wrong with f_inner_inner, the exception trace would let me inspect ComposedClass -> _InnerClass -> _InnerInnerClass, however we wanted to display to the user which field on the class was causing an error, i.e. inner_props -> inner_inner_props -> f_inner_inner. I can understand if this is not a generalizable pattern and only something we need, but if it is, would be nice to add it in!

[Tinche]

For the exceptions being combined: that's expected, the point of the exceptiongroup is to gather as many exceptions at once as we can.

[vedantpuri]

@Tinche
Gotcha, thats fair.

For the second point, would it make sense to somehow add that to cattrs ? Or is that too specific change

[vedantpuri]

@Tinche Do you want to close this out ? Saw it was in the latest dev version 👀

[Tinche]

Yeah let's close. We do need to work on better introspection of errors, i.e. mapping the tree of errors into something that's more usable. But that will be a different issue.