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

Cannot return bare Response when enable_validation=True #4085

Closed
nlykkei opened this issue Apr 7, 2024 · 9 comments · Fixed by #4101
Closed

Cannot return bare Response when enable_validation=True #4085

nlykkei opened this issue Apr 7, 2024 · 9 comments · Fixed by #4101
Assignees
Labels
bug Something isn't working

Comments

@nlykkei
Copy link
Contributor

nlykkei commented Apr 7, 2024

Expected Behaviour

Response objects are not validated, but return as-is. Only Pydantic classes should be validated

Current Behaviour

Response objects are validated as if they are Pydantic classes, resulting in the following error:

Unable to generate pydantic-core schema for <class 'aws_lambda_powertools.event_handler.api_gateway.Response'>

Code snippet

from aws_lambda_powertools import Logger, Tracer
from aws_lambda_powertools.event_handler import APIGatewayRestResolver, Response
from aws_lambda_powertools.event_handler.openapi.models import Contact, Server
from aws_lambda_powertools.event_handler.openapi.params import Path, Body, Query
from aws_lambda_powertools.utilities.typing import LambdaContext

tracer = Tracer()

logger = Logger()

app = APIGatewayRestResolver(enable_validation=True)

@app.get(
    "/helloworld",
    summary="Hello, World!",
    description="Hello, World!",
    response_description="Hello, World!",
    responses={
        200: {"description": "Hello, World!"},
    },
    tags=["hello-world"],
)
@tracer.capture_method
def helloworld() -> Response:
    logger.info("Hello, World!")
    return Response(status_code=200, body={"message": "Hello, World!"}, content_type="application/json")

@logger.inject_lambda_context
@tracer.capture_lambda_handler
def lambda_handler(event: dict, context: LambdaContext) -> dict:
    return app.resolve(event, context)

Possible Solution

No response

Steps to Reproduce

Run the provided sample using e.g. SAM

Powertools for AWS Lambda (Python) version

latest

AWS Lambda function runtime

3.12

Packaging format used

PyPi

Debugging logs

