Add error codes to ValidationError

This change addresses use cases that require more information about reported
validation errors. Currently for each error that REST Framework reports users
get only that error's message string. The message can be translated so there's
no good way to recognize programmatically what sort of an error it is.

When building an API that is supposed to return error codes, I've found it very
limiting. For example, I was supposed to differentiate between missing fields
and invalid arguments. The only way to do it right now was to monkey-patch all
hard coded error messages and prefix them with an error code, then write a custom
exception handler that unpacked those error codes and acted accordingly.
Alternatively, I could write my own set of Field and Validator classes that
would throw different exception. In either case, it felt like this is something
that has to be addressed within the REST Framework itself.

This commit introduces proper error codes handling to the Validation Error, and
a customizable error builder that let's users pick how they want to represent
their errors.

ValidationError can hold a single error itself (text), a list of those, or a
dictionary mapping errors to fields. Error code is only meaningful for a single
error, and I've added assertions to check for proper usage.

To help with my development, I've added a setting that makes error code a
mandatory argument. Thanks to this, I was able to correct all uses of
ValidationError across the code.

Information about errors was originally available via ValidationError.detail,
and format of these data must not change, because users can have custom
exception handlers that rely on it. So to maintain backward compatibility, I've
added customizable error builder. By default, it discards the error code.

Users are supposed to change that error builder in settings if they want to use
error codes in their exception handler. If they do so, the structure of
ValidationError.detail does not change, but in the leafs they'll find results
from their error builder.
This commit is contained in:
Tomasz Rydzyński 2015-07-10 10:21:08 +02:00
parent 061081c732
commit aa4421c84c
11 changed files with 166 additions and 23 deletions

View File

@ -18,13 +18,22 @@ class AuthTokenSerializer(serializers.Serializer):
if user: if user:
if not user.is_active: if not user.is_active:
msg = _('User account is disabled.') msg = _('User account is disabled.')
raise exceptions.ValidationError(msg) raise exceptions.ValidationError(
msg,
error_code='authorization'
)
else: else:
msg = _('Unable to log in with provided credentials.') msg = _('Unable to log in with provided credentials.')
raise exceptions.ValidationError(msg) raise exceptions.ValidationError(
msg,
error_code='authorization'
)
else: else:
msg = _('Must include "username" and "password".') msg = _('Must include "username" and "password".')
raise exceptions.ValidationError(msg) raise exceptions.ValidationError(
msg,
error_code='authorization'
)
attrs['user'] = user attrs['user'] = user
return attrs return attrs

View File

@ -14,6 +14,7 @@ from django.utils.translation import ugettext_lazy as _
from django.utils.translation import ungettext from django.utils.translation import ungettext
from rest_framework import status from rest_framework import status
from rest_framework.settings import api_settings
def _force_text_recursive(data): def _force_text_recursive(data):
@ -51,6 +52,33 @@ class APIException(Exception):
return self.detail return self.detail
def build_error_from_django_validation_error(exc_info):
code = exc_info.code or 'invalid'
return [
build_error(msg, error_code=code) for msg in exc_info.messages
]
def build_error(detail, error_code=None):
assert not isinstance(detail, dict) and not isinstance(detail, list), (
'Use `build_error` only with single error messages. Dictionaries and '
'lists should be passed directly to ValidationError.'
)
if api_settings.REQUIRE_ERROR_CODES:
assert error_code is not None, (
'The `error_code` argument is required for single errors. Strict '
'checking of error_code is enabled with REQUIRE_ERROR_CODES '
'settings key.'
)
return api_settings.ERROR_BUILDER(detail, error_code)
def default_error_builder(detail, error_code=None):
return detail
# The recommended style for using `ValidationError` is to keep it namespaced # The recommended style for using `ValidationError` is to keep it namespaced
# under `serializers`, in order to minimize potential confusion with Django's # under `serializers`, in order to minimize potential confusion with Django's
# built in `ValidationError`. For example: # built in `ValidationError`. For example:
@ -61,12 +89,21 @@ class APIException(Exception):
class ValidationError(APIException): class ValidationError(APIException):
status_code = status.HTTP_400_BAD_REQUEST status_code = status.HTTP_400_BAD_REQUEST
def __init__(self, detail): def __init__(self, detail, error_code=None):
# For validation errors the 'detail' key is always required. # For validation errors the 'detail' key is always required.
# The details should always be coerced to a list if not already. # The details should always be coerced to a list if not already.
if not isinstance(detail, dict) and not isinstance(detail, list): if not isinstance(detail, dict) and not isinstance(detail, list):
detail = [detail] detail = [build_error(detail, error_code=error_code)]
else:
if api_settings.REQUIRE_ERROR_CODES:
assert error_code is None, (
'The `error_code` argument must not be set for compound '
'errors. Strict checking of error_code is enabled with '
'REQUIRE_ERROR_CODES settings key.'
)
self.detail = _force_text_recursive(detail) self.detail = _force_text_recursive(detail)
self.error_code = error_code
def __str__(self): def __str__(self):
return six.text_type(self.detail) return six.text_type(self.detail)

