Skip to content

Classify validation errors #3111

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

Closed
qsorix opened this issue Jul 3, 2015 · 6 comments
Closed

Classify validation errors #3111

qsorix opened this issue Jul 3, 2015 · 6 comments

Comments

@qsorix
Copy link

qsorix commented Jul 3, 2015

I'm using REST framework to implement an API that's very verbose about errors in the request. One thing I cannot do right at this moment is reporting the type of a validation error. I need this type because automatically generated bindings make good use of it.

For example, when field is required, I need not only to say which field it is, but also add error code "missing". Other error codes include "blank", "incorrect_value" and so on.

I'm aiming for things like this in my error response:

{
  "field": "/customer/address/city",
  "code": "blank",
  "details": "The attribute '/customer/address/city' can't be blank (neither null nor empty)."
}

Right now serializers.ValidationError gives me everything I need, except the code. The type of a problem that a validator/field recognized is lost. We'll technically, it is described in the message carried by the exception, but that's not intended for such use.

By reading the source code, I've found method Field.fail(self, key, **kwargs). The key is kinda what would do the trick for me if only it was available somewhere in the exception.

Any hints how I can solve my problem? Maybe it makes sense to turn this into an enhancement?

Right now I can monkey patch the Field.fail, add the key into exceptions and map known keys into my codes in an exception handler, but that's obviously an abuse of private implementation and will likely break when implementation changes.

@tomchristie
Copy link
Member

It's feasible that we'd accept a pull request that included the 'code' information in the validation error so that just a custom exception handler would be sufficient (I don't know how easy it'd be to add in the 'code' information to the exception to)

Might be worth sharing what you're doing at the moment to get a better idea of what that change might look like.

@qsorix
Copy link
Author

qsorix commented Jul 3, 2015

Initially I thought about monkey patching Field.fail. That itself was easy, but I quickly realized the information is lost anyway, when errors are being gathered by the serializers (in the summarized ValidationError we have only messages from individual exceptions).

Instead, I've bypassed the issue by prepending the key to the error messages of fields and validators. That's hackish but stays within the public API. One problem I still have is dealing with errors I don't understand in my code, e.g. validations performed by Django Models. Perhaps I'll just use some generic "unknown" code for those.

Now, I still think that adding dedicated 'code' to validation errors is a better way to go, so if you'll be happy to see a pull request, I may prepare one. After all, I'm not happy with running my hackish workarounds on production. ;)

However, at this point, a suggestion how the summarized exception should look like is welcomed. By summarized, I mean the code in this area: https://github.com/tomchristie/django-rest-framework/blob/master/rest_framework/serializers.py#L270

Some ideas of myself...

I could just change each error string to a tuple (code, message) but this isn't backward compatible...
I could make this controllable by a configuration option... (e.g. REST_FRAMEWORK.INCLUDE_ERROR_CODES)
Or I could add some sort of a builder hook method that users can use. It would take an exception and return a value that'll be included in the summarized error tree. By default it would just use strings, and a custom version could gather both strings and codes.
Or I could change existing strings into some objects that behave like strings, but also include code property.

I think I prefer the idea with a customizable builder function. Your thoughts?

@qsorix
Copy link
Author

qsorix commented Jul 10, 2015

Hi again, I've been working on this and I'm rather content with the results. There weren't so many changes in the REST framework, and the code of my use case got considerably simpler when I switched to the forked version.

Here's the pull request, I'll be happy to hear your comments:
#3137

I hope the commit message describes the whole solution clearly.

On another branch I've made sure everything works as intended. I'm not convinced this should be pulled in as well, so I kept it separate.
I did the check by customizing errors to be tuples of (message, error_code) and updating all REST tests to expect errors in such form. Time consuming but easy change. Those changes are here, in case you're interested how I check that the pull request is covering all errors:

qsorix/django-rest-framework@error-codes-enhancement...qsorix:error-codes-enhancement-self-check-all-errors

@qsorix
Copy link
Author

qsorix commented Jul 23, 2015

Updated pull request: #3169

johnraz added a commit to johnraz/django-rest-framework that referenced this issue Dec 8, 2015
This patch is meant to fix encode#3111, regarding comments made to encode#3137
and encode#3169.

The `ValidationError` will now contain a `code` attribute.

Before this patch, `ValidationError.detail` only contained a
`dict` with values equal to a  `list` of string error messages or
directly a `list` containing string error messages.

Now, the string error messages are replaced with `ValidationError`.

