Opened 16 years ago
Closed 13 years ago
#8997 closed Bug (fixed)
Misleading error message when calling get_absolute_url
Reported by: | Owned by: | Julien Phalip | |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 1.2 |
Severity: | Normal | Keywords: | |
Cc: | pb@…, loryjn@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
If a model has a get_absolute_url method that has an error in it (such as a trivial syntax area like a typo in a field name), when you try to view the page, django reports the error as "<Foo> objects don't have get_absolute_url() methods".
This is very misleading and makes it difficult to debug the error if you are not especially familiar with django. I'm guessing the appropriate exceptions are being swallowed somewhere and turned into the wrong error message.
Attachments (2)
Change History (15)
comment:1 by , 16 years ago
comment:2 by , 16 years ago
Triage Stage: | Unreviewed → Design decision needed |
---|
comment:3 by , 16 years ago
Component: | Uncategorized → Database layer (models, ORM) |
---|
comment:4 by , 15 years ago
Cc: | added |
---|
comment:5 by , 15 years ago
Version: | 1.0 → 1.1 |
---|
How to make this happen:
Make sure DEBUG=True (otherwise it'll just 404).
Make a model object with a get_absolute_url() function with an error in it. Try a divide by zero, that's fun.
Hook the model up to the django admin.
Create one of these models.
Click the 'view on site' button.
This applies to Django 1.1 as well as 1.0, so I'm updating the version.
comment:6 by , 14 years ago
Version: | 1.1 → 1.2 |
---|
The test case proposed by GDorn appears to be fixed: creating a model with a get_absolute_url() function containing a divide by zero correctly returns a 500 Debug page when DEBUG=True. However the original bug (misleading error with a typo in a field name) is still present in Django 1.2.3. How to make this happen:
Create a model with a get_absolute_url function that references an invalid field, e.g.:
def get_absolute_url(self): return "/foo/bar/" + self.invalid_field
Hook the model up to the django admin. Create one of these models. Click the 'view on site' button.
Expected behavior: When DEBUG=True, display a 500 Debug page showing an AttributeError
in the model.
Actual output: Django reports a 404 error: "<Foo> objects don't have get_absolute_url() methods".
by , 14 years ago
Attachment: | get_absolute_url_patch.diff added |
---|
Patch to allow AttributeError to propagate + tests
comment:7 by , 14 years ago
Has patch: | set |
---|---|
Owner: | changed from | to
Added a patch with tests -- not sure if I put the tests in the correct location. By not catching the AttributeError
in shortcut() the error is allowed to propagate normally.
comment:8 by , 14 years ago
Cc: | added |
---|---|
Status: | new → assigned |
comment:9 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → Bug |
comment:10 by , 13 years ago
Easy pickings: | unset |
---|---|
Triage Stage: | Design decision needed → Accepted |
UI/UX: | unset |
Yes, the check for the existence of get_absolute_url
must be improved:
- either use
hasattr()
, as in the current patch; - or something like this:
try: get_absolute_url = obj.get_absolute_url except AttributeError: # whatever else: # do something with get_absolute_url
The first approach is shorter and usually more readable.
comment:11 by , 13 years ago
Patch needs improvement: | set |
---|
In this case the second technique looks better to me:
--- django/contrib/contenttypes/views.py (revision 17200) +++ django/contrib/contenttypes/views.py (working copy) @@ -14,9 +14,10 @@ except (ObjectDoesNotExist, ValueError): raise http.Http404("Content type %s object %s doesn't exist" % (content_type_id, object_id)) try: - absurl = obj.get_absolute_url() + get_absolute_url = obj.get_absolute_url except AttributeError: - raise http.Http404("%s objects don't have get_absolute_url() methods" % content_type.name) + raise http.Http404("%s objects don't have a get_absolute_url() method" % content_type.name) + absurl = get_absolute_url() # Try to figure out the object's domain, so we can do a cross-site redirect # if necessary.
The tests shouldn't monkey patch django.contrib.auth.models.Group
; they should define a new model with a buggy get_absolute_url
method.
comment:12 by , 13 years ago
Owner: | removed |
---|---|
Patch needs improvement: | unset |
Status: | assigned → new |
New patch added, I'd rather not split contenttypes tests (I hate those model/regression test splits).
We need more details to figure this one out. Where, exactly, are you seeing that error message? Is there a traceback? If so, post it. Is this only with a specific model or setup, or does this happen elsewhere?