Opened 16 years ago

Closed 13 years ago

#8997 closed Bug (fixed)

Misleading error message when calling get_absolute_url

Reported by: Rob <digital_illuminati@…> 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)

get_absolute_url_patch.diff (2.8 KB ) - added by joshlory 14 years ago.
Patch to allow AttributeError to propagate + tests
8997-2.diff (2.3 KB ) - added by Claude Paroz 13 years ago.
Patch following aaugustin example

Download all attachments as: .zip

Change History (15)

comment:1 by Jacob, 16 years ago

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?

comment:2 by Jacob, 16 years ago

Triage Stage: UnreviewedDesign decision needed

comment:3 by Thejaswi Puthraya, 16 years ago

Component: UncategorizedDatabase layer (models, ORM)

comment:4 by anonymous, 15 years ago

Cc: pb@… added

comment:5 by Sam Thompson, 15 years ago

Version: 1.01.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 joshlory, 14 years ago

Version: 1.11.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 joshlory, 14 years ago

Attachment: get_absolute_url_patch.diff added

Patch to allow AttributeError to propagate + tests

comment:7 by joshlory, 14 years ago

Has patch: set
Owner: changed from nobody to joshlory

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 joshlory, 14 years ago

Cc: loryjn@… added
Status: newassigned

comment:9 by Luke Plant, 14 years ago

Severity: Normal
Type: Bug

comment:10 by Aymeric Augustin, 13 years ago

Easy pickings: unset
Triage Stage: Design decision neededAccepted
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.

Version 0, edited 13 years ago by Aymeric Augustin (next)

comment:11 by Aymeric Augustin, 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.

by Claude Paroz, 13 years ago

Attachment: 8997-2.diff added

Patch following aaugustin example

comment:12 by Claude Paroz, 13 years ago

Owner: joshlory removed
Patch needs improvement: unset
Status: assignednew

New patch added, I'd rather not split contenttypes tests (I hate those model/regression test splits).

comment:13 by Julien Phalip, 13 years ago

Owner: set to Julien Phalip
Resolution: fixed
Status: newclosed

In [17328]:

Fixed #8997 -- Prevented the contenttypes shortcut view to hide errors occurring inside the get_absolute_url() method. Thanks to Rob for the report and to joshlory, Aymeric Augustin and Claude Paroz for the patch.

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