This means that, depending on the case, you will not only get a
string back but a all object containing both the error message and
the error code, respectively `ValidationError.detail` and
`ValidationError.code`.

It is important to note that the `code` attribute is not relevant
when the `ValidationError` represents a combination of errors and
hence is `None` in such cases.

The main benefit of this change is that the error message and
error code are now accessible the custom exception handler and can
be used to format the error response.

An custom exception handler example is available in the
`TestValidationErrorWithCode` test.
johnraz added a commit to johnraz/django-rest-framework that referenced this issue Dec 8, 2015
This patch is meant to fix encode#3111, regarding comments made to encode#3137
and encode#3169.

The `ValidationError` will now contain a `code` attribute.

Before this patch, `ValidationError.detail` only contained a
`dict` with values equal to a  `list` of string error messages or
directly a `list` containing string error messages.

Now, the string error messages are replaced with `ValidationError`.

This means that, depending on the case, you will not only get a
string back but a all object containing both the error message and
the error code, respectively `ValidationError.detail` and
`ValidationError.code`.

It is important to note that the `code` attribute is not relevant
when the `ValidationError` represents a combination of errors and
hence is `None` in such cases.

The main benefit of this change is that the error message and
error code are now accessible the custom exception handler and can
be used to format the error response.

An custom exception handler example is available in the
`TestValidationErrorWithCode` test.
johnraz added a commit to johnraz/django-rest-framework that referenced this issue Feb 23, 2016
This patch is meant to fix encode#3111, regarding comments made to encode#3137
and encode#3169.

The `ValidationError` will now contain a `code` attribute.

Before this patch, `ValidationError.detail` only contained a
`dict` with values equal to a  `list` of string error messages or
directly a `list` containing string error messages.

Now, the string error messages are replaced with `ValidationError`.

This means that, depending on the case, you will not only get a
string back but a all object containing both the error message and
the error code, respectively `ValidationError.detail` and
`ValidationError.code`.

It is important to note that the `code` attribute is not relevant
when the `ValidationError` represents a combination of errors and
hence is `None` in such cases.

The main benefit of this change is that the error message and
error code are now accessible the custom exception handler and can
be used to format the error response.

An custom exception handler example is available in the
`TestValidationErrorWithCode` test.
johnraz added a commit to johnraz/django-rest-framework that referenced this issue Apr 7, 2016
This patch is meant to fix encode#3111, regarding comments made to encode#3137
and encode#3169.

The `ValidationError` will now contain a `code` attribute.

Before this patch, `ValidationError.detail` only contained a
`dict` with values equal to a  `list` of string error messages or
directly a `list` containing string error messages.

Now, the string error messages are replaced with `ValidationError`.

This means that, depending on the case, you will not only get a
string back but a all object containing both the error message and
the error code, respectively `ValidationError.detail` and
`ValidationError.code`.

It is important to note that the `code` attribute is not relevant
when the `ValidationError` represents a combination of errors and
hence is `None` in such cases.

The main benefit of this change is that the error message and
error code are now accessible the custom exception handler and can
be used to format the error response.

An custom exception handler example is available in the
`TestValidationErrorWithCode` test.
johnraz added a commit to johnraz/django-rest-framework that referenced this issue Aug 28, 2016
This patch is meant to fix encode#3111, regarding comments made to encode#3137
and encode#3169.

The `ValidationError` will now contain a `code` attribute.

Before this patch, `ValidationError.detail` only contained a
`dict` with values equal to a  `list` of string error messages or
directly a `list` containing string error messages.

Now, the string error messages are replaced with `ValidationError`.

This means that, depending on the case, you will not only get a
string back but a all object containing both the error message and
the error code, respectively `ValidationError.detail` and
`ValidationError.code`.

It is important to note that the `code` attribute is not relevant
when the `ValidationError` represents a combination of errors and
hence is `None` in such cases.

The main benefit of this change is that the error message and
error code are now accessible the custom exception handler and can
be used to format the error response.

An custom exception handler example is available in the
`TestValidationErrorWithCode` test.

We keep `Serializer.errors`'s return type unchanged in
order to maintain backward compatibility.

The error codes will only be propagated to the `exception_handler`
or accessible through the `Serializer._errors` private attribute.
@tomchristie
Copy link
Member

Error codes now included in the upcoming 3.5 release.

@qsorix
Copy link
Author

qsorix commented Oct 12, 2016

👍 ❤️

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

Successfully merging a pull request may close this issue.

2 participants