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

Dict(values=Nested()) doesn't collect complete validation errors #730

Closed
shabble opened this Issue Jan 29, 2018 · 1 comment

Comments

Projects
None yet
2 participants
@shabble

shabble commented Jan 29, 2018

Using the new structured Dict support from #483 / #700, the error handling/collection logic seems unable to provide access to the original/detailed failures due to how it aggregates multiple errors.

  • marshmallow.version = '3.0.0b6'
  • Python 2.7.14

Simple test is something like:

from marshmallow import Schema, pprint, fields
from marshmallow.validate import OneOf

class InnerSchema(Schema):
    inner = fields.String(validate=OneOf(['a','b'], error='inner validation fail on {input}'))

class OuterSchema(Schema):
    outer = fields.Dict(values=fields.Nested(InnerSchema))

foo = {
    'outer': {
        'x': {'inner': 'a'},
    }
}
foo2 = {
    'outer': {
        'x': {'inner': 'xxx'},
    }
}

# This works (no validation failures)
outer_sch = OuterSchema()
ret = outer_sch.load(foo)
pprint(ret)
# >>> UnmarshalResult(data={'outer': {'x': {'inner': u'a'}}}, errors={})

# Fails to validate Inner.inner
outer_sch2 = OuterSchema()
ret2 = outer_sch.load(foo2)
pprint(ret2)
# >>> UnmarshalResult(data={}, errors={'outer': {'x': [u'Invalid value: inner']}})
# Note: the custom error message on outer.x.inner is not present.

# validation + custom message is working as expected on the inner String field.
inner_sch = InnerSchema()
inner2 = inner_sch.load(foo2['outer']['x'])
pprint(inner2)
# >>> UnmarshalResult(data={}, errors={'inner': ['inner validation fail on xxx']})

I don't think it's possible to extract the actual failure message ('inner validation fail on xxx) with any of hte existing error handling overrides, because it's getting dropped (I think) here:
https://github.com/marshmallow-code/marshmallow/blob/3.0.0b6/marshmallow/fields.py#L1219

(since e.messages is a dict but the loop is only taking its keys).

Not sure exactly what an appropriate result would be. Any access to the inner validation structures would be fine for what I need.

@lafrech

This comment has been minimized.

Member

lafrech commented Jan 29, 2018

If possible, we should have the same structure as what the List field generates, except it would have "keys" rather than "indexes".

From List:

errors.update({idx: e.messages})

Maybe errors[key] should be a dict rather than a list, with at most two keys: key for the list of key errors and value for the list of value errors.

errors[key] = {
    'key': [],
    'value': [],
}

We could do (untested):

                    key = keys[idx]
                    if key not in errors:  # Avoid this test by using a defaultdict?
                        errors[key] = {}
                    errors[key]['value'] = e.messages

and likewise for key errors.

To be closer to List, errors itself could be a list rather than a dict. But I don't see any benefit to this apart from having similar structures for both. And it makes sense that List has an error list and Dict an error dict.

@lafrech lafrech added this to the 3.0 milestone Feb 3, 2018

lafrech added a commit to Nobatek/marshmallow that referenced this issue Feb 7, 2018

lafrech added a commit to Nobatek/marshmallow that referenced this issue Mar 18, 2018

@sloria sloria closed this in #734 Mar 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment