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

[BUG] 404 handler override not working as expected #1390

Open
ErikPohl444 opened this issue Jan 12, 2025 · 4 comments
Open

[BUG] 404 handler override not working as expected #1390

ErikPohl444 opened this issue Jan 12, 2025 · 4 comments

Comments

@ErikPohl444
Copy link

Describe the bug

I have a simple demo Django-Ninja app working thanks to a helpful onboarding documentation on the related docs.

All of the endpoints in my simple demo work as expected.

However, the following code in my urls.py file does not produce the expected result based on the docs:

from django.http import Http404
@api.exception_handler(Http404)
def page_not_found(request, exc):
    return api.create_response(
        request,
        {"message": "Please retry later"},
        status=404,
    ) 

I expect my handler to pick up 404s when the server is running, but I get the default

Not Found
The requested resource was not found on this server.

Did a lot of research beforehand and tried a variety of overrides, but none worked-- so am documenting the method from the dn docs:
https://django-ninja.dev/guides/errors/

My code is here:
https://github.com/ErikPohl444/django_ninja_playground

Working control case:
http://127.0.0.1:8000/api/hello_html?name=you

Not working experimental case:
http://127.0.0.1:8000/api/this_endpoint_is_undefined

I've done similar captures of the 404 in Flask and in Django, but I'm hitting a wall here.

It is either:
a) something is buggy with d-n [unlikely]
b) something can be added to for newbies like me to the documentation [potentially]
c) my code is buggy [in which case I might want to make a PR with doc changes --see b -- for folks like me :) ]

Versions (please complete the following information):

  • Python version: [e.g. 3.6]
    Python 3.12.0

  • Django version: [e.g. 4.0]
    Django==5.1.4

  • Django-Ninja version: [e.g. 0.16.2]
    django-ninja==1.3.0

  • Pydantic version: [e.g. 1.9.0]
    pydantic==2.10.4

@crbunney
Copy link

My own digging around when I was writing tests for our error handling suggests that the handling for Http404 is affected by settings.DEBUG, and when DEBUG == True, the expected handling is bypassed in order to display the "yellow screen of death".

@vitalik
Copy link
Owner

vitalik commented Feb 13, 2025

@ErikPohl444 I guess 404 is a bit special case

django ninja fully relies on django url router to find the right callback

so if you never defined a url (this_endpoint_is_undefined) it will only be handled by default django behavior (CommonMiddleware I bealive)

django ninja will catch this 404 only in case when you raised it inside your operation:

@api.get('/some/{id}')
def some(request, id: int):
        obj = get_object_or_404(Model, id=id)

it does not make sense for django-ninja to overrule default dispatcher as it most likely will suffer in performance

for your case when you really need a special case for /api/ 404s you can just define as very last method catch all and raise 404

api = NinjaAPI()
...add routers and endpoints...

# last - catch all
api.api_operation(["POST", "GET", "PATCH", "PUT"], "/{path:endpoint}", include_in_schema=False)
def catch_all_callback(request, endpoint: str):
     raise Http404(f'{endpoint} does not exist')

@crbunney
Copy link

Or you could override handler404?

and add an if statement that returns something different depending on the path requested:

def handler404(request: HttpRequest, exception: Exception, template_name="404.html"):
    if request.path.startswith("/api/"):
        content = {"message": "Please retry later"}
    else:
        content = render_to_string(template_name, request=request)
    return HttpResponseNotFound(content)

@vitalik
Copy link
Owner

vitalik commented Feb 13, 2025

@crbunney yep - that looks perfect solution for this case

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

3 participants