Code

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#16935 closed Bug (fixed)

Misleading message 'super object has no attribute __getattr__' on AttributError during SimpleTemplateResponse.render

Reported by: isagalaev Owned by: isagalaev
Component: Template system Version: 1.3
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Consider this code:

from django.template import Template
from django.template.response import SimpleTemplateResponse

class A(object):
    def __unicode__(self):
        return None.something

def index(request):
    return SimpleTemplateResponse(Template('{{ var }}'), {'var': A()})

This code emulates a situation of any AttributeError raised during initial template rendering from within SimpleTemplateReponse.render. The real error "NoneType object has no attribute something" is masked by a very weird-looking "super object has no attribute getattr". The traceback also points inside SimpleTemplateResponse.getattr which has no relation whatsoever to the real point of error.

Attachments (1)

16935.diff (1.2 KB) - added by isagalaev 3 years ago.
Patch

Download all attachments as: .zip

Change History (5)

Changed 3 years ago by isagalaev

Patch

comment:1 Changed 3 years ago by isagalaev

  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

It looks like the error is caused by an AttributeError raised while accessing property SimpleTemplateReponse.rendered_content from with super(...).__getattr__ call. In this case any internal AttributeError is indistinguishable from the one of absence of the __getattr__ itself.

I've reimplemented the code using __getattribute__ that doesn't introduce this side-effect.

I'm in doubts about tests… Should we really test such a strange corner case and if yes, how? It seems that the only way is to check for certain words in an exception message which seems fragile.

comment:2 Changed 3 years ago by lukeplant

  • Triage Stage changed from Unreviewed to Accepted

There is a double problem here - the code in question should use getattr on the super object, not __getattr__ which doesn't exist.

However, I'm proposing we remove it altogether - the point of the code is to aid debugging in an obscure case, but actually massively hinders debugging in a (probably) vastly more common case. See https://groups.google.com/forum/#!topic/django-developers/Vmyi0b6VJgo

I imagine your approach is fine, but I'd prefer not to implement __getattribute__ unless we really need it, and it seems overkill here.

comment:3 Changed 3 years ago by isagalaev

(I posted to django-dev@ but it stuck in moderation queue for some reason.)

I completely agree. My natural approach was to just fix the code without changing semantics but may be I was overly cautious :-). And your point on using getattr() is correct but it didn't help masking of the original exception so I went the __getattribute__ way.

comment:4 Changed 3 years ago by lukeplant

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

In [16917]:

Fixed #16935 - misleading message if AttributeError escapes during SimpleTemplateResponse.render

Thanks to isagalaev for the report.

As discussed on django-devs, this reverts some of the changes in [16568]
i.e. the addition of SimpleTemplateResponse.__getattr__, because this
makes it much harder to debug the common case of an AttributeError somewhere
during the rendering of a SimpleTemplateResponse.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.