Skip to content

fix(event_handler): fix forward references resolution in OpenAPI #5885

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

Conversation

xdxindustries
Copy link
Contributor

Issue number: #5884

Summary

Changes

Fix type resolution for forward references in Annotated type hints to correctly handle body parameters in API Gateway handlers. Currently, when using quoted type annotations with Annotated and Body, parameters are incorrectly treated as query parameters instead of body parameters due to improper globals resolution in get_typed_signature().

The fix updates the globals retrieval logic to properly resolve forward references in type annotations.

User experience

Before:

@app.post("/users")
def create_user(
    user_create_request: 'Annotated[UserCreate, Body()]',
) -> dict:
    return {"message": "success"}

Results in a 422 error treating user_create_request as a missing query parameter, even when the JSON body is correctly provided.

After:

# Same code works as expected

Correctly identifies the parameter as a body parameter and properly validates the incoming JSON request body.

Checklist

Is this a breaking change?

No, this is a bug fix that restores expected functionality for forward references in type annotations.

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@xdxindustries xdxindustries requested a review from a team January 19, 2025 22:32
@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jan 19, 2025
Copy link

boring-cyborg bot commented Jan 19, 2025

Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need.
In the meantime, check out the #python channel on our Powertools for AWS Lambda Discord: Invite link

@github-actions github-actions bot added the bug Something isn't working label Jan 20, 2025
Copy link

codecov bot commented Jan 20, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.17%. Comparing base (4a8d4e8) to head (ddb46b5).
Report is 2 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #5885   +/-   ##
========================================
  Coverage    96.17%   96.17%           
========================================
  Files          232      232           
  Lines        10941    10941           
  Branches      2023     2023           
========================================
  Hits         10522    10522           
  Misses         329      329           
  Partials        90       90           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@leandrodamascena leandrodamascena changed the title fix(event_handler): fix typo in OpenAPI type resolution parser" fix(event_handler): fix forward references resolution Jan 20, 2025
@leandrodamascena leandrodamascena changed the title fix(event_handler): fix forward references resolution fix(event_handler): fix forward references resolution in OpenAPI Jan 20, 2025
@leandrodamascena
Copy link
Contributor

Hi @xdxindustries! Thank you SO MUCH for fixing this! This also paves the way to solve this other bug: #5098.

I'll send some PRs to resolve this other bug definitively and add more tests. I hope to make a release today or tomorrow and will ping you to test.

Copy link
Contributor

@leandrodamascena leandrodamascena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved!!!

@leandrodamascena leandrodamascena merged commit 57becbb into aws-powertools:develop Jan 20, 2025
21 checks passed
Copy link

boring-cyborg bot commented Jan 20, 2025

Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience!

@xdxindustries
Copy link
Contributor Author

@leandrodamascena as you work on the other issues I will say there was one thing that took a lot of debugging when working on complicated types/this issue. that might help your fix/debugging other bugs, you really want the @app decorator directly next to your function, opposite of docs examples. Otherwise types were not getting resolved correctly even when getting the globals, as it ends up getting the globals of the wrapper:

@app.post(...)
@tracer.capture_method
def function_with_forwardTypes(some_var: "Annotated[ ....etc"

^--- This fails as when resolving the globals it gets the globals of the tracer capture function instead of the target function
/---- this works
@tracer.capture_method
@app.post(...)
def function_with_forwardTypes(some_var: "Annotated[ ....etc"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working event_handlers size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
3 participants