Opened 7 years ago

Closed 3 years ago

#8997 closed Bug (fixed)

Misleading error message when calling get_absolute_url

Reported by: Rob <digital_illuminati@…> Owned by: julien
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 4 years ago.
Patch to allow AttributeError to propagate + tests
8997-2.diff (2.3 KB) - added by claudep 3 years ago.
Patch following aaugustin example

Download all attachments as: .zip

Change History (15)

comment:1 Changed 6 years ago by jacob

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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

  • Triage Stage changed from Unreviewed to Design decision needed

comment:3 Changed 6 years ago by thejaswi_puthraya

  • Component changed from Uncategorized to Database layer (models, ORM)

comment:4 Changed 5 years ago by anonymous

  • Cc pb@… added

comment:5 Changed 5 years ago by GDorn

  • Version changed from 1.0 to 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 Changed 4 years ago by joshlory

  • Version changed from 1.1 to 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".

Changed 4 years ago by joshlory

Patch to allow AttributeError to propagate + tests

comment:7 Changed 4 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 4 years ago by joshlory

  • Cc loryjn@… added
  • Status changed from new to assigned

comment:9 Changed 4 years ago by lukeplant

  • Severity set to Normal
  • Type set to Bug

comment:10 Changed 3 years ago by aaugustin

  • Easy pickings unset
  • Triage Stage changed from Design decision needed to 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
    
Last edited 3 years ago by aaugustin (previous) (diff)

comment:11 Changed 3 years ago by aaugustin

  • 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 3 years ago by claudep

Patch following aaugustin example

comment:12 Changed 3 years ago by claudep

  • Owner joshlory deleted
  • Patch needs improvement unset
  • Status changed from assigned to new

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

comment:13 Changed 3 years ago by julien

  • Owner set to julien
  • Resolution set to fixed
  • Status changed from new to closed

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