GibbsConsulting / django-plotly-dash

Expose plotly dash apps as django tags
MIT License
548 stars 124 forks source link

Avoid 500 Internal Server Error if empty body is posted to _dash-update_component #458

Closed erl987 closed 1 year ago

erl987 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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

erl987 commented 1 year ago

@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>