View File

@ -25,7 +25,9 @@ from rest_framework.compat import (
MinValueValidator, OrderedDict, URLValidator, duration_string, MinValueValidator, OrderedDict, URLValidator, duration_string,
parse_duration, unicode_repr, unicode_to_repr parse_duration, unicode_repr, unicode_to_repr
) )
from rest_framework.exceptions import ValidationError from rest_framework.exceptions import (
ValidationError, build_error_from_django_validation_error
)
from rest_framework.settings import api_settings from rest_framework.settings import api_settings
from rest_framework.utils import html, humanize_datetime, representation from rest_framework.utils import html, humanize_datetime, representation
@ -401,7 +403,9 @@ class Field(object):
raise raise
errors.extend(exc.detail) errors.extend(exc.detail)
except DjangoValidationError as exc: except DjangoValidationError as exc:
errors.extend(exc.messages) errors.extend(
build_error_from_django_validation_error(exc)
)
if errors: if errors:
raise ValidationError(errors) raise ValidationError(errors)
@ -439,7 +443,7 @@ class Field(object):
msg = MISSING_ERROR_MESSAGE.format(class_name=class_name, key=key) msg = MISSING_ERROR_MESSAGE.format(class_name=class_name, key=key)
raise AssertionError(msg) raise AssertionError(msg)
message_string = msg.format(**kwargs) message_string = msg.format(**kwargs)
raise ValidationError(message_string) raise ValidationError(message_string, error_code=key)
@property @property
def root(self): def root(self):

View File

@ -20,6 +20,7 @@ from django.db.models.fields import FieldDoesNotExist
from django.utils.functional import cached_property from django.utils.functional import cached_property
from django.utils.translation import ugettext_lazy as _ from django.utils.translation import ugettext_lazy as _
from rest_framework import exceptions
from rest_framework.compat import DurationField as ModelDurationField from rest_framework.compat import DurationField as ModelDurationField
from rest_framework.compat import postgres_fields, unicode_to_repr from rest_framework.compat import postgres_fields, unicode_to_repr
from rest_framework.utils import model_meta from rest_framework.utils import model_meta
@ -276,7 +277,8 @@ def get_validation_error_detail(exc):
# exception class as well for simpler compat. # exception class as well for simpler compat.
# Eg. Calling Model.clean() explicitly inside Serializer.validate() # Eg. Calling Model.clean() explicitly inside Serializer.validate()
return { return {
api_settings.NON_FIELD_ERRORS_KEY: list(exc.messages) api_settings.NON_FIELD_ERRORS_KEY:
exceptions.build_error_from_django_validation_error(exc)
} }
elif isinstance(exc.detail, dict): elif isinstance(exc.detail, dict):
# If errors may be a dict we use the standard {key: list of values}. # If errors may be a dict we use the standard {key: list of values}.
@ -398,8 +400,9 @@ class Serializer(BaseSerializer):
message = self.error_messages['invalid'].format( message = self.error_messages['invalid'].format(
datatype=type(data).__name__ datatype=type(data).__name__
) )
error = exceptions.build_error(message, error_code='invalid')
raise ValidationError({ raise ValidationError({
api_settings.NON_FIELD_ERRORS_KEY: [message] api_settings.NON_FIELD_ERRORS_KEY: [error]
}) })
ret = OrderedDict() ret = OrderedDict()
@ -416,7 +419,9 @@ class Serializer(BaseSerializer):
except ValidationError as exc: except ValidationError as exc:
errors[field.field_name] = exc.detail errors[field.field_name] = exc.detail
except DjangoValidationError as exc: except DjangoValidationError as exc:
errors[field.field_name] = list(exc.messages) errors[field.field_name] = (
exceptions.build_error_from_django_validation_error(exc.messages)
)
except SkipField: except SkipField:
pass pass
else: else:
@ -534,7 +539,9 @@ class ListSerializer(BaseSerializer):
value = self.validate(value) value = self.validate(value)
assert value is not None, '.validate() should return the validated data' assert value is not None, '.validate() should return the validated data'
except (ValidationError, DjangoValidationError) as exc: except (ValidationError, DjangoValidationError) as exc:
raise ValidationError(detail=get_validation_error_detail(exc)) raise ValidationError(
detail=get_validation_error_detail(exc)
)
return value return value
@ -549,8 +556,9 @@ class ListSerializer(BaseSerializer):
message = self.error_messages['not_a_list'].format( message = self.error_messages['not_a_list'].format(
input_type=type(data).__name__ input_type=type(data).__name__
) )
error = exceptions.build_error(message, error_code='not_a_list')
raise ValidationError({ raise ValidationError({
api_settings.NON_FIELD_ERRORS_KEY: [message] api_settings.NON_FIELD_ERRORS_KEY: [error]
}) })
ret = [] ret = []

