django.template.loader.select_template should not silently skip a template which includes another template that does not exist

[Michael P. Jung]

When using django.template.loader.select_template for a template that does exist, but contains an {% include ... %} tag which throws a TemplateDoesNotExist exception, the template is silently skipped instead of raising an error. That makes it extremely hard to track down an include error within a template that is loaded using select_template.

I haven't dug very deep into this issue, but as far as I got it seams that the most simple solution to this problem would be a different exception to be thrown by the include tag.

[James Bennett]

I'm punting this to post-1.0; it's a minor issue, an instance of a larger "problem" that's not specific to Django, and there are much more important things to do for the 1.0 release.

[Mike Amy]

Ran into this one...

Problem is bad exception handling... TemplateDoesNotExist exceptions arising from the missing child templates are ignored in select_template, in effect causing it to ignore the parent template. A fix is to check the message in the exception (in django/template/loader.py ):

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, e:
            if e.message is template_name: # <<< Check the template is not a child template
                continue
            raise
    # If we get here, none of the templates could be loaded
    raise TemplateDoesNotExist, template_name_list

However, that is making assumptions about the exception, there ought to be a better way, like a template_name attribute in the exception

[(none)]

Milestone post-1.0 deleted

[Antti Kaihola]

patch: included templates throw a different exception

[Antti Kaihola]

The idea of the patch above is that all TemplateDoesNotExist exceptions encountered while compiling a template get changed to IncludedTemplateDoesNotExist exceptions, which are then checked for in select_template().

This is kind of what Michael P. Jung suggested, except it's handled at the loader level instead of in the {% include %} tag.
Therefore, I think this will catch sub-templates loaded with whatever mechanism, not only with {% include %}.

Also, in case of a missing included template, the exception displayed is:

IncludedTemplateDoesNotExist: template_path/template_name.html

Does this make sense, or would Make Amy's patch or his suggestion for a template_name exception attribute be a better approach? Some kind of a fix would be nice since the current behavior can be really confusing.

[Rob Hudson]

Updated to work with 1.3alpha1

[Rob Hudson]

Updated to work with 1.3alpha1. Now with passing tests.

[Remco Wendt]

Closing this as invalid, the current behavior is well tested and the expected behavior. If you want template tags to raise errors then one should set TEMPLATE_DEBUG to True (even though DEBUG is False) this will correctly lead to a 500 error. Otherwise template errors fail silently. See table below for what happens in each situation.

[Joeri Bekker]

DEBUG	TEMPLATE_DEBUG	Comments
True	True	Django raises a TemplateDoesNotExist in the debug screen.
True	False	Django silently ignores the missing template.
False	True	Django raises a TemplateDoesNotExist resulting in a simple HTTP 500 error.
False	False	Django silently ignores the missing template.

I concur with shanx that this is the expected behaviour.

[Michael P. Jung <michael.jung@…>]

I have to disagree. This behavior is neither expected nor well documented.

It is about an existing template that includes another template which does not exist.

If the template does exist but raises a TemplateDoesNotExist due to {% include "does_not_exist.html" %} the error should be raised and not silently ignored. A template which includes a missing template is definitely an error in the code and nothing I would call "expected behavior".

Even if it was well documented it would be highly counter intuitive. I doubt that anybody uses broken templates on purpose and expects them to be silently skipped by select_template.

[Florian Apolloner]

I agree with Michael here, if select_template finds a template it has to use it, if it can't use it due to errors it should not be allowed to continue silently!

[FunkyBob]

It looks like this could be related to #3544.

Oddly enough, the fix refers to [optionally] allowing {% include %} to "quietly" fail if the template doesn't exist.

However, as it no longer tries to load the included template at parse time, it won't raise an error then.

[Unai Zalakain]

I would rather add a template_name attribute to the TemplateDoesNotExist class. This attribute could be used not only in this case but also in normal TemplateDoesNotExist exception.

[Unai Zalakain]

PR with the IncludedTemplateDoesNotExist exception, passing tests and documentation: ​https://github.com/django/django/pull/1795

[jonathanslenders]

Can't we inherit IncludedTemplateDoesNotExist from TemplateDoesNotExist? That way, we don't have to change existing unit tests.

[Unai Zalakain]

Good point, I would inherit from TemplateDoesNotExist but I still would check for IncludedTemplateDoesNotExist in tests, checking for specific exception is better than checking for a broader one.

[jonathanslenders]

Sure.

[Unai Zalakain]

PR updated!

[jonathanslenders]

Sorry, one more remark. You should actually do the same in ExtendsNode.render. There we have exactly the same issue. (There are also tests for "extends nonexistent" which needs to be updated.)

Maybe you should rename the exception to something like ReferencedTemplateDoesNotExist.

[Unai Zalakain]

I'm not sure about what to do. The problem that this ticket was addressing is that no exception is raised when a included template does not exist. In this case, the solution has been to create a new exception so that this one isn't captured.

The way things are now, a TemplateDoesNotExist exception is raised if an extended template doesn't exist. Though it could make sense to be more explicit about this and raise a ReferencedTemplateDoesNotExist, if we use that instead of the IncludedTemplateDoesNotExist for included templates too, we aren't being less explicit.

A solution could be to create two new exceptions: ExtendedTemplateDoesNotExist and IncludedTemplateDoesNotExist. Maybe it's a bit of overkill, I don't know.

[jonathanslenders]

Maybe I'm wrong, but I don't think it's really overkill in this case. Maybe even all of them:

class ReferencedTemplateDoesNotExist(TemplateDoesNotExist):
    pass

class ExtendedTemplateDoesNotExist(ReferencedTemplateDoesNotExist):
    pass

class IncludedTemplateDoesNotExist(ReferencedTemplateDoesNotExist):
    pass

[Unai Zalakain]

It could be, but I think the best thing to do is to open an other ticket about being more explicit with the exception raised if an extended template doesn't exist and leave these ticket for the include tag thing. Because, after all, they're two different discussions. Any thoughts? Will you open the ticket or do I?

[jonathanslenders]

Okay, can you open a new ticket?
I'll put this in ready for checkin.

[Unai Zalakain]

Done! #21393

[Anssi Kääriäinen]

How is the select_template() issue resolved by the patch? The select_template() method still catches TemplateDoesNotExist, and thus IncludedTemplateDoesNotExist. Am I missing something?

I don't think the patch actually solves this ticket's issue. It does look good to me otherwise.

[Unai Zalakain]

Uh, you are quite right, I don't know how I worked on it so blindly!!

The thing is that select_template doesn't even render the templates so there isn't any chance of handling the inexistence of included templates. The included template is rendered when the master template is rendered and there, there isn't any issue with TemplateDoesNotExist being ignored. I think this issue could be from a time when instantiating a Template would automatically render it (dunno).

I would like somebody else to confirm it but I would close this issue as invalid.

Thanks for making sense of all this and stopping it before it was merged!

[chrismedrela]

I confirm -- IncludeNode doesn't raise TemplateDoesNotExist in its __init__ since ​https://github.com/django/django/commit/e2f06226ea4a38377cdb69f2f5768e4e00c2d88e.