[Django] #19362: Recursion error when deleting model object through admin
Django
-------------------------------+-------------------------
Reporter: m3wolf | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.5-alpha-1
Severity: Normal | Keywords: python2.7.3
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+-------------------------
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
--
Ticket URL: <https://code.djangoproject.com/ticket/19362>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: m3wolf | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.5-alpha-1
Keywords: python2.7.3 | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by akaariai):
* needs_better_patch: => 0
* needs_docs: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/19362#comment:1>
Django
Reporter: m3wolf | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.5-alpha-1
Keywords: python2.7.3 | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/19362#comment:2>
Django
Reporter: m3wolf | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.5-alpha-1
Keywords: python2.7.3 | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
Indeed, replacing `force_text(self)` with `self.__unicode__()` makes the
recursion go away. But replacing it with `unicode(self)`
(or`type(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.
--
Ticket URL: <https://code.djangoproject.com/ticket/19362#comment:3>
Django
Reporter: m3wolf | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.5-alpha-1
Keywords: python2.7.3 | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
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 calls`obj.__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?
--
Ticket URL: <https://code.djangoproject.com/ticket/19362#comment:4>
Django
Reporter: m3wolf | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.5-alpha-1
Keywords: python2.7.3 | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by m3wolf):
Thanks, aaugustine. Applied your patch and it correctly identified the
offending model. Defined `__str__` for that model and I can now delete the
instance.
--
Ticket URL: <https://code.djangoproject.com/ticket/19362#comment:5>
Django
Reporter: m3wolf | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: python2.7.3 | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* has_patch: 0 => 1
* component: Uncategorized => Python 2
* severity: Normal => Release blocker
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/19362#comment:6>
Django
Reporter: m3wolf | Owner: nobody
Type: Bug | Status: closed
Component: Python 2 | Version: 1.5-alpha-1
Keywords: python2.7.3 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Aymeric Augustin <aymeric.augustin@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"2ea80b94d7f6e0d25207d6b716c52ca4c57a02fb"]:
{{{
#!CommitTicketReference repository=""
revision="2ea80b94d7f6e0d25207d6b716c52ca4c57a02fb"
Fixed #19362 -- Detected invalid use of @python_2_unicode_compatible.
Thanks m3wolf for the report and akaariai for reproducing the problem.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/19362#comment:7>
Django
Reporter: m3wolf | Owner: nobody
Type: Bug | Status: closed
Component: Python 2 | Version: 1.5-alpha-1
Severity: Release blocker | Resolution: fixed
Keywords: python2.7.3 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Aymeric Augustin <aymeric.augustin@…>):
In [changeset:"71e5ad248e104f68a0d755cd5f76e86631607be5"]:
{{{
#!CommitTicketReference repository=""
revision="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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/19362#comment:8>
Django
Reporter: m3wolf | Owner: nobody
Type: Bug | Status: closed
Component: Python 2 | Version: 1.5-alpha-1
Severity: Release blocker | Resolution: fixed
Keywords: python2.7.3 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
#21198 suggests a better approach: raising an exception at compile time
rather than at runtime.
--
Ticket URL: <https://code.djangoproject.com/ticket/19362#comment:9>
Django
Reporter: m3wolf | Owner: nobody
Type: Bug | Status: closed
Component: Python 2 | Version: 1.5-alpha-1
Severity: Release blocker | Resolution: fixed
Keywords: python2.7.3 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Aymeric Augustin <aymeric.augustin@…>):
In [changeset:"f0c7649b1692f8441eb9b9b923b2bed8e95f9185"]:
{{{
#!CommitTicketReference repository=""
revision="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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/19362#comment:10>
Django
Reporter: m3wolf | Owner: nobody
Type: Bug | Status: closed
Component: Python 2 | Version: 1.5-alpha-1
Severity: Release blocker | Resolution: fixed
Keywords: python2.7.3 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Aymeric Augustin <aymeric.augustin@…>):
In [changeset:"589dc49e129f63801c54c15e408c944a345b3dfe"]:
{{{
#!CommitTicketReference repository=""
revision="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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/19362#comment:11>