Skip to content
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

Allow Customizing Validation Errors #1380

Merged
merged 3 commits into from
Jan 12, 2025
Merged

Allow Customizing Validation Errors #1380

merged 3 commits into from
Jan 12, 2025

Conversation

jceipek
Copy link
Contributor

@jceipek jceipek commented Jan 3, 2025

Summary

ninja.errors.ValidationErrors have some fundamental bugs and discard crucial error context (see #1381). I've been working around these problems with monkeypatching in my own project.

This PR allows developers to access the missing error context in order to make much friendlier error messages than is currently possible with Django Ninja. It solves Problem 1 from #1381 and partially addresses Problem 2.
Fixing the other problems I found will take more PRs and a larger discussion, but I think this PR makes the situation better.

Before

Prior to this PR, users had access to ninja.errors.ValidationErrors via @api.exception_handler(ValidationError), but this has some problems:

  1. It discards error context needed for understanding the error (especially model.__pydantic_core_schema__)
  2. It transforms the loc field in pydantic errors, which makes it impossible for downstream code to understand what each element of the loc means
  3. (Not directly addressed in this PR) It sometimes produces incorrect locs

After

This PR allows users to control how pydantic.ValidationErrors get turned into ninja.errors.ValidationErrors, without breaking backwards compatibility with existing user code.
The PR:

  1. Introduces an intermediate ValidationErrorContext
  2. Adds an overridable method, validation_error_from_error_contexts(self, error_contexts: List[ValidationErrorContext]) -> ValidationError
  3. Adds test code and usage documentation for this new functionality.

Context

Consider the following example:

# Example 1
# urls.py:
from __future__ import annotations

import ninja
from django.http import HttpRequest
from django.urls import path

class UserSchema(ninja.Schema):
    salutation: str
    short_name: str
    full_name: str
    friends: list[UserSchema] = []

class HelloSchema(ninja.Schema):
    user: str | UserSchema

demo_api = ninja.NinjaAPI()

@demo_api.post("/hello")
def hello(request: HttpRequest, data: HelloSchema) -> str:
    if isinstance(data.user, str):
        return f"Hello {data.user}"
    else:
        return f"{data.user.salutation} {data.user.short_name}"

urlpatterns = [
    path("demo/", demo_api.urls)
]

If you POST { "user": {} } to /demo/hello (you can use "Try it out" at "/demo/docs"), the response (with whitespace changes for readability) is:

{
  "detail": [
    { "type": "string_type", "loc": [ "body", "data", "user", "str" ],                                                "msg": "Input should be a valid string" }, 
    { "type": "missing",     "loc": [ "body", "data", "user", "function-wrap[_run_root_validator()]", "salutation" ], "msg": "Field required" },
    { "type": "missing",     "loc": [ "body", "data", "user", "function-wrap[_run_root_validator()]", "short_name" ], "msg": "Field required" },
    { "type": "missing",     "loc": [ "body", "data", "user", "function-wrap[_run_root_validator()]", "full_name" ],  "msg": "Field required" }
  ]
}

Note the confusing 4th value of each "loc" field: "str" or "function-wrap[_run_root_validator()]".
Note also that the 2nd value of each "loc" field is "data", which is the name of a parameter to the hello function.
These parts of "loc" are not meaningful to API callers and are brittle in the face of future changes (for example, they will change if the data function parameter gets renamed or if UserSchema gets a new model_validator).

A developer using Django Ninja should be able to produce user-friendly errors like the following instead of the errors in the example above:

{ "message": "body.user: Should be a string or a UserSchema but was `{}`" }

This is usually possible with Pydantic by walking model.__pydantic_core_schema__ with the error loc (see below), but Django Ninja discards model.__pydantic_core_schema__ and transforms loc, so it's not possible in Django Ninja without user control of what makes it into ValidationError.

Understanding Pydantic Errors and model.__pydantic_core_schema__

In Pydantic 2, the errors in e.errors relate not to the input data (the end-user's perspective) but to the schema against which the input data was validated.
To construct user-friendly errors, we can crawl the tree rooted in model.__pydantic_core_schema__ using the original "loc" field of each error.
This allows us to, for example, identify whether a given loc element refers to a field, a dictionary key, a union discriminator, or something else.

For the hello endpoint in Example 1 above, the model.__pydantic_core_schema__ of the body is:

{'definitions': [{'function': {'function': <bound method Schema._run_root_validator of <class 'demo.urls.UserSchema'>>,
                               'type': 'with-info'},
                  'metadata': {'pydantic_js_functions': [<bound method BaseModel.__get_pydantic_json_schema__ of <class 'demo.urls.UserSchema'>>]},
                  'ref': 'demo.urls.UserSchema:5721087360',
                  'schema': {'cls': <class 'demo.urls.UserSchema'>,
                             'config': {'from_attributes': True,
                                        'title': 'UserSchema'},
                             'custom_init': False,
                             'root_model': False,
                             'schema': {'computed_fields': [],
                                        'fields': {'friends': {'metadata': {},
                                                               'schema': {'default': [],
                                                                          'schema': {'items_schema': {'schema_ref': 'demo.urls.UserSchema:5721087360',
                                                                                                      'type': 'definition-ref'},
                                                                                     'type': 'list'},
                                                                          'type': 'default'},
                                                               'type': 'model-field'},
                                                   'full_name': {'metadata': {},
                                                                 'schema': {'type': 'str'},
                                                                 'type': 'model-field'},
                                                   'salutation': {'metadata': {},
                                                                  'schema': {'type': 'str'},
                                                                  'type': 'model-field'},
                                                   'short_name': {'metadata': {},
                                                                  'schema': {'type': 'str'},
                                                                  'type': 'model-field'}},
                                        'model_name': 'UserSchema',
                                        'type': 'model-fields'},
                             'type': 'model'},
                  'type': 'function-wrap'}],
 'schema': {'cls': <class 'abc.BodyParams'>,
            'config': {'title': 'BodyParams'},
            'custom_init': False,
            'metadata': {'pydantic_js_functions': [<bound method BaseModel.__get_pydantic_json_schema__ of <class 'abc.BodyParams'>>]},
            'ref': 'abc.BodyParams:5721102816',
            'root_model': False,
            'schema': {'computed_fields': [],
                       'fields': {'data': {'metadata': {'pydantic_js_extra': {}},
                                           'schema': {'function': {'function': <bound method Schema._run_root_validator of <class 'demo.urls.HelloSchema'>>,
                                                                   'type': 'with-info'},
                                                      'metadata': {'pydantic_js_functions': [<bound method BaseModel.__get_pydantic_json_schema__ of <class 'demo.urls.HelloSchema'>>]},
                                                      'ref': 'demo.urls.HelloSchema:5721091792',
                                                      'schema': {'cls': <class 'demo.urls.HelloSchema'>,
                                                                 'config': {'from_attributes': True,
                                                                            'title': 'HelloSchema'},
                                                                 'custom_init': False,
                                                                 'root_model': False,
                                                                 'schema': {'computed_fields': [],
                                                                            'fields': {'user': {'metadata': {},
                                                                                                'schema': {'choices': [{'type': 'str'},
                                                                                                                       {'schema_ref': 'demo.urls.UserSchema:5721087360',
                                                                                                                        'type': 'definition-ref'}],
                                                                                                           'type': 'union'},
                                                                                                'type': 'model-field'}},
                                                                            'model_name': 'HelloSchema',
                                                                            'type': 'model-fields'},
                                                                 'type': 'model'},
                                                      'type': 'function-wrap'},
                                           'type': 'model-field'}},
                       'model_name': 'BodyParams',
                       'type': 'model-fields'},
            'type': 'model'},
 'type': 'definitions'}

and the first e.errors() error is:

{'type': 'string_type', 'loc': ('data', 'user', 'str'), 'msg': 'Input should be a valid string', 'input': {}, 'url': 'https://errors.pydantic.dev/2.10/v/string_type'}

We can see that 'str' isn't a field here because it's a discriminator for this 'union' schema:

{'choices': [{'type': 'str'},
            {'schema_ref': 'demo.urls.UserSchema:5721087360',
            'type': 'definition-ref'}],
'type': 'union'}

Moreover, we can see that the 3 other errors refer to a different union variant because the 3rd element of "loc" is 'function-wrap[_run_root_validator()]' rather than 'str':

    {'type': 'missing', 'loc': ('data', 'user', 'function-wrap[_run_root_validator()]', 'salutation'), 'msg': 'Field required', 'input': <DjangoGetter: {}>, 'url': 'https://errors.pydantic.dev/2.10/v/missing'} 
    {'type': 'missing', 'loc': ('data', 'user', 'function-wrap[_run_root_validator()]', 'short_name'), 'msg': 'Field required', 'input': <DjangoGetter: {}>, 'url': 'https://errors.pydantic.dev/2.10/v/missing'}
    {'type': 'missing', 'loc': ('data', 'user', 'function-wrap[_run_root_validator()]', 'full_name'), 'msg': 'Field required', 'input': <DjangoGetter: {}>, 'url': 'https://errors.pydantic.dev/2.10/v/missing'}

Comment on lines +522 to +543
def validation_error_from_error_contexts(
self, error_contexts: List[ValidationErrorContext]
) -> ValidationError:
errors: List[Dict[str, Any]] = []
for context in error_contexts:
model = context.model
e = context.pydantic_validation_error
for i in e.errors(include_url=False):
i["loc"] = (
model.__ninja_param_source__,
) + model.__ninja_flatten_map_reverse__.get(i["loc"], i["loc"])
# removing pydantic hints
del i["input"] # type: ignore
if (
"ctx" in i
and "error" in i["ctx"]
and isinstance(i["ctx"]["error"], Exception)
):
i["ctx"]["error"] = str(i["ctx"]["error"])
errors.append(dict(i))
return ValidationError(errors)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking of exposing this on Routers or api_operations instead, but since exception handlers must be registered on APIs, I couldn't think of a reasonable way to do so.

Comment on lines -291 to -307
items = []
for i in e.errors(include_url=False):
i["loc"] = (
model.__ninja_param_source__,
) + model.__ninja_flatten_map_reverse__.get(i["loc"], i["loc"])
# removing pydantic hints
del i["input"] # type: ignore
if (
"ctx" in i
and "error" in i["ctx"]
and isinstance(i["ctx"]["error"], Exception)
):
i["ctx"]["error"] = str(i["ctx"]["error"])
items.append(dict(i))
errors.extend(items)
if errors:
raise ValidationError(errors)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I preserved all of this behavior by moving it to validation_error_from_error_contexts in main.py

@@ -28,6 +30,22 @@ class ConfigError(Exception):
pass


TModel = TypeVar("TModel", bound="ParamModel")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a copy of TModel in ninja.params.models. I couldn't figure out a way to import it without causing cyclical import issues, because it's used below in class ValidationErrorContext(Generic[TModel])

Comment on lines +36 to +46
class ValidationErrorContext(Generic[TModel]):
"""
The full context of a `pydantic.ValidationError`, including all information
needed to produce a `ninja.errors.ValidationError`.
"""

def __init__(
self, pydantic_validation_error: pydantic.ValidationError, model: TModel
):
self.pydantic_validation_error = pydantic_validation_error
self.model = model
Copy link
Contributor Author

@jceipek jceipek Jan 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a good place to use a @dataclass, but I think this better matches the rest of the code style here.

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

Successfully merging this pull request may close these issues.

2 participants