View File

@ -85,6 +85,8 @@ DEFAULTS = {
# Exception handling # Exception handling
'EXCEPTION_HANDLER': 'rest_framework.views.exception_handler', 'EXCEPTION_HANDLER': 'rest_framework.views.exception_handler',
'NON_FIELD_ERRORS_KEY': 'non_field_errors', 'NON_FIELD_ERRORS_KEY': 'non_field_errors',
'REQUIRE_ERROR_CODES': False,
'ERROR_BUILDER': 'rest_framework.exceptions.default_error_builder',
# Testing # Testing
'TEST_REQUEST_RENDERER_CLASSES': ( 'TEST_REQUEST_RENDERER_CLASSES': (
@ -138,6 +140,7 @@ IMPORT_STRINGS = (
'DEFAULT_VERSIONING_CLASS', 'DEFAULT_VERSIONING_CLASS',
'DEFAULT_PAGINATION_CLASS', 'DEFAULT_PAGINATION_CLASS',
'DEFAULT_FILTER_BACKENDS', 'DEFAULT_FILTER_BACKENDS',
'ERROR_BUILDER',
'EXCEPTION_HANDLER', 'EXCEPTION_HANDLER',
'TEST_REQUEST_RENDERER_CLASSES', 'TEST_REQUEST_RENDERER_CLASSES',
'UNAUTHENTICATED_USER', 'UNAUTHENTICATED_USER',

View File

@ -11,7 +11,7 @@ from __future__ import unicode_literals
from django.utils.translation import ugettext_lazy as _ from django.utils.translation import ugettext_lazy as _
from rest_framework.compat import unicode_to_repr from rest_framework.compat import unicode_to_repr
from rest_framework.exceptions import ValidationError from rest_framework.exceptions import ValidationError, build_error
from rest_framework.utils.representation import smart_repr from rest_framework.utils.representation import smart_repr
@ -60,7 +60,7 @@ class UniqueValidator(object):
queryset = self.filter_queryset(value, queryset) queryset = self.filter_queryset(value, queryset)
queryset = self.exclude_current_instance(queryset) queryset = self.exclude_current_instance(queryset)
if queryset.exists(): if queryset.exists():
raise ValidationError(self.message) raise ValidationError(self.message, error_code='unique')
def __repr__(self): def __repr__(self):
return unicode_to_repr('<%s(queryset=%s)>' % ( return unicode_to_repr('<%s(queryset=%s)>' % (
@ -101,7 +101,10 @@ class UniqueTogetherValidator(object):
return return
missing = dict([ missing = dict([
(field_name, self.missing_message) (
field_name,
build_error(self.missing_message, error_code='required')
)
for field_name in self.fields for field_name in self.fields
if field_name not in attrs if field_name not in attrs
]) ])
@ -147,7 +150,8 @@ class UniqueTogetherValidator(object):
] ]
if None not in checked_values and queryset.exists(): if None not in checked_values and queryset.exists():
field_names = ', '.join(self.fields) field_names = ', '.join(self.fields)
raise ValidationError(self.message.format(field_names=field_names)) raise ValidationError(self.message.format(field_names=field_names),
error_code='unique')
def __repr__(self): def __repr__(self):
return unicode_to_repr('<%s(queryset=%s, fields=%s)>' % ( return unicode_to_repr('<%s(queryset=%s, fields=%s)>' % (
@ -185,7 +189,10 @@ class BaseUniqueForValidator(object):
'required' state on the fields they are applied to. 'required' state on the fields they are applied to.
""" """
missing = dict([ missing = dict([
(field_name, self.missing_message) (
field_name,
build_error(self.missing_message, error_code='required')
)
for field_name in [self.field, self.date_field] for field_name in [self.field, self.date_field]
if field_name not in attrs if field_name not in attrs
]) ])
@ -211,7 +218,8 @@ class BaseUniqueForValidator(object):
queryset = self.exclude_current_instance(attrs, queryset) queryset = self.exclude_current_instance(attrs, queryset)
if queryset.exists(): if queryset.exists():
message = self.message.format(date_field=self.date_field) message = self.message.format(date_field=self.date_field)
raise ValidationError({self.field: message}) error = build_error(message, error_code='unique')
raise ValidationError({self.field: error})
def __repr__(self): def __repr__(self):
return unicode_to_repr('<%s(queryset=%s, field=%s, date_field=%s)>' % ( return unicode_to_repr('<%s(queryset=%s, field=%s, date_field=%s)>' % (

View File

@ -1178,7 +1178,10 @@ class TestFieldFieldWithName(FieldValues):
# call into it's regular validation, or require PIL for testing. # call into it's regular validation, or require PIL for testing.
class FailImageValidation(object): class FailImageValidation(object):
def to_python(self, value): def to_python(self, value):
raise serializers.ValidationError(self.error_messages['invalid_image']) raise serializers.ValidationError(
self.error_messages['invalid_image'],
error_code='invalid_image'
)
class PassImageValidation(object): class PassImageValidation(object):

View File

@ -7,6 +7,7 @@ import pytest
from rest_framework import serializers from rest_framework import serializers
from rest_framework.compat import unicode_repr from rest_framework.compat import unicode_repr
from rest_framework.fields import DjangoValidationError
from .utils import MockObject from .utils import MockObject
@ -59,7 +60,10 @@ class TestValidateMethod:
integer = serializers.IntegerField() integer = serializers.IntegerField()
def validate(self, attrs): def validate(self, attrs):
raise serializers.ValidationError('Non field error') raise serializers.ValidationError(
'Non field error',
error_code='test'
)
serializer = ExampleSerializer(data={'char': 'abc', 'integer': 123}) serializer = ExampleSerializer(data={'char': 'abc', 'integer': 123})
assert not serializer.is_valid() assert not serializer.is_valid()
@ -299,3 +303,10 @@ class TestCacheSerializerData:
pickled = pickle.dumps(serializer.data) pickled = pickle.dumps(serializer.data)
data = pickle.loads(pickled) data = pickle.loads(pickled)
assert data == {'field1': 'a', 'field2': 'b'} assert data == {'field1': 'a', 'field2': 'b'}
class TestGetValidationErrorDetail:
def test_get_validation_error_detail_converts_django_errors(self):
exc = DjangoValidationError("Missing field.", code='required')
detail = serializers.get_validation_error_detail(exc)
assert detail == {'non_field_errors': ['Missing field.']}

View File

@ -280,7 +280,10 @@ class TestListSerializerClass:
def test_list_serializer_class_validate(self): def test_list_serializer_class_validate(self):
class CustomListSerializer(serializers.ListSerializer): class CustomListSerializer(serializers.ListSerializer):
def validate(self, attrs): def validate(self, attrs):
raise serializers.ValidationError('Non field error') raise serializers.ValidationError(
'Non field error',
error_code='test'
)
class TestSerializer(serializers.Serializer): class TestSerializer(serializers.Serializer):
class Meta: class Meta:

View File

@ -41,7 +41,8 @@ class ShouldValidateModelSerializer(serializers.ModelSerializer):
def validate_renamed(self, value): def validate_renamed(self, value):
if len(value) < 3: if len(value) < 3:
raise serializers.ValidationError('Minimum 3 characters.') raise serializers.ValidationError('Minimum 3 characters.',
error_code='min_length')
return value return value
class Meta: class Meta:

View File

@ -0,0 +1,56 @@
import pytest
from django.test import TestCase
from rest_framework import serializers
from rest_framework.exceptions import build_error
from rest_framework.settings import api_settings
class TestMandatoryErrorCodeArgument(TestCase):
"""
If mandatory error code is enabled in settings, it should prevent throwing
ValidationError without the code set.
"""
def setUp(self):
self.DEFAULT_REQUIRE_ERROR_CODES = api_settings.REQUIRE_ERROR_CODES
api_settings.REQUIRE_ERROR_CODES = True
def tearDown(self):
api_settings.REQUIRE_ERROR_CODES = self.DEFAULT_REQUIRE_ERROR_CODES
def test_validation_error_requires_code_for_simple_messages(self):
with pytest.raises(AssertionError):
serializers.ValidationError("")
def test_validation_error_requires_no_code_for_structured_errors(self):
"""
ValidationError can hold a list or dictionary of simple errors, in
which case the code is no longer meaningful and should not be
specified.
"""
with pytest.raises(AssertionError):
serializers.ValidationError([], error_code='min_value')
with pytest.raises(AssertionError):
serializers.ValidationError({}, error_code='min_value')
def test_validation_error_stores_error_code(self):
exception = serializers.ValidationError("", error_code='min_value')
assert exception.error_code == 'min_value'
class TestCustomErrorBuilder(TestCase):
def setUp(self):
self.DEFAULT_ERROR_BUILDER = api_settings.ERROR_BUILDER
def error_builder(message, error_code):
return (message, error_code, "customized")
api_settings.ERROR_BUILDER = error_builder
def tearDown(self):
api_settings.ERROR_BUILDER = self.DEFAULT_ERROR_BUILDER
def test_class_based_view_exception_handler(self):
error = build_error("Too many characters", error_code="max_length")
assert error == ("Too many characters", "max_length", "customized")