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

request parsing with ujson #78

Closed
Midnighter opened this issue Sep 30, 2018 · 3 comments
Closed

request parsing with ujson #78

Midnighter opened this issue Sep 30, 2018 · 3 comments

Comments

@Midnighter
Copy link

Since you are advertising the use of ujson, I was expecting that it is being used as a drop-in everywhere in starlette. When I post to a simple route handler without any body content, I see an exception coming from the standard library, however. Is this a special case when there is no body content or is ujson only used in serializing the JSONResponse?

Code:

@app.route('/', methods=["POST"], )
async def search(request: Request) -> JSONResponse:
    query = await request.json()
    return query

Traceback:

web_1  |     query = await request.json()
web_1  |   File "/usr/local/lib/python3.7/site-packages/starlette/requests.py", line 96, in json
web_1  |     self._json = json.loads(body)
web_1  |   File "/usr/local/lib/python3.7/json/__init__.py", line 348, in loads
web_1  |     return _default_decoder.decode(s)
web_1  |   File "/usr/local/lib/python3.7/json/decoder.py", line 337, in decode
web_1  |     obj, end = self.raw_decode(s, idx=_w(s, 0).end())
web_1  |   File "/usr/local/lib/python3.7/json/decoder.py", line 355, in raw_decode
web_1  |     raise JSONDecodeError("Expecting value", s, err.value) from None
web_1  | json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)
@alexbotello
Copy link
Contributor

...is ujson only used in serializing the JSONResponse?

This is the answer right here.

@tomchristie
Copy link
Member

Closing this off. In fact I think we should possibly add a UJSONResponse, and only use ujson when that's done so explicitly. (Since it has some edge-cases where its behavior differs.)

@gsakkis
Copy link

gsakkis commented Apr 18, 2019

and only use ujson when that's done so explicitly.

It would be quite handy if there was a way to (explicitly) tell starlette to use ujson everywhere. I only realized this isn't the case by profiling a high volume route that's already using UJSONResponse and seeing json.loads taking more time than the rest of the (non-trivial) route-specific logic combined.

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

4 participants