Opened 4 years ago

Closed 4 years ago

#31478 closed Bug (duplicate)

Template.get_exception_info() relies on internal state which isn't correct for sub-templates.

Reported by: Keryn Knight Owned by: nobody
Component: Template system Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Currently, the method is defined as:

def get_exception_info(self, exception, token):

and internal to it's scope, makes use of the values self.source and self.origin.name

These 2 variables are not necessarily the data on which the operations should apply, at least in some scenarios. I have a demo project while I'll attach to hopefully demonstrate this.

The problem exists on 2.2, 3.0 & master (being 3.1 currently). Whilst I've not done an exhaustive sweep of the potential problems, a solution which seems to work in my limited test is thus:

Method signature becomes

def get_exception_info(self, exception, token, origin=None, source=None):
        if source is None:
            source = self.source
        if origin is None:
            origin = self.origin

Why support Nones? Because at the very least I think django-debug-toolbar makes use of the method, and it leaves it backwards compatible.

The callers I'm aware of are django.template.base.Template.compile_nodelist, which would become:

e.template_debug = self.get_exception_info(e, e.token, self.origin, self.source)

though I think that given that is passing in the *same* data, it could actually remain unchanged, because the internal state is the same both ways.

... and django.template.base.Node.render_annotated:

e.template_debug = context.render_context.template.get_exception_info(e, self.token, context.template.origin, context.template.source)

This one is what I presume is the crux of the problem, because the render context's template is not the same as the child template.

Of course, changing the origin and source might have other knock on effects that I've not encountered in my tests, and I haven't tried the proposed changes against the test suite etc.

Change History (3)

comment:1 by Keryn Knight, 4 years ago

Example project is here: https://github.com/kezabelle/django-ticket-31478

The 2 dependencies are Django and the app with which I encountered the problem, which is itself a terrible set of monkeypatches upon the Django Template Language. There's probably cleaner ways to demonstrate the issue, but this is the quickest one I obviously have at hand. As far as I know it's not specifically the monkeypatch at fault, but I could be wrong.

Steps to hopefully reproduce:

  • clone repo
  • install requirements in root of repo (pip install -r requirements.txt)
  • boot runserver.
  • navigate your browser to /fine with DEBUG turned on, so the technical 500 renders. You should hopefully observe that the template's HTML is shown, and the error is highlighted correctly as {{ force_error }}
  • navigate to /problem to see the error being described in the ticket proper. You should hopefully observe that the HTML shown is that of the base.html template rather than the subtemplate.html which extends base.html - as a result, the HTML is wrong, there is no line to highlight, and it reports the error at line 0.

With the proposed possible changes, the error is highlighted correctly on /problem, the line is reported as line 4 correctly, and the path to the template shown is the correct subtemplate rather than the base one.

comment:2 by Tim Graham, 4 years ago

Duplicate of #28935?

comment:3 by Mariusz Felisiak, 4 years ago

Resolution: duplicate
Status: newclosed
Summary: Template.get_exception_info relies on internal state which isn't correct for sub-templates.Template.get_exception_info() relies on internal state which isn't correct for sub-templates.

Thanks Keryn, I think we can mark this as a duplicate of #28935 (as Tim suggested). It's a regression in e643ba8bcf0b1da517cbab689ac157ee031202a3 and your patch restores the previous behavior. Unfortunately it also causes two failures:

======================================================================
FAIL: test_compile_tag_error_27584 (template_tests.tests.TemplateTests)
----------------------------------------------------------------------
...
AssertionError: '' != '{% badtag %}'
+ {% badtag %}
======================================================================
FAIL: test_compile_tag_error_27956 (template_tests.tests.TemplateTests)
Errors in a child of {% extends %} are displayed correctly.
----------------------------------------------------------------------
...
AssertionError: 'nt.html" %}\n' != '{% badtag %}'
Note: See TracTickets for help on using tickets.
Back to Top