falconry / falcon

The no-magic web data plane API and microservices framework for Python developers, with a focus on reliability, correctness, and performance at scale.
https://falcon.readthedocs.io/en/stable/
Apache License 2.0
9.51k stars 937 forks source link

JSONHandler with orjson.dumps breaks in ASGI app #2100

Closed vytas7 closed 2 years ago

vytas7 commented 2 years ago

As an optimization, Falcon opts to de/serialize media using sync methods of the standard handlers shipped with Falcon even for an ASGI app, where we can be sure this is safe.

Something seems to break when using JSONHandler with orjson.dumps (which then takes the "binary" path as orjson dumps directly to a bytestring, not str as stdlib):

import falcon
import falcon.asgi
import falcon.media
import falcon.testing
import orjson

class Resource:
    async def on_get(self, req, resp):
        resp.media = {'message': 'Hello'}

def create_app():
    json_handler = falcon.media.JSONHandler(dumps=orjson.dumps)

    app = falcon.asgi.App()
    app.resp_options.media_handlers[falcon.MEDIA_JSON] = json_handler
    app.add_route('/', Resource())

    return app

def test_app():
    client = falcon.testing.TestClient(create_app())

    resp = client.get('/')
    assert resp.json == {'message': 'Hello'}

:arrow_down:

ERROR    falcon:app.py:1047 [FALCON] Unhandled exception in ASGI app
Traceback (most recent call last):
  File "/tmp/.venv/lib/python3.8/site-packages/falcon/asgi/app.py", line 451, in __call__
    resp._media_rendered = serialize_sync(resp._media)
  File "falcon/media/json.py", line 184, in falcon.media.json.JSONHandler._serialize_b
TypeError: _serialize_b() takes exactly 3 positional arguments (2 given)
CaselIT commented 2 years ago

nice catch. I'm assuming we just need to have content_type specify a default.

I think this is a good candidate for 3.1.1

vytas7 commented 2 years ago

Resolved fixed in https://github.com/falconry/falcon/pull/2102