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

response_model does not invalidate None #2719

Closed
hukkin opened this issue Jan 29, 2021 · 13 comments
Closed

response_model does not invalidate None #2719

hukkin opened this issue Jan 29, 2021 · 13 comments
Labels
bug Something isn't working

Comments

@hukkin
Copy link
Contributor

hukkin commented Jan 29, 2021

Example

from fastapi import FastAPI

from pydantic import BaseModel


app = FastAPI()


class RespModel(BaseModel):
    example_field: str


@app.get("/", response_model=RespModel)
def get_resp():
    return None

Description

  • Open the browser and call the endpoint /.
  • It returns a JSON with null, with HTTP code 200 OK.
  • But I expected it to return HTTP 500, since I don't see how None is a valid RespModel .

Environment

  • OS: Linux
  • FastAPI Version: 0.63.0
  • Python version: 3.8.6

Additional context

I'm not sure if this is a bug, or expected behavior. I tried to look for expected behavior in the docs but didn't find.

If this is expected, is there perhaps a way in the API to make FastAPI invalidate None when response_model is set?

@hukkin hukkin added the question Question or problem label Jan 29, 2021
@Kludex
Copy link
Sponsor Collaborator

Kludex commented Jan 29, 2021

Looking at the code, it looks like it's a bug. Good job!

Let me explain why I think you're right and how this is happening:

On the APIRoute we have this line:

            self.response_field = create_response_field(
                name=response_name, type_=self.response_model
            )

Which is the line that determines the response behavior. The problem is not apparent just seeing this, so we need to follow the create_response_field() function in here:

def create_response_field(
    name: str,
    type_: Type[Any],
    class_validators: Optional[Dict[str, Validator]] = None,
    default: Optional[Any] = None,
    required: Union[bool, UndefinedType] = False,
    model_config: Type[BaseConfig] = BaseConfig,
    field_info: Optional[FieldInfo] = None,
    alias: Optional[str] = None,
) -> ModelField:
    ...

This is enough for us. Do you see the required field? It's the parameter that is causing what you experienced.

Just to be sure, I've changed the default value to True, and then I was able to get the 500 that you wanted.

Maybe adding a response_required field on the APIRoute can be a solution.

@hukkin
Copy link
Contributor Author

hukkin commented Jan 29, 2021

Thanks for the explanation @Kludex. I took a stab at this here #2725

@wbadart
Copy link

wbadart commented Feb 22, 2021

@Kludex if I may ask a tangentially related question, why does FastAPI.post use a response_model parameter at all, rather than inspecting the return type annotation?

@Kludex
Copy link
Sponsor Collaborator

Kludex commented Feb 22, 2021

I'm not going to be able to provide the link now and I'll probably forget later, but if you go to the docs and check "/response-model" path, there's a note about this. You should search for "type hint" on that page if I remember correctly.

@chbndrhnns
Copy link

chbndrhnns commented Feb 22, 2021

why does FastAPI.post use a response_model parameter at all, rather than inspecting the return type annotation?

This is a response I once got to that question: #653 (comment)

@wbadart
Copy link

wbadart commented Feb 22, 2021

Gotcha. Thanks folks!

@tiangolo tiangolo added bug Something isn't working question Question or problem and removed question Question or problem labels Aug 22, 2022
@tiangolo
Copy link
Owner

Indeed, this is a bug! Thanks @hukkin for the report, example code, and fix in the PR. 🚀

And thanks @Kludex for all the help and debugging! 🤓 🙇

The fix will be available in FastAPI 0.80.0, released in some hours. 🎉

@orenwang
Copy link

Okay, my server has complained a ton of errors since my yesterday's release. I suppose that's the price I'll have to pay for not pinning dependencies' versions...

@github-actions github-actions bot removed the answered label Aug 25, 2022
@JarroVGIT
Copy link
Contributor

What kind of errors? I haven’t updated yet, will try in the next few days

@Kludex
Copy link
Sponsor Collaborator

Kludex commented Aug 25, 2022

If I'm not mistaken... I think the problem he mentions is that changing this behavior was a breaking change.

@orenwang
Copy link

@JarroVGIT Something like this:

pydantic.error_wrappers.ValidationError: 1 validation error for MyModel
  response
  none is not an allowed value (type=type_error.none.not_allowed)

But if you've never sent None as HTTP response, it shouldn't be a problem. If it is the case, the fix is easy: put Optional with whatever your response_model is.

@AlonMorgen
Copy link

AlonMorgen commented Aug 30, 2022

Hi, I have some error regarding this change triggered by ddtrace (https://github.com/DataDog/dd-trace-py) library wrapping my fastapi application, but I think its not really, related to Datadog:

Upon getting the error after returning None instead of my response_model class (which in my case was "bool"), the datadog library gets the exception object generated by fastapi with type pydantic.error_wrappers.ValidationError which makes sense, because my route returned "None" where it should return "bool". But, then when it tries to stringify the object using the built in str function, It gets an exception regarding the self.model field of the ValidationError object:
AttributeError("type object 'bool' has no attribute 'pydantic_model'"), here:

def errors(self) -> List['ErrorDict']:
        if self._error_cache is None:
            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))
        return self._error_cache

apparently, the "model" field is just a "class bool" type because of the original response_model I put in my endpoint, and it doesn't have config nor pydantic_model fields, hence the exception.

any clue about that?

@tiangolo
Copy link
Owner

tiangolo commented Mar 4, 2023

Thanks for the conversation all! ☕

@AlonMorgen if you're still having problems, please create a new GitHub Discussion following the template, with a replication example, etc. That way we would be able to help you.

@tiangolo tiangolo removed the reviewed label Mar 4, 2023
Repository owner locked and limited conversation to collaborators Mar 4, 2023
@tiangolo tiangolo converted this issue into discussion #9211 Mar 4, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

8 participants