Invalid lambda response received: Invalid API Gateway Response Keys: {'stackTrace', 'errorMessage', 'requestId', 'errorType'} in {'errorMessage': "Unable to generate
pydantic-core schema for <class 'aws_lambda_powertools.event_handler.api_gateway.Response'>. Set `arbitrary_types_allowed=True` in the model_config to ignore this
error or implement `__get_pydantic_core_schema__` on your type to fully support it.\n\nIf you got this error by calling handler(<some type>) within
`__get_pydantic_core_schema__` then you likely need to call `handler.generate_schema(<some type>)` since we do not call `__get_pydantic_core_schema__` on `<some
type>` otherwise to avoid infinite recursion.\n\nFor further information visit https://errors.pydantic.dev/2.6/u/schema-for-unknown-type", 'errorType':
'PydanticSchemaGenerationError', 'requestId': 'f15f198a-50f7-4166-8f55-2db18e5f096a', 'stackTrace': ['  File "/var/task/aws_lambda_powertools/logging/logger.py", line
447, in decorate\n    return lambda_handler(event, context, *args, **kwargs)\n', '  File "/var/task/aws_lambda_powertools/tracing/tracer.py", line 317, in decorate\n
response = lambda_handler(event, context, **kwargs)\n', '  File "/var/task/app.py", line 96, in lambda_handler\n    return app.resolve(event, context)\n', '  File
"/var/task/aws_lambda_powertools/event_handler/api_gateway.py", line 1821, in resolve\n    response = self._resolve().build(self.current_event, self._cors)\n', '
File "/var/task/aws_lambda_powertools/event_handler/api_gateway.py", line 1928, in _resolve\n    return self._call_route(route, route_keys)  # pass fn args\n', '
File "/var/task/aws_lambda_powertools/event_handler/api_gateway.py", line 2006, in _call_route\n    route(router_middlewares=self._router_middlewares, app=self,
route_arguments=route_arguments),\n', '  File "/var/task/aws_lambda_powertools/event_handler/api_gateway.py", line 400, in __call__\n    return
self._middleware_stack(app)\n', '  File "/var/task/aws_lambda_powertools/event_handler/api_gateway.py", line 1292, in __call__\n    return
self.current_middleware(app, self.next_middleware)\n', '  File "/var/task/aws_lambda_powertools/event_handler/middlewares/base.py", line 121, in __call__\n    return
self.handler(app, next_middleware)\n', '  File "/var/task/aws_lambda_powertools/event_handler/middlewares/openapi_validation.py", line 80, in handler\n
route.dependant.path_params,\n', '  File "/var/task/aws_lambda_powertools/event_handler/api_gateway.py", line 449, in dependant\n    self._dependant =
get_dependant(path=self.openapi_path, call=self.func, responses=self.responses)\n', '  File "/var/task/aws_lambda_powertools/event_handler/openapi/dependant.py", line
207, in get_dependant\n    _add_return_annotation(dependant, endpoint_signature)\n', '  File "/var/task/aws_lambda_powertools/event_handler/openapi/dependant.py",
line 238, in _add_return_annotation\n    param_field = analyze_param(\n', '  File "/var/task/aws_lambda_powertools/event_handler/openapi/params.py", line 965, in
analyze_param\n    field = _create_model_field(field_info, type_annotation, param_name, is_path_param)\n', '  File
"/var/task/aws_lambda_powertools/event_handler/openapi/params.py", line 1098, in _create_model_field\n    return create_response_field(\n', '  File
"/var/task/aws_lambda_powertools/event_handler/openapi/params.py", line 1067, in create_response_field\n    return ModelField(**kwargs)  # type: ignore[arg-type]\n',
'  File "<string>", line 6, in __init__\n', '  File "/var/task/aws_lambda_powertools/event_handler/openapi/compat.py", line 86, in __post_init__\n
self._type_adapter: TypeAdapter[Any] = TypeAdapter(\n', '  File "/var/task/pydantic/type_adapter.py", line 211, in __init__\n    core_schema = _get_schema(type,
config_wrapper, parent_depth=_parent_depth + 1)\n', '  File "/var/task/pydantic/type_adapter.py", line 81, in _get_schema\n    schema = gen.generate_schema(type_)\n',
'  File "/var/task/pydantic/_internal/_generate_schema.py", line 490, in generate_schema\n    schema = self._generate_schema(obj)\n', '  File
"/var/task/pydantic/_internal/_generate_schema.py", line 721, in _generate_schema\n    schema = self._generate_schema_inner(obj)\n', '  File
"/var/task/pydantic/_internal/_generate_schema.py", line 727, in _generate_schema_inner\n    return self._annotated_schema(obj)\n', '  File
"/var/task/pydantic/_internal/_generate_schema.py", line 1697, in _annotated_schema\n    schema = self._apply_annotations(source_type, annotations)\n', '  File
"/var/task/pydantic/_internal/_generate_schema.py", line 1765, in _apply_annotations\n    schema = get_inner_schema(source_type)\n', '  File
"/var/task/pydantic/_internal/_schema_generation_shared.py", line 82, in __call__\n    schema = self._handler(__source_type)\n', '  File
"/var/task/pydantic/_internal/_generate_schema.py", line 1847, in new_handler\n    schema = metadata_get_schema(source, get_inner_schema)\n', '  File
"/var/task/pydantic/_internal/_generate_schema.py", line 1843, in <lambda>\n    lambda source, handler: handler(source)\n', '  File
"/var/task/pydantic/_internal/_schema_generation_shared.py", line 82, in __call__\n    schema = self._handler(__source_type)\n', '  File
"/var/task/pydantic/_internal/_generate_schema.py", line 1746, in inner_handler\n    schema = self._generate_schema(obj)\n', '  File
"/var/task/pydantic/_internal/_generate_schema.py", line 721, in _generate_schema\n    schema = self._generate_schema_inner(obj)\n', '  File
"/var/task/pydantic/_internal/_generate_schema.py", line 747, in _generate_schema_inner\n    return self.match_type(obj)\n', '  File
"/var/task/pydantic/_internal/_generate_schema.py", line 834, in match_type\n    return self._unknown_type_schema(obj)\n', '  File
"/var/task/pydantic/_internal/_generate_schema.py", line 393, in _unknown_type_schema\n    raise PydanticSchemaGenerationError(\n']}
2024-04-07 11:52:16 127.0.0.1 - - [07/Apr/2024 11:52:16] "GET /helloworld HTTP/1.1" 502 -
@nlykkei nlykkei added bug Something isn't working triage Pending triage from maintainers labels Apr 7, 2024
@rubenfonseca
Copy link
Contributor

@nlykkei sorry about that and thank you for opening an issue, I'll look at this first thing in the morning! Godaften :)

@nlykkei
Copy link
Contributor Author

nlykkei commented Apr 8, 2024

Hi @rubenfonseca,

Thanks for your reply.

I solved it by providing a type argument to Response, i.e.:

class HelloWorldResponse(BaseModel):
    message: str

@app.get(
    "/helloworld",
    summary="Hello, World!",
    description="Hello, World!",
    response_description="Hello, World!",
    responses={
        200: {"description": "Hello, World!"},
    },
    tags=["hello-world"],
)
@tracer.capture_method
def helloworld() -> Response[HelloWorldResponse]:
    logger.info("Hello, World!")
    return Response(status_code=200, body=HelloWorldResponse(message="Hello, World!"), content_type="application/json")

https://github.com/aws-powertools/powertools-lambda-python/blob/develop/aws_lambda_powertools/event_handler/api_gateway.py#L218

Nothing is mentioned about this in the docs, neither is there any documentation in code about the usage of the ResponseT type parameter, so that might be confusing to new users 🙂

Godmorgen :)

@rubenfonseca rubenfonseca self-assigned this Apr 8, 2024
@rubenfonseca
Copy link
Contributor

Thank you for that update! Response is a generic class and when we don't specify the concrete type (like you did on the second example), the validation middleware breaks because it cannot infer the underlying type.

I want to make this clearer in the docs.

But looking at the original code, what would be the expected behaviour here? Since we cannot determine the concrete type, we could always skip the validation of the returned data, but I'm worried that this might be unexpected for some users. Any thoughts?

@rubenfonseca rubenfonseca removed the triage Pending triage from maintainers label Apr 8, 2024
@nlykkei
Copy link
Contributor Author

nlykkei commented Apr 8, 2024

I like your reasoning. Maybe it would be wrong to skip validation for Response without type argument. However, I did expect that data validation would only apply to Pydantic classes, so that validation would be skipped for Response.

But looking at the original code, what would be the expected behaviour here? Since we cannot determine the concrete type, we could always skip the validation of the returned data, but I'm worried that this might be unexpected for some users. Any thoughts?

Yes, I would update the docs. For new users, there is nothing linking ResponseT to data validation. I had to figure this out myself :)

@rubenfonseca
Copy link
Contributor

Great! Let me work on the docs then!

@rubenfonseca
Copy link
Contributor

@nlykkei can you check the PR to see if it looks ok? Any feedback is appreciated :)

@rubenfonseca rubenfonseca changed the title Cannot return Response when enable_validation=True for APIGatewayRestResolver: Unable to generate pydantic-core schema for <class 'aws_lambda_powertools.event_handler.api_gateway.Response'> Cannot return bare Response when enable_validation=True Apr 10, 2024
Copy link
Contributor

⚠️COMMENT VISIBILITY WARNING⚠️

This issue is now closed. Please be mindful that future comments are hard for our team to see.

If you need more assistance, please either tag a team member or open a new issue that references this one.

If you wish to keep having a conversation with other community members under this issue feel free to do so.

@github-actions github-actions bot added the pending-release Fix or implementation already in dev waiting to be released label Apr 10, 2024
@nlykkei
Copy link
Contributor Author

nlykkei commented Apr 10, 2024

@rubenfonseca It looks great, very clear explanation to new users 🚀 Thank you for the contribution!

Copy link
Contributor

This is now released under 2.37.0 version!

@github-actions github-actions bot removed the pending-release Fix or implementation already in dev waiting to be released label Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Shipped
2 participants