Code

Opened 9 months ago

Closed 2 months ago

#20745 closed Bug (fixed)

Document the template language's silencing of TypeError

Reported by: robin Owned by: Baptiste Mispelon <bmispelon@…>
Component: Template system Version: 1.5
Severity: Normal Keywords:
Cc: bmispelon@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Say I create myapp/admin.py which contains:

from django.contrib import admin
from myapp.models import MyModel

class MyModelAdmin(admin.ModelAdmin):
    
    list_display = ('pk','abc')

    def abc(self, user):
        return 999+None

admin.site.register(MyModel, MyModelAdmin)

Then when you visit http://localhost:8000/admin/myapp/mymodel/ a TypeError will raise because you cannot add 999 and None together. This is the expected outcome.

However if I add templates/admin/myapp/mymodel/change_list.html which contains:

{% extends "admin/change_list.html" %}

{% block content %}
    {{ block.super }}
{% endblock %}

The TypeError will not appear and the page renders but without any content, which is not the expected outcome.

This ticket seems similar to https://code.djangoproject.com/ticket/18169

Attachments (0)

Change History (14)

comment:1 Changed 9 months ago by bmispelon

  • Cc bmispelon@… added
  • Component changed from Uncategorized to Template system
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

I made a small template tag and managed to reproduced the issue easily: https://gist.github.com/bmispelon/6031523

Note that only TypeError seems to trigger the bug (I tested with KeyError or AttributeError and they're both propagated correctly).

Digging a bit deeper, it seems this is a regression introduced by 4397c587a43ff9bfddd295d48d850676778c6e77.

comment:2 Changed 7 months ago by anonymous

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

This actually works for me on trunk/master using bmispelon's gist, so it should have been fixed in some point between 1.6 or 1.7. The only change i did to the gist was using __builtin__ instead of builtins for python2.7 compatibility. I also tried raising a simple TypeError.

I'm marking this a closed, reopen if it's still happening.

comment:3 Changed 7 months ago by aaugustin

  • Resolution fixed deleted
  • Status changed from closed to new

I'm not aware of any fixes in this area over the last week. Since it was reproduced by a core dev and not reproduced by an anonymous I'd rather err on the side of caution until we get a third opinion.

comment:4 Changed 7 months ago by bmispelon

Just tried it again and the issue is indeed still present.

Note that if you used the code from my earlier comment, the way to test if the issue is still present is to try and render the bar.html template.
The correct behavior is that rendering this template should raise a TypeError.
As of now (1.7, 1.6 or 1.5), no exception is raised, which is what the original report describes.

I should have provided an actual testcase but I couldn't find how to register a template library for a test.

comment:5 Changed 7 months ago by bmispelon

  • Easy pickings set
  • Summary changed from {{ block.super }} silences/hides errors to Document the template language's silencing of TypeError

After some digging, I think I'm starting to understand what's happening here.

When django finds a callable element in a template (which {{ block.super }} is, since it refers to a BlockNode's super method), it attempts to call it (without passing any argument), wrapping the calling in a try/except and catching a TypeError (which happens when you call a function/method without enough arguments).

In our case, a TypeError is raised from inside the function and subsequently caught and silenced in the except clause.

Before 4397c587a43ff9bfddd295d48d850676778c6e77, any exception raised while rendering a template would be wrapped in a TemplateSyntaxError.
This TemplateSyntaxError would then not be caught by the aforementionned except TypeError clause and would bubble up.
Note that this would only happen with settings.TEMPLATE_DEBUG = True (this discrepancy between the two values of TEMPLATE_DEBUG is actually one of the reason why this change was introduced in the first place).

This is therefore a limitation of django's implementation, which stems from the fact that it can't distinguish between a TypeError caused by calling a function without enough arguments and a TypeError raised from inside the function.
This limitation is noted in a comment in the code [1], but as far as I can tell, it's not documented.

I'm leaving this ticket open but I see it as a documentation issue, unless someone can come up with a clever way of side-stepping the problem but I'm not sure there is one.

Since this details is somehow technical, I think a good place for it would be this section: https://docs.djangoproject.com/en/1.5/ref/templates/api/#variables-and-lookups

[1] https://github.com/django/django/blob/028db9750357d504c55a7ac686c9abaa3c847ac6/django/template/base.py#L791-L794

comment:6 Changed 7 months ago by kedmiston

  • Owner changed from nobody to kedmiston
  • Status changed from new to assigned

comment:7 Changed 7 months ago by kedmiston

  • Easy pickings unset
  • Owner kedmiston deleted
  • Status changed from assigned to new

comment:8 Changed 7 months ago by aaugustin

We can distinguish between the two kinds of TypeError with inspect.getcallargs.

We recently did it to fix a similar problem in django/contrib/auth/__init__.py.

Last edited 7 months ago by aaugustin (previous) (diff)

comment:9 Changed 7 months ago by bmispelon

  • Has patch set

Interesting, I didn't know about getcallargs.

Here's a pull request that uses it to let TypeError bubble up when it needs to:

https://github.com/django/django/pull/1602

comment:10 Changed 7 months ago by claudep

  • Triage Stage changed from Accepted to Ready for checkin

comment:11 Changed 7 months ago by Baptiste Mispelon <bmispelon@…>

  • Owner set to Baptiste Mispelon <bmispelon@…>
  • Resolution set to fixed
  • Status changed from new to closed

In 28a571348bca9c5a3c137e495e7d3c9349a5bd56:

Fix #20745: Don't silence TypeError raised inside templates.

Thanks to robin for the report and claudep for the review.

comment:12 Changed 2 months ago by pczerkas@…

inspect.getcallargs is unavailable in Python 2.6

comment:13 Changed 2 months ago by pczerkas@…

  • Resolution fixed deleted
  • Status changed from closed to new

comment:14 Changed 2 months ago by bmispelon

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

Hi,

The fix for this issue will be part of the 1.7 release which only supports versions of Python above 2.7 [1] so this shouldn't be a problem.

[1] https://docs.djangoproject.com/en/dev/releases/1.7/#python-compatibility

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.