Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#24143 closed Cleanup/optimization (fixed)

Documentation uses uninstantiated Http404s

Reported by: Keryn Knight Owned by: nobody
Component: Documentation Version: dev
Severity: Normal Keywords:
Cc: django@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I've always used instantiated Http404 exceptions in my stuff, but it doesn't seem to be a well-defined practice, and isn't the demonstrated way in the documentation.

In DEBUG, when an Http404 is encountered it is re-routed to the 404 handler, which renders using TECHNICAL_404_TEMPLATE, and an opportunity arises to provide hints about what code was causing the error, much like I asked for in #22756 (if there's a theme to my tickets, it's that I value debugging simplicity), which puts the view causing the 404 into the result. By suggesting instantiating 404s with messages, we can help users help themselves further.

Given this view:

def myview(request, obj_id):
    obj = get_object_or_404(MyModel, pk=obj_id)
    if not mymodel.published:
        Http404("Unpublished")
    if not mymodel.is_accessible_via(request.method):
        Http404("Not via this method")
    if not mymodel.allows_user(request.user):
        Http404("Not found for this user")
    # ...

It is easy to see which 404 was thrown in the technical debug view because force_bytes is called on the exception (which inherits __str__ from Exception) and the reason is rendered as the error message if no urlpatterns were tried.

Without instantiating (raise Http404 vs raise Http404('message')) it becomes a debugging guessing game as to which condition threw the exception (ignoring the merit of the above conditions, which are total garbage)

I'm suggesting that the places in the documentation where bare, un-instantiated Http404s are raised is updated to reflect this idea, which is already used in places like get_object_or_404 anyway.

Change History (5)

comment:1 by Keryn Knight, 9 years ago

Has patch: set

comment:2 by Tim Graham, 9 years ago

Patch needs improvement: set
Triage Stage: UnreviewedAccepted

As I mentioned on the PR, I think this ticket's rationale could be added to the docs as well.

comment:3 by Berker Peksag, 9 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

PR #3907 LGTM.

comment:4 by Tim Graham <timograham@…>, 9 years ago

Resolution: fixed
Status: newclosed

In 726a9550db5129badc1c44809b0bed728fa1ad90:

Fixed #24143 -- Encouraged use of Http404 messages for debugging.

comment:5 by Tim Graham <timograham@…>, 9 years ago

In bd08cfca6ff04e7cec940f5b59e97cdcceddcc69:

[1.7.x] Fixed #24143 -- Encouraged use of Http404 messages for debugging.

Backport of 726a9550db5129badc1c44809b0bed728fa1ad90 from master

Note: See TracTickets for help on using tickets.
Back to Top