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

#181 - ensure bytes are always returned from .body() method #182

Merged
merged 4 commits into from
Apr 17, 2021
Merged

#181 - ensure bytes are always returned from .body() method #182

merged 4 commits into from
Apr 17, 2021

Conversation

nathanglover
Copy link
Contributor

I believe this closes #181.

All tests pass, and I have tested the solution on a live lambda function being severed by ALB.

The type annotations suggest that def body(self) -> bytes: should ALWAYS return bytes. However the code (and tests) sometimes returns None. In these cases I forced the None to return an empty byte string: b"". As a result most of the methods now look like this:

    @property
    def body(self) -> bytes:
        body = self.trigger_event.get("body", b"")

        if not body:
            body = b""
        if self.trigger_event.get("isBase64Encoded", False):
            return base64.b64decode(body)
        if not isinstance(body, bytes):
            body = body.encode()

        return body

If that is incorrect I can update my PR to reflect that bytes OR None can be returned from def body(self) and remove the code that forces None to become b"".

Let me know if I should add anything else! Have loved using Mangum these past few months, made moving my Django application to serverless pretty easy.

@jordaneremieff
Copy link
Owner

Hi @nathanglover, thanks for the PR!

Yep, you're right. Before the recent refactoring this was handled like this:

is_binary = event.get("isBase64Encoded", False)
body = event.get("body") or b""
if is_binary:
    body = base64.b64decode(body)
elif not isinstance(body, bytes):
    body = body.encode()

The intent of the or in body = event.get("body") or b"" was for handling this case. Could you update your PR to do body = self.trigger_event.get("body") or b"" to replace the additional if statement?

Then I think this is good to merge, thank you.

@nathanglover
Copy link
Contributor Author

nathanglover commented Apr 16, 2021

@jordaneremieff You're welcome. Removed the extra if and added the or as you asked. That way is a bit cleaner.

@jordaneremieff jordaneremieff merged commit 619790e into jordaneremieff:main Apr 17, 2021
@jordaneremieff
Copy link
Owner

@nathanglover great, thanks!

@nathanglover nathanglover deleted the 181-body-bytes-TypeError branch April 18, 2021 00:23
khamaileon pushed a commit to khamaileon/mangum that referenced this pull request Jan 13, 2024
…thod (jordaneremieff#182)

* jordaneremieff#181 - ensure bytes are always returned from .body method

* jordaneremieff#181 - remove extra if

* jordaneremieff#181 - black formatting

* jordaneremieff#181 - remove extra if
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

Successfully merging this pull request may close these issues.

Bug - handler.body returns a str resulting in TypeError: a bytes-like object is required, not 'str'
2 participants