Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Exception handler exception for response ValidationError with non-pydantic model #1275

Closed
jomasti opened this issue Apr 16, 2020 · 5 comments

Comments

@jomasti
Copy link

jomasti commented Apr 16, 2020

Describe the bug

FastAPI will raise a ValidationError for the invalid response and set the field type. This can go wrong if the response model is not a pydantic model (or dataclass). When trying to produce the string representation of the exception, pydantic will try to produce the errors and fail because of the field type passed into the ValidationError not being a BaseModel or DataclassType. I am not sure if we need something like a ResponseValidationError to prevent this issue. Or maybe I am missing something else?

To Reproduce

Steps to reproduce the behavior with a minimum self-contained file.

Replace each part with your own scenario:

  1. Create a file with:
from typing import List

from fastapi import FastAPI, Request
from fastapi.responses import JSONResponse

app = FastAPI()


@app.exception_handler(Exception)
async def exception_handler(request: Request, exc: Exception):
    error_message = f"Unexpected error occurred: {exc}"
    return JSONResponse(status_code=500, content={"detail": error_message})


@app.get('/', response_model=List[int])
async def foo() -> List[int]:
    return 1
  1. Call the endpoint /.
  2. It raises an exception when trying to get the string representation of the ValidationError.
Traceback (most recent call last):
  File "/Users/jomasti/.local/pyenv/versions/repro/lib/python3.7/site-packages/starlette/middleware/errors.py", line 159, in __call__
    await self.app(scope, receive, _send)
  File "/Users/jomasti/.local/pyenv/versions/repro/lib/python3.7/site-packages/starlette/exceptions.py", line 82, in __call__
    raise exc from None
  File "/Users/jomasti/.local/pyenv/versions/repro/lib/python3.7/site-packages/starlette/exceptions.py", line 71, in __call__
    await self.app(scope, receive, sender)
  File "/Users/jomasti/.local/pyenv/versions/repro/lib/python3.7/site-packages/starlette/routing.py", line 550, in __call__
    await route.handle(scope, receive, send)
  File "/Users/jomasti/.local/pyenv/versions/repro/lib/python3.7/site-packages/starlette/routing.py", line 227, in handle
    await self.app(scope, receive, send)
  File "/Users/jomasti/.local/pyenv/versions/repro/lib/python3.7/site-packages/starlette/routing.py", line 41, in app
    response = await func(request)
  File "/Users/jomasti/.local/pyenv/versions/repro/lib/python3.7/site-packages/fastapi/routing.py", line 213, in app
    is_coroutine=is_coroutine,
  File "/Users/jomasti/.local/pyenv/versions/repro/lib/python3.7/site-packages/fastapi/routing.py", line 126, in serialize_response
    raise ValidationError(errors, field.type_)
pydantic.error_wrappers.ValidationError: <unprintable ValidationError object>

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/jomasti/.local/pyenv/versions/repro/lib/python3.7/site-packages/pydantic/error_wrappers.py", line 50, in errors
    config = self.model.__config__  # type: ignore
AttributeError: type object 'int' has no attribute '__config__'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/jomasti/.local/pyenv/versions/repro/lib/python3.7/site-packages/uvicorn/protocols/http/httptools_impl.py", line 385, in run_asgi
    result = await app(self.scope, self.receive, self.send)
  File "/Users/jomasti/.local/pyenv/versions/repro/lib/python3.7/site-packages/uvicorn/middleware/proxy_headers.py", line 45, in __call__
    return await self.app(scope, receive, send)
  File "/Users/jomasti/.local/pyenv/versions/repro/lib/python3.7/site-packages/fastapi/applications.py", line 149, in __call__
    await super().__call__(scope, receive, send)
  File "/Users/jomasti/.local/pyenv/versions/repro/lib/python3.7/site-packages/starlette/applications.py", line 102, in __call__
    await self.middleware_stack(scope, receive, send)
  File "/Users/jomasti/.local/pyenv/versions/repro/lib/python3.7/site-packages/starlette/middleware/errors.py", line 172, in __call__
    response = await self.handler(request, exc)
  File "./repro.py", line 11, in exception_handler
    error_message = f"Unexpected error occurred: {exc}"
  File "/Users/jomasti/.local/pyenv/versions/repro/lib/python3.7/site-packages/pydantic/error_wrappers.py", line 60, in __str__
    errors = self.errors()
  File "/Users/jomasti/.local/pyenv/versions/repro/lib/python3.7/site-packages/pydantic/error_wrappers.py", line 52, in errors
    config = self.model.__pydantic_model__.__config__  # type: ignore
