From e62165b898785e890661953c3b2c9c36d98aee57 Mon Sep 17 00:00:00 2001 From: Tim Graham Date: Sat, 2 Sep 2017 20:24:01 -0400 Subject: [PATCH] Refs #27175 -- Removed exception silencing from the {% include %} template tag. Per deprecation timeline. --- django/template/loader_tags.py | 66 ++++++------------- docs/ref/templates/builtins.txt | 15 ----- docs/releases/2.1.txt | 3 + docs/topics/logging.txt | 5 -- .../syntax_tests/test_include.py | 61 ++++------------- tests/template_tests/test_logging.py | 48 +------------- 6 files changed, 37 insertions(+), 161 deletions(-) diff --git a/django/template/loader_tags.py b/django/template/loader_tags.py index 6cc8dfc8e3..0393239a75 100644 --- a/django/template/loader_tags.py +++ b/django/template/loader_tags.py @@ -1,9 +1,6 @@ -import logging import posixpath -import warnings from collections import defaultdict -from django.utils.deprecation import RemovedInDjango21Warning from django.utils.safestring import mark_safe from .base import ( @@ -15,8 +12,6 @@ register = Library() BLOCK_CONTEXT_KEY = 'block_context' -logger = logging.getLogger('django.template') - class BlockContext: def __init__(self): @@ -170,46 +165,27 @@ class IncludeNode(Node): in render_context to avoid reparsing and loading when used in a for loop. """ - try: - template = self.template.resolve(context) - # Does this quack like a Template? - if not callable(getattr(template, 'render', None)): - # If not, we'll try our cache, and get_template() - template_name = template - cache = context.render_context.dicts[0].setdefault(self, {}) - template = cache.get(template_name) - if template is None: - template = context.template.engine.get_template(template_name) - cache[template_name] = template - # Use the base.Template of a backends.django.Template. - elif hasattr(template, 'template'): - template = template.template - values = { - name: var.resolve(context) - for name, var in self.extra_context.items() - } - if self.isolated_context: - return template.render(context.new(values)) - with context.push(**values): - return template.render(context) - except Exception as e: - if context.template.engine.debug: - raise - template_name = getattr(context, 'template_name', None) or 'unknown' - warnings.warn( - "Rendering {%% include '%s' %%} raised %s. In Django 2.1, " - "this exception will be raised rather than silenced and " - "rendered as an empty string." % - (template_name, e.__class__.__name__), - RemovedInDjango21Warning, - ) - logger.warning( - "Exception raised while rendering {%% include %%} for " - "template '%s'. Empty string rendered instead.", - template_name, - exc_info=True, - ) - return '' + template = self.template.resolve(context) + # Does this quack like a Template? + if not callable(getattr(template, 'render', None)): + # If not, try the cache and get_template(). + template_name = template + cache = context.render_context.dicts[0].setdefault(self, {}) + template = cache.get(template_name) + if template is None: + template = context.template.engine.get_template(template_name) + cache[template_name] = template + # Use the base.Template of a backends.django.Template. + elif hasattr(template, 'template'): + template = template.template + values = { + name: var.resolve(context) + for name, var in self.extra_context.items() + } + if self.isolated_context: + return template.render(context.new(values)) + with context.push(**values): + return template.render(context) @register.tag('block') diff --git a/docs/ref/templates/builtins.txt b/docs/ref/templates/builtins.txt index 49e07aeec2..e27551b9b8 100644 --- a/docs/ref/templates/builtins.txt +++ b/docs/ref/templates/builtins.txt @@ -711,21 +711,6 @@ available to the included template:: {% include "name_snippet.html" with greeting="Hi" only %} -If the included template causes an exception while it's rendered (including -if it's missing or has syntax errors), the behavior varies depending on the -:class:`template engine's ` ``debug`` option (if not -set, this option defaults to the value of :setting:`DEBUG`). When debug mode is -turned on, an exception like :exc:`~django.template.TemplateDoesNotExist` or -:exc:`~django.template.TemplateSyntaxError` will be raised. When debug mode -is turned off, ``{% include %}`` logs a warning to the ``django.template`` -logger with the exception that happens while rendering the included template -and returns an empty string. - -.. deprecated:: 1.11 - - Silencing exceptions raised while rendering the ``{% include %}`` template - tag is deprecated. In Django 2.1, the exception will be raised. - .. note:: The :ttag:`include` tag should be considered as an implementation of "render this subtemplate and include the HTML", not as "parse this diff --git a/docs/releases/2.1.txt b/docs/releases/2.1.txt index 6ab9303ccb..7f0b6e77e8 100644 --- a/docs/releases/2.1.txt +++ b/docs/releases/2.1.txt @@ -241,3 +241,6 @@ how to remove usage of these features. passing ``pylibmc`` behavior settings as top-level attributes of ``OPTIONS``. * The ``host`` parameter of ``django.utils.http.is_safe_url()`` is removed. + +* Silencing of exceptions raised while rendering the ``{% include %}`` template + tag is removed. diff --git a/docs/topics/logging.txt b/docs/topics/logging.txt index 800be95885..3202ee9477 100644 --- a/docs/topics/logging.txt +++ b/docs/topics/logging.txt @@ -501,11 +501,6 @@ Log messages related to the rendering of templates. * Missing context variables are logged as ``DEBUG`` messages. -* Uncaught exceptions raised during the rendering of an - :ttag:`{% include %} ` are logged as ``WARNING`` messages when - debug mode is off (helpful since ``{% include %}`` silences the exception and - returns an empty string in that case). - .. _django-db-logger: ``django.db.backends`` diff --git a/tests/template_tests/syntax_tests/test_include.py b/tests/template_tests/syntax_tests/test_include.py index 760a801377..ca98e09771 100644 --- a/tests/template_tests/syntax_tests/test_include.py +++ b/tests/template_tests/syntax_tests/test_include.py @@ -1,10 +1,7 @@ -import warnings - from django.template import ( Context, Engine, TemplateDoesNotExist, TemplateSyntaxError, loader, ) -from django.test import SimpleTestCase, ignore_warnings -from django.utils.deprecation import RemovedInDjango21Warning +from django.test import SimpleTestCase from ..utils import setup from .test_basic import basic_templates @@ -39,24 +36,8 @@ class IncludeTagTests(SimpleTestCase): @setup({'include04': 'a{% include "nonexistent" %}b'}) def test_include04(self): template = self.engine.get_template('include04') - - if self.engine.debug: - with self.assertRaises(TemplateDoesNotExist): - template.render(Context({})) - else: - with warnings.catch_warnings(record=True) as warns: - warnings.simplefilter('always') - output = template.render(Context({})) - - self.assertEqual(output, "ab") - - self.assertEqual(len(warns), 1) - self.assertEqual( - str(warns[0].message), - "Rendering {% include 'include04' %} raised " - "TemplateDoesNotExist. In Django 2.1, this exception will be " - "raised rather than silenced and rendered as an empty string.", - ) + with self.assertRaises(TemplateDoesNotExist): + template.render(Context({})) @setup({ 'include 05': 'template with a space', @@ -178,48 +159,28 @@ class IncludeTagTests(SimpleTestCase): @setup({'include-error07': '{% include "include-fail1" %}'}, include_fail_templates) def test_include_error07(self): template = self.engine.get_template('include-error07') - - if self.engine.debug: - with self.assertRaises(RuntimeError): - template.render(Context()) - else: - with ignore_warnings(category=RemovedInDjango21Warning): - self.assertEqual(template.render(Context()), '') + with self.assertRaises(RuntimeError): + template.render(Context()) @setup({'include-error08': '{% include "include-fail2" %}'}, include_fail_templates) def test_include_error08(self): template = self.engine.get_template('include-error08') - - if self.engine.debug: - with self.assertRaises(TemplateSyntaxError): - template.render(Context()) - else: - with ignore_warnings(category=RemovedInDjango21Warning): - self.assertEqual(template.render(Context()), '') + with self.assertRaises(TemplateSyntaxError): + template.render(Context()) @setup({'include-error09': '{% include failed_include %}'}, include_fail_templates) def test_include_error09(self): context = Context({'failed_include': 'include-fail1'}) template = self.engine.get_template('include-error09') - - if self.engine.debug: - with self.assertRaises(RuntimeError): - template.render(context) - else: - with ignore_warnings(category=RemovedInDjango21Warning): - self.assertEqual(template.render(context), '') + with self.assertRaises(RuntimeError): + template.render(context) @setup({'include-error10': '{% include failed_include %}'}, include_fail_templates) def test_include_error10(self): context = Context({'failed_include': 'include-fail2'}) template = self.engine.get_template('include-error10') - - if self.engine.debug: - with self.assertRaises(TemplateSyntaxError): - template.render(context) - else: - with ignore_warnings(category=RemovedInDjango21Warning): - self.assertEqual(template.render(context), '') + with self.assertRaises(TemplateSyntaxError): + template.render(context) class IncludeTests(SimpleTestCase): diff --git a/tests/template_tests/test_logging.py b/tests/template_tests/test_logging.py index 89db04fafd..db4c9454be 100644 --- a/tests/template_tests/test_logging.py +++ b/tests/template_tests/test_logging.py @@ -1,8 +1,7 @@ import logging -from django.template import Context, Engine, Variable, VariableDoesNotExist -from django.test import SimpleTestCase, ignore_warnings -from django.utils.deprecation import RemovedInDjango21Warning +from django.template import Engine, Variable, VariableDoesNotExist +from django.test import SimpleTestCase class TestHandler(logging.Handler): @@ -81,46 +80,3 @@ class VariableResolveLoggingTests(BaseTemplateLoggingTestCase): def test_no_log_when_variable_exists(self): Variable('article.section').resolve({'article': {'section': 'News'}}) self.assertIsNone(self.test_handler.log_record) - - -class IncludeNodeLoggingTests(BaseTemplateLoggingTestCase): - loglevel = logging.WARN - - @classmethod - def setUpClass(cls): - super().setUpClass() - cls.engine = Engine(loaders=[ - ('django.template.loaders.locmem.Loader', { - 'child': '{{ raises_exception }}', - }), - ], debug=False) - - def error_method(): - raise IndexError("some generic exception") - - cls.ctx = Context({'raises_exception': error_method}) - - def test_logs_exceptions_during_rendering_with_debug_disabled(self): - template = self.engine.from_string('{% include "child" %}') - template.name = 'template_name' - with ignore_warnings(category=RemovedInDjango21Warning): - self.assertEqual(template.render(self.ctx), '') - self.assertEqual( - self.test_handler.log_record.getMessage(), - "Exception raised while rendering {% include %} for template " - "'template_name'. Empty string rendered instead." - ) - self.assertIsNotNone(self.test_handler.log_record.exc_info) - self.assertEqual(self.test_handler.log_record.levelno, logging.WARN) - - def test_logs_exceptions_during_rendering_with_no_template_name(self): - template = self.engine.from_string('{% include "child" %}') - with ignore_warnings(category=RemovedInDjango21Warning): - self.assertEqual(template.render(self.ctx), '') - self.assertEqual( - self.test_handler.log_record.getMessage(), - "Exception raised while rendering {% include %} for template " - "'unknown'. Empty string rendered instead." - ) - self.assertIsNotNone(self.test_handler.log_record.exc_info) - self.assertEqual(self.test_handler.log_record.levelno, logging.WARN)