Opened 6 years ago

Closed 6 years ago

#29318 closed Bug (wontfix)

ValidationError has no attribute `error_list` if message is a dict, but Field.run_validators() depends on it

Reported by: Michael Käufl Owned by: nobody
Component: Core (Other) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

If the message is a dict, ValidationError has no attribute error_list:

class ValidationError(Exception):

    def __init__(self, message, code=None, params=None):
        # …
        if isinstance(message, dict):  # <----
            self.error_dict = {}
            for field, messages in message.items():
                if not isinstance(messages, ValidationError):
                    messages = ValidationError(messages)
                self.error_dict[field] = messages.error_list
        elif isinstance(message, list):
            self.error_list = …
            # …
        else:
            # …
            self.error_list = [self]

See

But Field.run_validators() depends on ValidationError having an attribute error_list:

class Field(RegisterLookupMixin):

    def run_validators(self, value):
        # …
        for v in self.validators:
            try:
                v(value)
            except exceptions.ValidationError as e:
                if hasattr(e, 'code') and e.code in self.error_messages:
                    e.message = self.error_messages[e.code]
                errors.extend(e.error_list)  # <----

        # …

See

This leads to an AttributeError when using a dict as message when raising a ValidationError inside a validator.

Change History (4)

comment:1 by Tim Graham, 6 years ago

Component: UncategorizedCore (Other)

Could you elaborate on the use case for using a dictionary message inside a validator's ValidationError? I would expect validators to be generic and not know the field names they are being used with.

comment:2 by Michael Käufl, 6 years ago

Sure. Consider a JSONField with an allowed list of keys:

if key not in allowed_keys:
    raise ValidationError({key: 'Not allowed.'})

We use these exceptions not only for the admin, but also for API error messages through django rest framework's custom exception handling. This is also the reason, why we use the field's name in some other (therefore non-generic) validators. Using the field's name as key in the error_dict allows our front-end to display the error message at the affected field and not as a generic error message of the form.

Last edited 6 years ago by Michael Käufl (previous) (diff)

comment:3 by Simon Charette, 6 years ago

I think that Field.run_validators() should not to handle dict based ValidationError as they should only be used to map concrete fields to errors in multi-fields cleaning functions (e.g. Form.clean(), Model.clean()). Since Field instances cannot have nested-fields I wouldn't expect dict to be handled.

In your JSONField subclass example you're using an invalid key as your field mapping. This key doesn't map to any field and raising ValidationError(f'Key {key} not allowed.') within the field would be more appropriate.

Keep in mind that errors have to be ultimately flattened to a {field_name -> error_list} map at the form/model level while a different model is used for DRF serializers since they have nested fields.

comment:4 by Tim Graham, 6 years ago

Resolution: wontfix
Status: newclosed
Note: See TracTickets for help on using tickets.
Back to Top