Skip to content

Commit

Permalink
fix: support Response[None] for 204 handler (#2915)
Browse files Browse the repository at this point in the history
This PR improves our detection of invalid response annotations for handlers that return a 204 response.

We have always checked that the return type was `NoneType`, now we additionally check if the return type is a subclass of `Response` that has been specialized with `NoneType` as its content.

Closes #2914
  • Loading branch information
peterschutt authored Dec 18, 2023
1 parent 21ab235 commit 6228640
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 4 deletions.
21 changes: 20 additions & 1 deletion litestar/handlers/http_handlers/_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@

from litestar.enums import HttpMethod
from litestar.exceptions import ValidationException
from litestar.response import Response
from litestar.status_codes import HTTP_200_OK, HTTP_201_CREATED, HTTP_204_NO_CONTENT
from litestar.types.builtin_types import NoneType

if TYPE_CHECKING:
from litestar.app import Litestar
from litestar.background_tasks import BackgroundTask, BackgroundTasks
from litestar.connection import Request
from litestar.datastructures import Cookie, ResponseHeader
from litestar.response import Response
from litestar.types import (
AfterRequestHookHandler,
ASGIApp,
Expand All @@ -22,12 +23,14 @@
ResponseType,
TypeEncodersMap,
)
from litestar.typing import FieldDefinition

__all__ = (
"create_data_handler",
"create_generic_asgi_response_handler",
"create_response_handler",
"get_default_status_code",
"is_empty_response_annotation",
"normalize_headers",
"normalize_http_method",
)
Expand Down Expand Up @@ -206,4 +209,20 @@ def get_default_status_code(http_methods: set[Method]) -> int:
return HTTP_200_OK


def is_empty_response_annotation(return_annotation: FieldDefinition) -> bool:
"""Return whether the return annotation is an empty response.
Args:
return_annotation: A return annotation.
Returns:
Whether the return annotation is an empty response.
"""
return (
return_annotation.is_subclass_of(NoneType)
or return_annotation.is_subclass_of(Response)
and return_annotation.has_inner_subclass_of(NoneType)
)


HTTP_METHOD_NAMES = {m.value for m in HttpMethod}
4 changes: 2 additions & 2 deletions litestar/handlers/http_handlers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
create_generic_asgi_response_handler,
create_response_handler,
get_default_status_code,
is_empty_response_annotation,
normalize_http_method,
)
from litestar.openapi.spec import Operation
Expand All @@ -41,7 +42,6 @@
ResponseType,
TypeEncodersMap,
)
from litestar.types.builtin_types import NoneType
from litestar.utils import ensure_async_callable
from litestar.utils.predicates import is_async_callable
from litestar.utils.warnings import warn_implicit_sync_to_thread, warn_sync_to_thread_with_async_callable
Expand Down Expand Up @@ -552,7 +552,7 @@ def _validate_handler_function(self) -> None:

if (
self.status_code < 200 or self.status_code in {HTTP_204_NO_CONTENT, HTTP_304_NOT_MODIFIED}
) and not return_type.is_subclass_of(NoneType):
) and not is_empty_response_annotation(return_type):
raise ImproperlyConfiguredException(
"A status code 204, 304 or in the range below 200 does not support a response body. "
"If the function should return a value, change the route handler status code to an appropriate value.",
Expand Down
36 changes: 35 additions & 1 deletion tests/unit/test_handlers/test_http_handlers/test_validations.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from pathlib import Path
from typing import Dict
from types import ModuleType
from typing import Callable, Dict

import pytest

Expand Down Expand Up @@ -110,3 +111,36 @@ def test_function_2(self, data: DataclassPerson) -> None: # type: ignore
Litestar(route_handlers=[test_function_2])

test_function_2.on_registration(Litestar())


@pytest.mark.parametrize(
("return_annotation", "should_raise"),
[
("None", False),
("Response[None]", False),
("int", True),
("Response[int]", True),
("Response", True),
],
)
def test_204_response_annotations(
return_annotation: str, should_raise: bool, create_module: Callable[[str], ModuleType]
) -> None:
module = create_module(
f"""
from litestar import get
from litestar.response import Response
from litestar.status_codes import HTTP_204_NO_CONTENT
@get(path="/", status_code=HTTP_204_NO_CONTENT)
def no_response_handler() -> {return_annotation}:
pass
"""
)

if should_raise:
with pytest.raises(ImproperlyConfiguredException):
Litestar(route_handlers=[module.no_response_handler])
return

Litestar(route_handlers=[module.no_response_handler])

0 comments on commit 6228640

Please sign in to comment.