Opened 13 months ago

Closed 13 months ago

Last modified 13 months ago

#27956 closed Bug (fixed)

Template error raised in an {% extends %} parent template shows incorrect source location on debug page

Reported by: Ling-Xiao Yang Owned by: nobody
Component: Template system Version: master
Severity: Normal Keywords:
Cc: ling-xiao.yang@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Ling-Xiao Yang)

Please see comment:2 for updated description. Below is the original description.

================================================

Hello,

It seems that this problem only concerns Django 1.9 and 1.10. The problem has been fixed in the newest 1.11b1, but I am not sure if it has been backported to older versions.

How to verify:

  1. Start a new Django project, create two template html files, and make sure that template A uses `{% include %} tag to include the content of template B.
  2. Make an error inside template B. For example, in the screenshots below, I am passing in a model Question and using {{ Question.objects.all }} in template B (inner.html) to trigger an error.
  3. Complete a view function that renders template A, and add a url route for it. Try access this route with DEBUG=True.
  4. You will see the debug interface. In the section "Error during template rendering," you will see the error line being highlighted. The position and line number will be correct, but the error will be pointed to template A, while it should be pointed to template B where it actually happens.

Expected behavior:
I attached two screenshots, showing the expected behavior (using Django 1.11b1 and 1.8.11) and the actual behavior (using Django 1.10.6).

Note:
I discovered this problem while investigating the issue 902 of Django debug toolbar (on GitHub).
It is acceptable to invalidate this ticket if the fix has been ported to the branch Django 1.10.x, or it is mentioned elsewhere in the documentation.

Attachments (3)

1-8-11.png (68.2 KB) - added by Ling-Xiao Yang 13 months ago.
Django 1.8.11 (correct)
1-10-6.png (77.8 KB) - added by Ling-Xiao Yang 13 months ago.
Django 1.10.6 (not correct)
1-11-b1.png (67.3 KB) - added by Ling-Xiao Yang 13 months ago.
Django 1.11b1 (correct)

Download all attachments as: .zip

Change History (12)

Changed 13 months ago by Ling-Xiao Yang

Attachment: 1-8-11.png added

Django 1.8.11 (correct)

Changed 13 months ago by Ling-Xiao Yang

Attachment: 1-10-6.png added

Django 1.10.6 (not correct)

Changed 13 months ago by Ling-Xiao Yang

Attachment: 1-11-b1.png added

Django 1.11b1 (correct)

comment:1 Changed 13 months ago by Tim Graham

Component: UncategorizedTemplate system
Resolution: duplicate
Status: newclosed

This is issue #27584. I didn't backport it to 1.9 or 1.10 since the patch wasn't trivial and it went unreported for so long after the release of Django 1.9.

comment:2 in reply to:  1 Changed 13 months ago by Ling-Xiao Yang

Cc: ling-xiao.yang@… added
Resolution: duplicate
Status: closednew
Version: 1.10master

Replying to Tim Graham:

This is issue #27584. I didn't backport it to 1.9 or 1.10 since the patch wasn't trivial and it went unreported for so long after the release of Django 1.9.

Hello Tim,

Thank you for pointing to #27584, but it seems that there's still a problem with {% extend %} tag, although it fixed the {% include %} tag. In its PR, there's only a unit test for {% include %} but not for {% extend %}.

Here's the test for your reference:

27956_child.html:

{% extends "27956_parent.html" %}

{% block content %}{% endblock %}

27956_parent.html:

{% load tag_27584 %}

{% badtag %}{% endbadtag %}

{% block content %}{% endblock %}

The unit test:

    def test_compile_tag_error_27956(self):
        engine = Engine(
            app_dirs=True,
            debug=True,
            libraries={'tag_27584': 'template_tests.templatetags.tag_27584'},
        )
        t = engine.get_template('27956_child.html')
        with self.assertRaises(TemplateSyntaxError) as e:
            t.render(Context())
        self.assertEqual(e.exception.template_debug['during'], '{% badtag %}')

{% badtag %} is the one added in #27584's PR that raises a TemplateSyntaxError during its rendering.

And this test doesn't pass on current master branch (7edeeb7):

======================================================================
FAIL: test_compile_tag_error_27956 (template_tests.tests.TemplateTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3.5/unittest/case.py", line 58, in testPartExecutor
    yield
  File "/usr/lib/python3.5/unittest/case.py", line 600, in run
    testMethod()
  File "/home/lxyang/django/tests/template_tests/tests.py", line 134, in test_compile_tag_error_27956
    self.assertEqual(e.exception.template_debug['during'], '{% badtag %}')
  File "/usr/lib/python3.5/unittest/case.py", line 820, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/usr/lib/python3.5/unittest/case.py", line 813, in _baseAssertEqual
    raise self.failureException(msg)
AssertionError: 'nt.html" %}\n' != '{% badtag %}'

----------------------------------------------------------------------

If I'm correct at my side, do we consider this still not fixed?

comment:3 Changed 13 months ago by Tim Graham

Summary: In DEBUG interface, the section "error during template rendering" doesn't go inside an included templateTemplate error raised in an {% extends %} parent template shows incorrect source location on debug page
Triage Stage: UnreviewedAccepted

Thanks for clarifying that. I can confirm that's a similar regression as what was fixed in #27584.

comment:4 Changed 13 months ago by Preston Timmons

This is happening because the ExtendsNode calls Template._render rather than Template.render:

https://github.com/django/django/blob/master/django/template/loader_tags.py#L152

That means context.render_context.template still references the original template when the debug information is collected:

https://github.com/django/django/blob/3dcc3516914f25b58bde9312831f1d94b05cdb53/django/template/base.py#L914

comment:5 Changed 13 months ago by Ling-Xiao Yang

Description: modified (diff)

Hello there,

Do we have a reason to have the both _render() and render() methods? In django.template, _render() is not used anywhere else.

Code reference:

~/django$ grep -r "\._render(" . -A 2 -B 2
./django/forms/widgets.py-        """Render the widget as an HTML string."""
./django/forms/widgets.py-        context = self.get_context(name, value, attrs)
./django/forms/widgets.py:        return self._render(self.template_name, context, renderer)
./django/forms/widgets.py-
./django/forms/widgets.py-    def _render(self, template_name, context, renderer=None):
--
./django/forms/boundfield.py-    def tag(self, wrap_label=False):
./django/forms/boundfield.py-        context = {'widget': self.data, 'wrap_label': wrap_label}
./django/forms/boundfield.py:        return self.parent_widget._render(self.template_name, context, self.renderer)
./django/forms/boundfield.py-
./django/forms/boundfield.py-    @property
--
./django/template/loader_tags.py-        # Call Template._render explicitly so the parser context stays
./django/template/loader_tags.py-        # the same.
./django/template/loader_tags.py:        return compiled_parent._render(context)
./django/template/loader_tags.py-
./django/template/loader_tags.py-
--
./django/template/base.py-                with context.bind_template(self):
./django/template/base.py-                    context.template_name = self.name
./django/template/base.py:                    return self._render(context)
./django/template/base.py-            else:
./django/template/base.py:                return self._render(context)
./django/template/base.py-
./django/template/base.py-    def compile_nodelist(self):

comment:6 Changed 13 months ago by Preston Timmons

I'm pretty sure calling render instead of _render for the extends tag would introduce errors. There's no reason the ExtendsNode can't be updated to set the correct template on context.render_context.template, though.

comment:7 Changed 13 months ago by Tim Graham

Has patch: set

comment:8 Changed 13 months ago by GitHub <noreply@…>

Resolution: fixed
Status: newclosed

In e643ba8b:

Fixed #27956 -- Fixed display of errors in an {% extends %} child.

Thanks Ling-Xiao Yang for the report and test, and
Preston Timmons for the fix.

comment:9 Changed 13 months ago by Tim Graham <timograham@…>

In 8cc2f5ae:

[1.11.x] Fixed #27956 -- Fixed display of errors in an {% extends %} child.

Thanks Ling-Xiao Yang for the report and test, and
Preston Timmons for the fix.

Backport of e643ba8bcf0b1da517cbab689ac157ee031202a3 from master

Note: See TracTickets for help on using tickets.
Back to Top