Opened 3 years ago

Closed 3 years ago

#32195 closed New feature (fixed)

Improve error messages from forgetting to call .as_view() on a CBV

Reported by: Angus Holder Owned by: Angus Holder
Component: Core (URLs) Version: 3.1
Severity: Normal Keywords:
Cc: 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 (last modified by Angus Holder)

We can detect early-on that the user has forgotten to call .as_view() on their CBV when passing it into path(). For:

urlpatterns = [
    path('home', HomeView)
]

The error currently happens only when you first load the route (rather than when constructing the routes), and looks like

Internal Server Error: /
Traceback (most recent call last):
  File "C:\Users\Angus\.virtualenvs\django-WBTbdxDv\lib\site-packages\django\core\handlers\exception.py", line 47, in inner
    response = get_response(request)
  File "C:\Users\Angus\.virtualenvs\django-WBTbdxDv\lib\site-packages\django\core\handlers\base.py", line 179, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
TypeError: __init__() takes 1 positional argument but 2 were given

Which is especially hard to work out given that the traceback doesn't even include any of the user's own code, and this is an error that's easy for beginners to run into when first using CBVs.

My PR changes it to fail early, inside the call to django.urls.path(), with a clear error:

URL route 'foo' should pass in 'EmptyCBView.as_view()' instead of 'EmptyCBView'

Pull request: https://github.com/django/django/pull/13682

Change History (9)

comment:1 by Angus Holder, 3 years ago

Description: modified (diff)

comment:2 by Carlton Gibson, 3 years ago

Needs documentation: set
Patch needs improvement: set
Triage Stage: UnreviewedAccepted
Type: UncategorizedNew feature

I agree with the motivation here. As per comments on the PR, I think we need to leverage the System check framework for this, rather than raising runtime errors, but other than that +1.

comment:3 by Craig Smith, 3 years ago

Would it be worthwhile to change the error message for if the name keyword argument is mistakenly passed as a positional argument? Currently the message is:

ValueError: dictionary update sequence element #0 has length 1; 2 is required


comment:4 by Angus Holder, 3 years ago

Needs documentation: unset
Patch needs improvement: unset

comment:5 by Mariusz Felisiak, 3 years ago

Owner: changed from nobody to Angus Holder
Patch needs improvement: set
Status: newassigned
UI/UX: unset

comment:6 by Jacob Walls, 3 years ago

Patch needs improvement: unset

Author appears caught up on feedback.

comment:7 by Mariusz Felisiak, 3 years ago

Triage Stage: AcceptedReady for checkin

comment:8 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

In 8f89454:

Refs #32195 -- Added path() test for invalid view.

comment:9 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

Resolution: fixed
Status: assignedclosed

In 3e73c65:

Fixed #32195 -- Added system check for invalid view in path() and improved error messages.

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