pixelpassion / django-saas-boilerplate

A Django + django-rest-framework + Heroku + SaaS application boilerplate
MIT License
51 stars 10 forks source link

Do send 5xx with JSON #32

Open jensneuhaus opened 4 years ago

jensneuhaus commented 4 years ago

☝️Basics

On staging:

Bildschirmfoto 2019-12-18 um 19 10 08 Bildschirmfoto 2019-12-18 um 19 10 19

💭 Implementation

📋 Todos

jensneuhaus commented 4 years ago

Please check if this is solved with the Error handling of #35 @vladborisov182

AntonDnepr commented 4 years ago

@itisallgood please check if this is solved

itisallgood commented 4 years ago
REST framework's views handle various exceptions, and deal with returning appropriate error responses.

The handled exceptions are:

Subclasses of APIException raised inside REST framework.
Django's Http404 exception.
Django's PermissionDenied exception.
In each case, REST framework will return a response with an appropriate status code and content-type. The body of the response will include any additional details regarding the nature of the error.

https://www.django-rest-framework.org/api-guide/exceptions/#custom-exception-handling

Vlad has implemented it for cases stated above (it is custom error exception for rest API), but it won't work for 500 issues with template and other that do not regard REST API. If that is what we want to achieve than we don't need to implement anything else. @jensneuhaus @AntonDnepr

AntonDnepr commented 4 years ago

@itisallgood we need that custom exception handling to work only with REST API, so no worries on other parts.

jensneuhaus commented 4 years ago

@AntonDnepr @itisallgood Yes, it only needs to be in the API. BUT we need to make sure it handles the whole workflow and any Django-related 500, when using the REST API are also handled nicely as part of the Response. We had cases before, where a Django 500 error happened, WHEN using the API and a page was rendered instead of a JSON response.

Please check the image above, it was an API call but returned HTML. Unfortunately the error is not represent on Sentry anymore.

This is the code we had on the DARE project at that time:

from rest_auth.views import (
    PasswordResetView,
)

path("password/reset/", PasswordResetView.as_view(), name=PASS_RESET_URL_NAME),`

django-rest-auth was used with version 0.9.5 (the same as we have here)

@itisallgood Could you spend a few minutes to try to somehow send wrong POST or so to reproduce a 500? You could also overwrite PasswordResetView and raise an Exception.

itisallgood commented 4 years ago

Sure, I will try to send wrong post request to reproduce 500

itisallgood commented 4 years ago

@jensneuhaus I customized PasswordResetView as this and got custom exception handling on exception image image

itisallgood commented 4 years ago

If you will encounter error again in sentry, please let me know and we will try to reproduce it

jensneuhaus commented 4 years ago

Okay, thanks!