#12787 closed (fixed)
TemplateDoesNotExist exception does not report the correct template_name
Reported by: | trigeek38 | Owned by: | anonymous |
---|---|---|---|
Component: | Template system | Version: | dev |
Severity: | Keywords: | TemplateDoesNotExist | |
Cc: | Lior Sion | Triage Stage: | Unreviewed |
Has patch: | yes | Needs documentation: | no |
Needs tests: | yes | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
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:
Attachments (4)
Change History (18)
comment:1 by , 15 years ago
Description: | modified (diff) |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 15 years ago
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.
comment:3 by , 15 years ago
Owner: | changed from | to
---|
comment:4 by , 15 years ago
Has patch: | set |
---|---|
Needs tests: | set |
Owner: | changed from | to
Status: | new → assigned |
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)
comment:5 by , 15 years ago
Needs documentation: | set |
---|---|
Needs tests: | unset |
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.
by , 15 years ago
Attachment: | 12787.diff added |
---|
comment:6 by , 15 years ago
Needs documentation: | unset |
---|---|
Needs tests: | set |
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.
by , 15 years ago
Attachment: | 12787.2.diff added |
---|
comment:7 by , 15 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:8 by , 15 years ago
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....
comment:9 by , 14 years ago
Patch needs improvement: | set |
---|---|
Resolution: | fixed |
Status: | closed → reopened |
Triage Stage: | Accepted → Unreviewed |
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.
comment:10 by , 14 years ago
Cc: | added |
---|
follow-up: 12 comment:11 by , 14 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
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.
comment:12 by , 14 years ago
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" %}
comment:13 by , 14 years ago
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.
(Reformatted description. Please use preview.)