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

APIGatewayProxyEvent json_body throws exception if no body #881

Closed
Tracked by #1009
bb-rrogers opened this issue Dec 9, 2021 · 5 comments
Closed
Tracked by #1009

APIGatewayProxyEvent json_body throws exception if no body #881

bb-rrogers opened this issue Dec 9, 2021 · 5 comments

Comments

@bb-rrogers
Copy link

Heya All,

Not sure if this would be seen as an issue or not, but I thought I would log it anyways. When using the APIGatewayProxyEvent.json_body function I have found that if there is no body, it will throw a TypeError:
[ERROR] TypeError: the JSON object must be str, bytes or bytearray, not NoneType

I feel on the fence whether it's better to validate that there is a body before attempting to json_body but I guess with the return type hint being Any I was maybe expecting a None or empty string.

Expected Behavior

I expect the json_body should return an empty string if the object is not present in the payload

Current Behavior

Function throws a TypeError because json.loads cannot handle None objects.

Steps to Reproduce (for bugs)

app = ApiGatewayResolver()

@app.get("/test")
def get_test():
  app.current_event.json_body

def handler(event: dict, context: dict) -> dict:
    return app.resolve(event=event, context=context)

Including a REST request with an None body.

Again, not sure if this should be considered a bug or not, looking for guidance here, is a minor issue.

Environment

  • Powertools version used: 1.22.0
  • Packaging format (Layers, PyPi): Layers
  • AWS Lambda function runtime: Python 3.9
  • Debugging logs
@bb-rrogers bb-rrogers added bug Something isn't working triage Pending triage from maintainers labels Dec 9, 2021
@heitorlessa
Copy link
Contributor

hey @bb-rrogers thanks a lot for raising this. We can indeed do a much better job here - I suspect we implicitly trust customers would only call as they know their payload better, however a type Any doesn't help .. despite JSON Values not having an actual agreement on a generic type in Python

@heitorlessa heitorlessa added help wanted Could use a second pair of eyes/hands p2 and removed triage Pending triage from maintainers labels Dec 10, 2021
@michaelbrewer
Copy link
Contributor

@heitorlessa what should we be returning ?

@heitorlessa
Copy link
Contributor

Return None if we can't decode, and update to Optional[Any].

MyPy used to be okay with Any also meaning a None, but that changed since an Union is safer (Optional[Any] == Union[T, None])

Thanks a lot @michaelbrewer

@bb-rrogers
Copy link
Author

Thanks both. The intent was to stir up discussion here but through further reflection I think maybe it's better to keep it as is, I can see positives for and against but will let you guys ultimately decide. I just wanted to log this to figure out the reasoning here.

@heitorlessa
Copy link
Contributor

Np- closing this then. We could spend the effort elsewhere like Idempotency serialisation to support dataclasses and Parser (Pydantic) models

@heitorlessa heitorlessa removed bug Something isn't working help wanted Could use a second pair of eyes/hands p2 labels Dec 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants