Recursion error when deleting model object through admin

[Mark Wolf]

When I try to delete a specific object using the admin interface I get an error:

RuntimeError at /admin/gtd/node/1470/delete/
maximum recursion depth exceeded while calling a Python object

It appears to be trying to convert something to unicode but I don't know what. From the Django error message I can see that the object appears as <Node: [<span style="color: rgba(230, 138, 0, 1.0)"><strong>DFRD</strong></span>] Test todo item>, which doesn't seem to have any weird characters in it. The title is what I expect to be return from __str__(self) as described below.

If I delete a different object (which has similar HTML markup in it) from the same model it works with no errors. The model in question defines __str__(self) which returns the value from a models.TextField() attribute wrapped in some HTML (<span style="..."></span>) which is passed through mark_safe() before being returned. The model is decorated with @python_2_unicode_compatible. Following the python3 migration guide, I added from __future__ import unicode_literals at the top of my models.py and removed __unicode__(self) but no difference.

The stacktrace doesn't seem to go through my code anywhere.

I git-pulled the latest changes to stable/1.5.x from github and reinstalled Django after removed the previous installation from dist-packages but the problem persists.

I have not tried deleting this object pythonically; I figured I'd keep that object for now in case anyone needs me to reproduce the problem.

Stacktrace: ​https://gist.github.com/4148524

[Anssi Kääriäinen]

I was able to reproduce similar stack trace with:

from django.db import models
from django.utils.encoding import python_2_unicode_compatible
@python_2_unicode_compatible
class Foof(models.Model):
    def __unicode__(self):
        return unicode(self.id)
repr(Foof())

I don't get a similar stack trace if I define __str__ instead of __unicode__ or if I don't have python_2_unicode_compatible.

I have also seen the same stack trace somewhere else, don't remember where and why.

There seems to be something strange going on. In the following code

    def __str__(self):
        if not six.PY3 and hasattr(self, '__unicode__'):
            return force_text(self).encode('utf-8')
        

if I replace the force_text(self) with self.__unicode__() then the recursion goes away. But to me it seems calling the __unicode__ method is the only thing that force_text is actually doing (apart of a couple of isinstance checks).

In any case it seems the __unicode__ + force_text() form an endless loop in some cases. I am marking this as accepted as it seems likely there is something strange going on in here. Although it is possible this is some kind of user error after all.

[Aymeric Augustin]

Anssi first encountered the problem by checking out stable/1.4.x, running the pagination tests, checking out master, and running the pagination tests again.

(The pagination tests were moved from modeltests to regressiontests; the sequence above leaves stale .pyc files in modeltests/pagination.)

At least, that gives us a reproducible way to trigger this infinite recursion.

[Aymeric Augustin]

Indeed, replacing force_text(self) with self.__unicode__() makes the recursion go away. But replacing it with unicode(self) (ortype(self).__unicode__(self), which is how the interpreter computes unicode(self)) brings it back.

In the situation explained just above, it seems that Django will run the tests from the stale .pyc in modeltests, but the Article model from regressiontests is also loaded in the app cache, and that causes problems.

[Aymeric Augustin]

Here's what happens when a subclass of models.Model that doesn't define __str__ is decorated with python_2_unicode_compatible:

• when the decorator is executed:
  • its __unicode__ method points to the __str__ method from models.Model — which returns a str and not a unicode as expected by python_2_unicode_compatible
  • its __str__ method is a new method that calls __unicode__ and returns the result encoded as utf-8
• when attempting to create the unicode representation of an instance, say, obj:
  • the interpreter treats unicode(obj) as type(obj).__unicode__(obj)
  • in this case, that's actually models.Model.__str__(obj)
  • this function calls force_text(obj), which callsobj.__unicode__() — and that's also models.Model.__str__(obj), hence the infinite recursion.

---

I wish I could just remove the definition of models.Model.__str__, but after having spent years educating developers to write a __unicode__ method for their models, this isn't a viable option.

It's possible to catch this programming mistake and raise an explicit exception at runtime like this:

--- a/django/db/models/base.py
+++ b/django/db/models/base.py
@@ -416,6 +416,11 @@ class Model(six.with_metaclass(ModelBase, object)):
 
     def __str__(self):
         if not six.PY3 and hasattr(self, '__unicode__'):
+            if type(self).__unicode__ == Model.__str__:
+                klass_name = type(self).__name__
+                raise RuntimeError("%s.__unicode__ is aliased to __str__. Did"
+                                   " you apply @python_2_unicode_compatible"
+                                   " without defining __str__?" % klass_name)
             return force_text(self).encode('utf-8')
         return '%s object' % self.__class__.__name__
 

That ugly, but it works... It's more difficult to catch the exception at compile time because python_2_unicode_compatible is defined in django.utils.encoding, and I don't want to import django.db.models.Model there.

---

m3wolf, I have an explanation for why you can delete one instance and not another of the same model.

You probably have a foreign key pointing from an instance of another model to the instance you can't delete, and that other model has @python_2_unicode_compatible but no __str__ method. When Django tries to display that instance to warn you about the cascade deletion, it enters the infinite loop.

If you apply the patch above, Django should tell you which model has this issue. Could you test that and report your results here?

[Mark Wolf]

Thanks, aaugustine. Applied your patch and it correctly identified the offending model. Defined __str__ for that model and I can now delete the instance.

[Aymeric Augustin <aymeric.augustin@…>]

In 2ea80b94d7f6e0d25207d6b716c52ca4c57a02fb:

Fixed #19362 -- Detected invalid use of @python_2_unicode_compatible.

Thanks m3wolf for the report and akaariai for reproducing the problem.

[Aymeric Augustin <aymeric.augustin@…>]

In 71e5ad248e104f68a0d755cd5f76e86631607be5:

[1.5.x] Fixed #19362 -- Detected invalid use of @python_2_unicode_compatible.

Thanks m3wolf for the report and akaariai for reproducing the problem.

Backport of 2ea80b9.

[Aymeric Augustin]

#21198 suggests a better approach: raising an exception at compile time rather than at runtime.

[Aymeric Augustin <aymeric.augustin@…>]

In f0c7649b1692f8441eb9b9b923b2bed8e95f9185:

Fixed #21198 -- Prevented invalid use of @python_2_unicode_compatible.

Thanks jpic for the report and chmodas for working on a patch.

Reverts 2ea80b94. Refs #19362.

Conflicts:

tests/utils_tests/test_encoding.py

[Aymeric Augustin <aymeric.augustin@…>]

In 589dc49e129f63801c54c15e408c944a345b3dfe:

Fixed #21198 -- Prevented invalid use of @python_2_unicode_compatible.

Thanks jpic for the report and chmodas for working on a patch.

Reverts 2ea80b94. Refs #19362.