TemplateDoesNotExist exception does not report the correct template_name

[trigeek38]

When calling templateA which {% includes templateB %} that does not exist, the exception reports that templateA does not exist instead of templateB. Version (1, 1, 0, 'final', 0) reported the correct template_name.

### This is how 1.1.0 reported templates that did not exist when called by {% include %} in another template.

_order_detail.html included in order_detail.html located at /orders/650/

TemplateDoesNotExist at /orders/650/

_order_detail.html

Request Method: 	GET
Request URL:  somewhere/outside/orders/650/
Exception Type: 	TemplateDoesNotExist
Exception Value: 	

_order_detail.html

Exception Location: 	/usr/lib/python2.4/site-packages/django/template/loader.py in find_template_source, line 74
Python Executable: 	/usr/bin/python
Python Version: 	2.4.3

## This is how trunk reports the same situation. Notice here that it reports that the actual template being called by the view as not existing instead of the included template. Here I included _order_detail.html (does not exist) in po_form.html

pos/po_form.html

Request Method: 	GET
Request URL: 	somewhere/else/pos/new/
Exception Type: 	TemplateDoesNotExist
Exception Value: 	

pos/po_form.html

Exception Location: 	/home/trigeek38/lib/python2.5/django/template/loader.py in find_template, line 125
Python Executable: 	/usr/local/bin/python
Python Version: 	2.5.4
Python Path: 	['/home/trigeek38/lib/python2.5/html5lib-0.11.1-py2.5.egg', '/home/trigeek38/lib/python2.5/pisa-3.0.30-py2.5.egg', '/home/trigeek38/lib/python2.5/django_pagination-1.0.5-py2.5.egg', '/home/trigeek38/lib/python2.5', '/home/trigeek38/webapps/django_trunk/lib/python2.5', '/usr/local/lib/python25.zip', '/usr/local/lib/python2.5', '/usr/local/lib/python2.5/plat-linux2', '/usr/local/lib/python2.5/lib-tk', '/usr/local/lib/python2.5/lib-dynload', '/usr/local/lib/python2.5/site-packages', '/usr/local/lib/python2.5/site-packages/PIL', '/home/trigeek38/webapps/django_trunk/projects/', '/home/trigeek38/webapps/django_trunk/projects/']
Server time: 	Thu, 4 Feb 2010 21:43:40 -0500
Template-loader postmortem

Django tried loading these templates, in this order:

    * Using loader django.template.loaders.filesystem.Loader:
    * Using loader django.template.loaders.app_directories.Loader: 

[Karen Tracey]

(Reformatted description. Please use preview.)

[anonymous]

Applying bisection shows this is a regression introduced in r11862 (cached templates feature and template loaders API refactoring). There is no need to use django.template.loaders.cached.Loader to reproduce this bug.

[jkatzer]

this patch fixes it, but my testing suite is currently broken otherwise on this machine. if someone else tests it should work, if not then i will rerun tomorrow after fixing my testing suite.

the problem is that code after version 1.1 throws array the real exception and creates it own based on the templatename being used. this patch is slightly uglier, but its simple and it works.

and its my first real python code!!!
(even though its only one line)

[jkatzer]

patch file to fix the bug

[jkatzer]

scrapped the old patch....

the new patch(es) overload the previous TemplateDoesNotExist execption, to add wether or not that error should be thrown out. it is worded as to wether or not a super function calling it "should_keep_trying"

in regards to this, any place that previously raised an TemplateDoesNotExist error needs to be rewritten to have an extra arg... this also is the case in the testing file, which has been modified accordingly...

please excuse my code as i am not fully familiar with python and the coding conventions used by the django community.

[jkatzer]

template patches (complete)

[Karen Tracey]

jkatzer: In the future please attach a single diff file created by svn diff from the root of the tree. See: ​http://docs.djangoproject.com/en/dev/internals/contributing/#patch-style. That's much easier to deal with than a bunch of individual .diff files zipped up.

I'm uncomfortable with the originally proposed way of fixing this, so I've attached an alternative approach. The root cause of this bug is similar to #12992. During load_template, the load of the requested template may succeed but then get_template_from_string may raise TemplateDoesNotExist if the loaded template can't be compiled due to some other template not existing. That TemplateDoesNotExist, raised by load_template, is interpreted by its caller to mean that the requested template doesn't exist, leading to an erroneous report of the actual problem on the debug page.

Attached patch changes the loader to code to fall back to returning the template source and name, as it used to, if it was able to find the requested template but attempting to compile it raises a TemplateSyntaxError. Thus the loader will only raise TemplateSyntaxError if the specifically-requested template does not exist. That prevents this code:

    for loader in template_source_loaders:
        try:
            source, display_name = loader(name, dirs)
            return (source, make_origin(display_name, loader, name, dirs))
        except TemplateDoesNotExist:
            pass
    raise TemplateDoesNotExist(name)

from moving on and trying other loaders, then ultimately ending with TemplateDoesNotExist(name), which mis-identifies the template that does not exist.

That change alone is not sufficient to fix the problem, though. The extends node get_parent method catches TemplateDoesNotExist and turns it into a TemplateSyntaxError stating that the template specified to be extended does not exist. Prior to r11862, the call covered by the try/except was a find_template_source call, which would only raise TemplateDoesNotExist if that specific template does not exist. In r11862 the call inside the try/except was changed to get_template, which both finds the source and compiles it, and so may raise TemplateDoesNotExist if some other template needed to compile the extended template does not exist. We could either put the code here back the way it was before r11862 or remove this try/except entirely. The attached patch does the latter, because I don't really see what additional value is added by the different message ("... can't be extended because it doesn't exist") over a plain template does not exist message.