AttributeError: type object 'int' has no attribute '__pydantic_model__'

Expected behavior

I expected it to return {"detail": "Unexpected error occurred: <exception string>"}. Even though the endpoint code is clearly wrong, I would hope that I would be able to log and return the resulting error instead of the exception being raised and receiving the default 500 response.

Environment

  • OS: macOS Catalina
  • FastAPI Version: 0.54.1
  • Python version: 3.7.6
@jomasti jomasti added the bug Something isn't working label Apr 16, 2020
@obataku
Copy link
Contributor

obataku commented Apr 18, 2020

for what it's worth this appears to be an upstream bug in pydantic

edit: OK, maybe not a bug in pydantic; from what I can tell this is from incorrect use of an internal pydantic method (ModelField.validate) that is explicitly not part of the public interface:

ModelField isn't designed to be used without BaseModel, you might get it to work, but it's highly likely it will be broken by a future change we do not consider "breaking" since ModelField is not considered part of the public interface.

For example: we always initialise ModelFields via infer, so we should probably move that logic into init and remove infer, I wouldn't consider this change to require a major version bump.

Better to use parse_obj_as or a BaseModel directly.


anyway, the code in question is in serialize_response:

if is_coroutine:
    value, errors_ = field.validate(response_content, {}, loc=("response",))
else:
    value, errors_ = await run_in_threadpool(
        field.validate, response_content, {}, loc=("response",)
    )
...
if errors:
    raise ValidationError(errors, field.type_)

ValidationError expects the second argument to be a model type (BaseModel or dataclass) proper, not just any type that can be used to validate a field (e.g. List[int]):

             try:
                config = self.model.__config__  # type: ignore
            except AttributeError:
                config = self.model.__pydantic_model__.__config__  # type: ignore
            self._error_cache = list(flatten_errors(self.raw_errors, config))

it may be worthwhile to replace the ModelFields (i.e. response_field, response_fields of APIRoute) used in response validation for serialize_response with a model proper, using something like pydantic's parse_obj_as to box non-model types appropriately (using __root__) so we can use the more reliable BaseModel.validate method. what do you think, @tiangolo?


for now I've had success monkey-patching pydantic.error_wrappers so that ValidationError.errors() can yield config = None:

        config = None
        for mdl in self.model, getattr(self.model, '__pydantic_model__', None):
            config = config or getattr(mdl, '__config__', None)

... and error_dict properly handles this case when trying to find a msg_template:

    msg_template = \
        config.error_msg_templates.get(type_) if config else getattr(exc, 'msg_template', None)

@Mause
Copy link
Contributor

Mause commented Sep 28, 2020

@ATRI2107 I think your issue is unrelated - your response doesn't match what you have defined in your response_model

@tedivm
Copy link

tedivm commented Aug 29, 2022

Anyone running into this from google, if you haven't seen this before and are suddenly getting it after upgrading to >=0.80 it may be because of the response model validation change.

@dqian3
Copy link

dqian3 commented Jan 19, 2023

I ran into this issue after upgrading to >=0.89 as well, since I had an invalid return type annotation on an endpoints. (0.89 adds support for using the return annotations on endpoints for validation).

To be fair, if I had static type checking enabled in my dev environment (which I found out I didn't while debugging this issue), that would have caught the incorrect return type annotation first. But I think it would still be nice to have a less opaque error for people new to fasapi, such as myself.

@obataku
Copy link
Contributor

obataku commented Jan 19, 2023

@tiangolo any thoughts re: my previous comment? #1275 (comment)

@tiangolo tiangolo added question Question or problem reviewed and removed bug Something isn't working labels Feb 24, 2023
@tiangolo tiangolo changed the title [BUG] Exception handler exception for response ValidationError with non-pydantic model Exception handler exception for response ValidationError with non-pydantic model Feb 24, 2023
Repository owner locked and limited conversation to collaborators Feb 28, 2023
@tiangolo tiangolo converted this issue into discussion #9048 Feb 28, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Projects
None yet
Development

No branches or pull requests

6 participants