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

Using PEP 649 annotations with status code 204 No Content and return type None raises AssertionError #9424

Open
1 task done
hofrob opened this issue Apr 21, 2023 · 12 comments · May be fixed by #9425 or #5860
Open
1 task done

Comments

@hofrob
Copy link

hofrob commented Apr 21, 2023

Privileged issue

  • I'm @tiangolo or he asked me directly to create an issue here.

Issue Content

Reproduce

  1. create FastAPI app
  2. create route with status code 204 No Content
  3. add return type None to said route
  4. add future annotations (PEP 649)
from __future__ import annotations

import http
import logging

from fastapi import FastAPI


app = FastAPI()


@app.post("/", status_code=http.HTTPStatus.NO_CONTENT)
async def root() -> None:
    logging.info("endpoint called.")

Expected

The file above should boot up fine when running uvicorn filename:app --reload

Actual

This raises an error: AssertionError: Status code 204 must not have a response body

full traceback

Traceback (most recent call last):
  File "/<python_location>/lib/python3.11/multiprocessing/process.py", line 314, in _bootstrap
    self.run()
  File "/<python_location>/lib/python3.11/multiprocessing/process.py", line 108, in run
    self._target(*self._args, **self._kwargs)
  File "/<project_dir>/venv/lib/python3.11/site-packages/uvicorn/_subprocess.py", line 76, in subprocess_started
    target(sockets=sockets)
  File "/<project_dir>/venv/lib/python3.11/site-packages/uvicorn/server.py", line 59, in run
    return asyncio.run(self.serve(sockets=sockets))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/<python_location>/lib/python3.11/asyncio/runners.py", line 190, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "/<python_location>/lib/python3.11/asyncio/runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "uvloop/loop.pyx", line 1517, in uvloop.loop.Loop.run_until_complete
  File "/<project_dir>/venv/lib/python3.11/site-packages/uvicorn/server.py", line 66, in serve
    config.load()
  File "/<project_dir>/venv/lib/python3.11/site-packages/uvicorn/config.py", line 471, in load
    self.loaded_app = import_from_string(self.app)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/<project_dir>/venv/lib/python3.11/site-packages/uvicorn/importer.py", line 21, in import_from_string
    module = importlib.import_module(module_str)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/<python_location>/lib/python3.11/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen importlib._bootstrap>", line 1206, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1178, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1149, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 690, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 940, in exec_module
  File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
  File "/<project_dir>/main.py", line 12, in <module>
    @app.post("/", status_code=http.HTTPStatus.NO_CONTENT)
     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/<project_dir>/venv/lib/python3.11/site-packages/fastapi/routing.py", line 661, in decorator
    self.add_api_route(
  File "/<project_dir>/venv/lib/python3.11/site-packages/fastapi/routing.py", line 600, in add_api_route
    route = route_class(
            ^^^^^^^^^^^^
  File "/<project_dir>/venv/lib/python3.11/site-packages/fastapi/routing.py", line 401, in __init__
    assert is_body_allowed_for_status_code(
AssertionError: Status code 204 must not have a response body

Current workarounds

  • remove from __future__ import annotations
  • or remove the return type None

PRs

There was a PR posted in the discussion #6060 (comment)

After stepping through the code, I think it might make sense to return early in fastapi.dependencies.utils.get_typed_annotationfastapi.dependencies.utils.get_typed_return_annotation if annotation == "None" (PR incoming).

@hofrob hofrob linked a pull request Apr 21, 2023 that will close this issue
@Munalula-Sikazwe
Copy link

Munalula-Sikazwe commented May 11, 2023

According to the provided link in the issue #9424, you're returning a string in the body " "

@app.post("/", status_code=http.HTTPStatus.NO_CONTENT)
async def root() -> str:
    logging.info("endpoint called.")
    return ""

Even if this was edit the code to

@app.post("/", status_code=http.HTTPStatus.NO_CONTENT)
async def root() -> None:
    logging.info("endpoint called.")
    return None

The error is thrown because you are still returning something in body as json though it may not be valid
e.g null
With the third approach of not returning anything like

@app.post("/", status_code=http.HTTPStatus.NO_CONTENT)
async def root() -> None:
    logging.info("endpoint called.")

The server does not seem to complain one bit , maybe there's a special case or more information should be provided to resolve this issue .

@hofrob
Copy link
Author

hofrob commented May 12, 2023

I already submitted a PR and it was approved (#9425). You can try the test there (with and without the change). The test fails on my machine as expected.

I deleted the repo (it wasn't up-to-date and I don't think it adds anything). The code in the issue already tells the whole story.

@Munalula-Sikazwe
Copy link

Oh ok I think this issue should be closed then .

@linpan
Copy link

linpan commented Jun 11, 2023

just like NoReturn will have same error.

@hofrob
Copy link
Author

hofrob commented Jun 11, 2023

Did you test this with my PR? I can add a test later and see if that's also fixed.

No idea when or if this will be merged though.

@hariacharya80
Copy link

hariacharya80 commented Jun 17, 2023

@hofrob firstly, thanks for #9425 it will be ease for dev experience but ideally that was an indeed an intended behavior. The -> None syntax is used to indicate that the function does not return anything. This is different from returning None, which is a value.

In your case, you are trying to return a 204 No Content response, which is used to indicate that the request was successful, but there is no content to return.

However, you are also using the -> None syntax, which is causing the response body to be set to None. This is not allowed for the status code 204, so you are getting an AssertionError.

To fix this error, you need to remove the -> None syntax from the root function. Here is an example:

@app.get("/", status_code = 204)
async def root():
    logging.info('endpoint called.')

This code will define a route that returns a 204 No Content response without a response body.

@Munalula-Sikazwe The server does not have a problem probably because of not using the annotations?

from __future__ import annotations

Maybe it's time for this issue to be closed?

@hofrob
Copy link
Author

hofrob commented Jun 17, 2023

The -> None syntax is used to indicate that the function does not return anything. This is different from returning None, which is a value.

This is incorrect.

  1. every function in python returns something; there are no real void functions in Python
  2. annotating the return value of a function is more correct than not annotating it; even if it's None

So the conclusion should be:

  • accept None as return annotation
  • allow it for http status 204

Please reconsider.

@hariacharya80
Copy link

annotating the return value of a function is more correct than not annotating it; even if it's None

I don't disagree but in the this scenario, allowing the -> None syntax for HTTP status 204 will introduces ambiguity regarding the presence or absence of a response body. For clarity and HTTP compliance, excluding response body for status 204, ensures consistency and avoids client confusion.

@hofrob
Copy link
Author

hofrob commented Jun 18, 2023

Well, let's see what the FastAPI devs think.

@hariacharya80
Copy link

Well, let's see what the FastAPI devs think.

Yeah, that's cool this way we have a guideline further because you know this is quite controversial and I follow what my team or the project requires me to personally.

@Kludex
Copy link
Sponsor Collaborator

Kludex commented Jun 18, 2023

I think None should be a valid return type hint for 204 endpoints.

I also think that anything different than None (or lack of return statement, which conceptually is the same, but since we retrieve the type hints at runtime, is a bit different, and that's why not using a type hint works as expected here) should raise an error.

EDIT: Also, this is in the "Issues" tab. 😁👍

@jrobbins-LiveData
Copy link

An explicit return NONE, a bare return, or "falling off the end of the function" all return None, and since the return type hint is really about what the function returns (and not, strictly speaking) about the function's role as a 204 endpoint, -> None should be the required return type hint on such a function. See this discussion.

@Kludex Kludex linked a pull request Jul 18, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants