Skip to content

Conversation

@erl987
Copy link
Contributor

@erl987 erl987 commented May 24, 2023

Bug

Requests with an empty body to the endpoint _dash-update-component are triggering an 500 Internal Server error response. This issue has been observed in operational practice, web crawlers seem to pick up this endpoint and send a request with an empty body. Consequently, the cloud environment reports 500 Internal Server errors.

This can be reproduced with the Demo Four app in this repository. Just send this request:

GET http://127.0.0.1:8000/django_plotly_dash/app/LiveInput/_dash-update-component
Accept: application/json

We get:

GET http://127.0.0.1:8000/django_plotly_dash/app/LiveInput/_dash-update-component

HTTP/1.1 500 Internal Server Error

Expected behavior

The endpoint should respond with a 400 Bad Request error.

Fix

The error occurs because the JSON deserialization of the empty (or otherwise invalid) body is failing, and this exception is not caught. It should be possible to simply catch the exceptions that can be thrown in this line.

With the changes in this pull request, we get this response:

GET http://127.0.0.1:8000/django_plotly_dash/app/LiveInput/_dash-update-component

HTTP/1.1 400 Bad Request

@delsim
Copy link
Contributor

delsim commented May 29, 2023

@erl987 thanks for this. Is there a reason to prefer the 400 response over (for example) using an empty body and/or doing nothing, thus returning a 200 code to indicate success in updating nothing?

@erl987
Copy link
Contributor Author

erl987 commented May 30, 2023

@delsim No, there is no specific reason. I can change to status 200.

@erl987
Copy link
Contributor Author

erl987 commented Jun 3, 2023

@delsim I changed to return HTTP status code 200. Now we get:

GET http://127.0.0.1:8000/django_plotly_dash/app/LiveInput/_dash-update-component

HTTP/1.1 200 OK

<Response body is empty>

@GibbsConsulting GibbsConsulting merged commit 279d650 into GibbsConsulting:master Aug 17, 2023
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.

3 participants