Opened 8 years ago

Closed 5 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 6 years ago.
Patch to allow AttributeError to propagate + tests
8997-2.diff (2.3 KB) - added by Claude Paroz 5 years ago.
Patch following aaugustin example

Download all attachments as: .zip

Change History (15)

comment:1 Changed 8 years ago by Jacob

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 Changed 8 years ago by Jacob

Triage Stage: UnreviewedDesign decision needed

comment:3 Changed 8 years ago by Thejaswi Puthraya

Component: UncategorizedDatabase layer (models, ORM)

comment:4 Changed 7 years ago by anonymous

Cc: pb@… added

comment:5 Changed 7 years ago by Sam Thompson

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 Changed 6 years ago by joshlory

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".

Changed 6 years ago by joshlory

Attachment: get_absolute_url_patch.diff added

Patch to allow AttributeError to propagate + tests

comment:7 Changed 6 years ago by joshlory

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 Changed 6 years ago by joshlory

Cc: loryjn@… added
Status: newassigned

comment:9 Changed 6 years ago by Luke Plant

Severity: Normal
Type: Bug

comment:10 Changed 5 years ago by Aymeric Augustin

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
    
Last edited 5 years ago by Aymeric Augustin (previous) (diff)

comment:11 Changed 5 years ago by Aymeric Augustin

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.

Changed 5 years ago by Claude Paroz

Attachment: 8997-2.diff added

Patch following aaugustin example

comment:12 Changed 5 years ago by Claude Paroz

Owner: joshlory deleted
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 Changed 5 years ago by Julien Phalip

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