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.

Sign up for GitHub

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

Jump to bottom

DRF returning HTML on 404 even with drf-standardized-errors installed #44

Closed
scottgigante opened this issue Aug 10, 2023 · 5 comments
Closed

DRF returning HTML on 404 even with drf-standardized-errors installed #44

scottgigante opened this issue Aug 10, 2023 · 5 comments

Comments

@scottgigante
Copy link

I've noticed that the standardized errors don't work if the route doesn't exist. I've made a minimum reproducible example based on the DRF tutorial, which successfully returns the standardized error if you ask for a primary key that doesn't exist:

➜  curl -H "Accept: application/json" "http://127.0.0.1:8000/users/-1/"
{"type":"client_error","errors":[{"code":"not_found","detail":"Not found.","attr":null}]}%

but still returns html if you ask for a route that doesn't exist

➜  curl -H "Accept: application/json" "http://127.0.0.1:8000/user/2/"

<!doctype html>
<html lang="en">
<head>
  <title>Not Found</title>
</head>
<body>
  <h1>Not Found</h1><p>The requested resource was not found on this server.</p>
</body>
</html>

mre.zip
@ghazi-git
Copy link
Owner

Thanks for raising the issue @scottgigante and providing a sample project.

The problem is that requests to non-existing routes do not reach drf, they are handled by django. So, the drf-standardized-errors exception handler of is never called.

To return the expected format, you can create a custom handler404 that returns the json you want.

This issue got me thinking: what should drf-standardized-errors do about error responses returned by django without reaching drf? like this one or those that happen in middleware... At the very least, some documentation is in order. Maybe provide default error handlers (400,403,404,500) matching what django provides out of the box?

@scottgigante
Copy link
Author

scottgigante commented Aug 10, 2023 •
edited

That would be great! On my end it appears this works -- though I'm not entirely sure as I'm not certain how to trigger core Django exceptions for anything other than 404 and 500.

# project/app/exceptions.py
import drf_standardized_errors.handler
from rest_framework.decorators import api_view
from rest_framework.exceptions import APIException

@api_view()
def error_handler(request, exception=APIException()):
    """
    Generic Django error handler.
    """
    return drf_standardized_errors.handler.exception_handler(exception, {'request': request})

and then

# project/project/urls.py
handler400 = "app.exceptions.error_handler"
handler403 = "app.exceptions.error_handler"
handler404 = "app.exceptions.error_handler"
handler500 = "app.exceptions.error_handler"

@ghazi-git
Copy link
Owner

reusing the exception handler in the error handler seems like a nice idea. For now, there is a small issue: for 500 errors, you might see duplicate errors in sentry and duplicate error logs. That's because the handler sends the got_request_exception and django also does that as well, and sentry relies on that to report errors

@scottgigante
Copy link
Author

Good point. Not a huge issue but a little suboptimal. I'd be interested if you come up with something better!

@ghazi-git
Copy link
Owner

I went ahead and documented this issue in the readme and linked to this issue.

I'm closing the issue as resolved as the solution is up to the developer. Scott's answer is a good start assuming the project provides a backend API only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@scottgigante @ghazi-git