There is a test in the patch, but it's not really sufficient since it does not test both the base loader case and the cached loader case. But it does illustrate when the problem arises.

[Karen Tracey]

(In [12792]) Fixed #12787: Correctly identify the template that does not exist when a template being extended includes another template that does not exist. Thanks to trigeek38 for the report.

[jkatzer]

thanks for the advice... i was at a code sprint, and asked around for the best way to fix it and thats what we came up with....

[Lior Sion]

This fix doesn't work - the original problem remains.

The original exception (found in loader.py, also in changeset:12792):

raise TemplateDoesNotExist(name) 

that's the correct line of code, however, at the same file (and same changeset:12792):

except TemplateDoesNotExist:
            # If compiling the template we found raises TemplateDoesNotExist, back off to
            # returning the source and display name for the template we were asked to load.
            # This allows for correct identification (later) of the actual template that does
            # not exist.
            return source, display_name

the exception is caught but value is not used, and so we "loose" the exception. However, since we try to render it again, it's thrown again at the same place. This time, it's caught, but there's an error there:

the original call look like this:

def select_template(template_name_list):
    "Given a list of template names, returns the first that can be loaded."
    for template_name in template_name_list:
        try:
            return get_template(template_name)
        except TemplateDoesNotExist:
            continue
    # If we get here, none of the templates could be loaded
    raise TemplateDoesNotExist(', '.join(template_name_list))

and so the exception caught there is not because the original template was not found, but because templateB was not found, but it's never recorded and identified.

A possible solution:

def select_template(template_name_list):
    "Given a list of template names, returns the first that can be loaded."
    error_list = []
    for template_name in template_name_list:
        try:
            return get_template(template_name)
        except TemplateDoesNotExist, e:
            if e.message not in template_name_list:
                error_list.append(e.message)
            continue
    # If we get here, none of the templates could be loaded
    template_name_list += error_list
    raise TemplateDoesNotExist(', '.join(template_name_list))

we need to make sure the reason for the TemplateDoesNotExist exception is not "just" looking for TemplateA in a wrong directory (the external loop).

A different approach would be to create SubTemplateDoesNotExist exception type.

[Karen Tracey]

The problem identified in this ticket was fixed, really. The fix includes tests, which failed before the code change and are still passing on current trunk:

~/django/trunk/tests
--> ./runtests.py --settings=testdb.sqlite -v2 templates.Templates.test_extends_include_missing_baseloader templates.Templates.test_extends_include_missing_cachedloader
Importing application templates
Creating test database for alias 'default' (':memory:')...
Creating tables ...
Creating table django_content_type
Creating table auth_permission
Creating table auth_group_permissions
Creating table auth_group
Creating table auth_user_user_permissions
Creating table auth_user_groups
Creating table auth_user
Creating table auth_message
Creating table django_site
Creating table django_flatpage_sites
Creating table django_flatpage
Creating table django_redirect
Creating table django_session
Creating table django_comments
Creating table django_comment_flags
Creating table django_admin_log
Installing custom SQL ...
Installing indexes ...
No fixtures found.
Creating test database for alias 'other' ('other_db')...
Destroying old test database 'other'...
Creating tables ...
Creating table django_content_type
Creating table auth_permission
Creating table auth_group_permissions
Creating table auth_group
Creating table auth_user_user_permissions
Creating table auth_user_groups
Creating table auth_user
Creating table auth_message
Creating table django_site
Creating table django_flatpage_sites
Creating table django_flatpage
Creating table django_redirect
Creating table django_session
Creating table django_comments
Creating table django_comment_flags
Creating table django_admin_log
Installing custom SQL ...
Installing indexes ...
No fixtures found.
test_extends_include_missing_baseloader (regressiontests.templates.tests.Templates) ... ok
test_extends_include_missing_cachedloader (regressiontests.templates.tests.Templates) ... ok

----------------------------------------------------------------------
Ran 2 tests in 0.004s

OK
Destroying test database for alias 'default' (':memory:')...
Destroying test database for alias 'other' ('other_db')...

Further, if you doubt the tests, if I change one of my current projects to have the problem noted in the original description -- one existing template including a 2nd existing template that in turn attempts to include a 3rd template that does not exist, the debug page I get correctly identifies the template that is missing.

You may be seeing a bug, but it is not this exact bug. Better than trying to show how the code change made here is wrong would be a test case that shows the case you are running into where a problem exists. And that should go into its own new ticket (that perhaps references this one), because this specific one is fixed.

[Lior Sion]

The issue that was fixed here + the tests are for the wrong thing: an extended template that is missing an include, and not a normal template that is missing an include - the tests are also testing a different scenario.

The tested template has:

{% extends "broken_base.html" %}

while it should have:

{% include "just/something.html" %}

[Karen Tracey]

Re-reading now, I find the original description inconclusive as to which case it was reporting -- depends on what was meant by "call": render or extend. I don't remember if I tried both while initially looking at the ticket and choose to focus on the extends case because it was a superset of the other, or if I just assumed "call" meant extend. In some brief tests now with running against r12791/r12792 (right before and after the fix) if I switch things around to be just a plain include of a missing template, I see the wrong template reported before the fix and the correct one after the fix, so the case you mention here was also fixed by this code change. I do also believe that case is a subset of the extends case that is actually tested, so I don't believe either the tests or the fix here are "wrong".

Perhaps the problem you are encountering is the one identified in #15502, which involves a similar problem when a list of templates is provided. Sorry, I did not consider that case when looking at this ticket (there was no mention of lists of templates being tried), so that case was not fixed by this ticket.

[Jacob]

Milestone 1.2 deleted