Inline template rendering in admin shouldn't silence errors
George Karpenkov
1) Specify a custom admin class (say A) which mentions a custom inline
with a custom template, say "a.html"
2) Write anything to "a.html" which will raise TemplateSyntaxError -
ie "{% extends "a.html" %}
3) Observe the change_form for A. Note that you do not see any errors,
but instead the whole inline does not appear at all.
I find this behavior extremely confusing and annoying. Is it just a
bug or is there any reasoning behind that?
Russell Keith-Magee
The broad reasoning is that a partial page rendering is preferable to
a 500 error when rendering a template. This is driven by production
requirements -- the end user shouldn't ever see a 500 error.
Admittedly, this can make template debugging difficult at times. There
has been some discussion in the past about whether the TEMPLATE_DEBUG
mode should be used allow for more aggressive error reporting during
testing; however, this discussion hasn't really moved beyond the
"vague initial discussion" phase.
Yours,
Russ Magee %-)
George Karpenkov
I don't quite understand how an error in the admin template is
different from the error in the inline template -- why one gets
silenced and another one doesn't?
Well if I am the only person who got repeatedly hit by that
particular issue, then I guess I'll have to deal with that.
On Sep 1, 10:44 pm, Russell Keith-Magee <russ...@keith-magee.com>
wrote:
>
bur...@gmail.com
I believe this is a bug since any other errors in admin (not related
to inlines) don't pass silently.
Silencing errors should always be documented, especially if error is
silenced when DEBUG is turned on.
So it's either documentation or implementation bug.
By the way, does it show 500 error page if DEBUG is on?
--
Best regards, Yuri V. Baburov, ICQ# 99934676, Skype: yuri.baburov,
MSN: bu...@live.com
Russell Keith-Magee
> Hi George,
>
> I believe this is a bug since any other errors in admin (not related
> to inlines) don't pass silently.
>
> Silencing errors should always be documented, especially if error is
> silenced when DEBUG is turned on.
>
> So it's either documentation or implementation bug.
IMHO, it's a documentation issue, not a bug. It all comes down to how
you interpret the operation of the {% include %} tag.
In one interpretation, you can look at at the {% include %} as a major
structural node in the template, whose role is to substitute the
contents of a subtemplate. In this case, the {% include %} doesn't
actually exist in the final template; it's just a placeholder that
gets expanded as part of the original parsing process. In this
interpretation, any syntax error in the include node would be surfaced
as a syntax error in the main parent template doing the including.
Alternatively, you can look at {% include %} as being just like any
other tag. It exists as a normal template node, and the parsed parent
template contains a node that represents the {% include %}. At
runtime, the {% include %} tag substitutes it's content; and if that
raises an error, then the rendering process swallows that error.
The second interpretation what actually happens in implementation. {%
includes %} are actual nodes in the parsed template tree. The reason
for this is that the name of the template that is rendered is actually
a variable - For example, consider the following:
{% for templ in template_list %}
{% include templ %}
{% endfor %}
The included template isn't actually loaded and resolved until time of
render. As a result, we aren't in a position to raise a
TemplateSyntaxError when we parse the original document tree. We only
parse the included template when we render the parent template with a
specific context.
For the record, this distinction is also why {% cycle %} tags don't
persist over iterated {% include %}s -- each {% include %} is an
independent rendering context, so a cycle in one include doesn't
affect subsequent iterations.
So - this is a documentation issue. However, it's a pretty subtle
point, so it's not easy to explain. If anyone wants to take a swing at
trying to explain the issue, it would certainly be a valuable
contribution to the docs, and would possibly reduce the number of
invalid bug reports that are logged against the behavior of the
include tag.
That said: if anyone has any bright ideas on how to improve error
handling when rendering templates, we're open to suggestions. However,
my original point -- that runtime template errors are considered
unacceptable -- still stands, and any suggestion needs to honor that.
Yours,
Russ Magee %-)
bur...@gmail.com
I'd define
> {% for templ in template_list %}
> {% include templ %}
> {% endfor %}
as a special case, for which special command or pattern should exist.
Should it be
{% for templ in template_list %}
{% try-include template %}
{% endfor %}
or the opposite to be called
{% require template %} instead of include,
or maybe this whole pattern should be written as
{% include-first templ %}
But in most cases {% include %} is used as "require", so in my
opinion it should raise errors!
I'd also consider a require-once pattern to fix common widget chrome
problems (i.e. different parts of the page might include jquery in
headers).
> --
> You received this message because you are subscribed to the Google Groups "Django developers" group.
> To post to this group, send email to django-d...@googlegroups.com.
> To unsubscribe from this group, send email to django-develop...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Russell Keith-Magee
> Hi Russell,
>
> I'd define
>> {% for templ in template_list %}
>> {% include templ %}
>> {% endfor %}
> as a special case, for which special command or pattern should exist.
>
> Should it be
> {% for templ in template_list %}
> {% try-include template %}
> {% endfor %}
> or the opposite to be called
> {% require template %} instead of include,
> or maybe this whole pattern should be written as
> {% include-first templ %}
>
> But in most cases {% include %} is used as "require", so in my
> opinion it should raise errors!
>
> I'd also consider a require-once pattern to fix common widget chrome
> problems (i.e. different parts of the page might include jquery in
> headers).
Either I'm completely missing the point you're trying to make, or
you've completely missed the point I was making.
Template rendering is a two step process:
1. Parse the template
2. Render the template.
The point I was trying to make is that at step 1, we *can't* know the
name of the subtemplate used in an {% include %}. This isn't a matter
for negotiation or something we are in a position to design -- it's
simply the way that the tag is implemented.
I don't see how changing the name of the include tag to "require" or
"try-include" changes anything, or how an alternate tag with those
names would behave differently.
Yours
Russ Magee %-)
bur...@gmail.com
Sorry, we didn't understand each other,
You're talking about additional problems for templates with variable names.
However main point that George made was that he wanted template
rendering to break when including templates fails, no matter if that
was in the parse time or rendering time.
To address your "original point -- that runtime template errors are
considered unacceptable" I suggested that we have 2 different template
tags instead of include, one that will break, another one that will
not, because me and George wanted to have not the current version of
include tag but one that breaks on errors.
Implementation could look like the following: wrapper in the render()
part of include node, that uses some internal "current template"
variable. Another variable in the parser for this purpose.
P.S. This improvement is also related to my syntax coloring feature,
since I wanted to improve template errors display as well.
Russell Keith-Magee
> Russell,
>
> Sorry, we didn't understand each other,
>
> You're talking about additional problems for templates with variable names.
>
> However main point that George made was that he wanted template
> rendering to break when including templates fails, no matter if that
> was in the parse time or rendering time.
>
> To address your "original point -- that runtime template errors are
> considered unacceptable" I suggested that we have 2 different template
> tags instead of include, one that will break, another one that will
> not, because me and George wanted to have not the current version of
> include tag but one that breaks on errors.
First off, I'm not about to add a second template tag for including
subtemplates. One is more than sufficient.
Secondly, I still don't see how what you're describing is possible.
The name of the template to be included is not determined until the
template is *rendered*. This is a completely to when the template is
*parsed*, and it is the *parsing* process that generates
TemplateSyntaxErrors. If we can't work out which subtemplate is to be
used until rendering, we can't raise a syntax error when the parent
template is parsed.
> Implementation could look like the following: wrapper in the render()
> part of include node, that uses some internal "current template"
> variable. Another variable in the parser for this purpose.
At this point, you're going to need to show me a patch. I can't see
how what you're describing is possible without violating the stateless
nature of the parsed template tree and/or doing some sort of template
pre-rendering.
Yours,
Russ Magee %-)
George Karpenkov
I still don't quite get why "runtime template errors are
unacceptable". My understanding is that if user has DEBUG=True, and
TEMPLATE_DEBUG=True, then clearly (at least to me) the user does want
to see all of the errors. DEBUG flags should be off in the production
environment, right?
Alternatively, the simplest solution that comes to my mind is to have
a third flag CAN_I_REALLY_HAVE_TEMPLATE_DEBUG_PLZ_K_THX (I am not good
at making up naming conventions) and if that one is set to True then
do not silence _any_ errors at all during template rendering.
George
On Sep 2, 9:27 pm, Russell Keith-Magee <russ...@keith-magee.com>
wrote:
Russell Keith-Magee
<true.c...@gmail.com> wrote:
> Dear Russ,
>
> I still don't quite get why "runtime template errors are
> unacceptable". My understanding is that if user has DEBUG=True, and
> TEMPLATE_DEBUG=True, then clearly (at least to me) the user does want
> to see all of the errors. DEBUG flags should be off in the production
> environment, right?
It's not quite as simple as that, for reasons that should be clear if
you read the rest of my responses on this thread.
> Alternatively, the simplest solution that comes to my mind is to have
> a third flag CAN_I_REALLY_HAVE_TEMPLATE_DEBUG_PLZ_K_THX (I am not good
> at making up naming conventions) and if that one is set to True then
> do not silence _any_ errors at all during template rendering.
1. Added settings to control behavior like this is a very bad idea,
and not one that I'm going to add to core.
2. This wouldn't work anyway; again, read the rest of my comments on
this thread.
Yours
Russ Magee %-)
bur...@gmail.com
<rus...@keith-magee.com> wrote:
> On Thu, Sep 2, 2010 at 5:04 PM, bur...@gmail.com <bur...@gmail.com> wrote:
>> Russell,
>>
>> Sorry, we didn't understand each other,
>>
>> You're talking about additional problems for templates with variable names.
>>
>> However main point that George made was that he wanted template
>> rendering to break when including templates fails, no matter if that
>> was in the parse time or rendering time.
>>
>> To address your "original point -- that runtime template errors are
>> considered unacceptable" I suggested that we have 2 different template
>> tags instead of include, one that will break, another one that will
>> not, because me and George wanted to have not the current version of
>> include tag but one that breaks on errors.
>
> First off, I'm not about to add a second template tag for including
> subtemplates. One is more than sufficient.
>
> Secondly, I still don't see how what you're describing is possible.
> The name of the template to be included is not determined until the
> template is *rendered*. This is a completely to when the template is
> *parsed*, and it is the *parsing* process that generates
> TemplateSyntaxErrors. If we can't work out which subtemplate is to be
> used until rendering, we can't raise a syntax error when the parent
> template is parsed.
Sorry. One more try.
We don't need a error to be raised exactly on parsing stage, it's
enough if parse error is raised on rendering.
Do you mean we can't raise a syntax error when the included template
is *rendered*?
How to do this:
If error was raised on parsing:
Parser instruments parsed tree with a *raise error here* node to be
raised on rendering this node later.
If error was raised on rendering but you inserted current template
into parent one:
Included template is guarded with special node that saves the
included template name for debugging purposes and re-raises parse
error.
Russell Keith-Magee
You managed to peak my curiosity, so I took a quick look at the
implementation of {% include %}. I'm now completely confused as to the
problem you're having.
The current Django 1.2 implementation of the {% include %} tag *does*
raise a TemplateSyntaxError during rendering if the subtemplate has an
error -- but only if TEMPLATE_DEBUG=True. If you have a syntax error
in an included subtemplate, and you're in TEMPLATE_DEBUG mode, you
*should* be getting template syntax errors for an invalid included
template.
My comments so far have all been based on trying to explain the
behavior George originally described in terms of the broader
philosophy behind Django's template language, but I didn't actually
verify that problem as described. So - I went back to George's
original report and tried to reproduce the problem. I've added a
custom template to an inline ModelAdmin class. If I put a syntax error
in that file (I tried both an {%extends%} of a non-existing template,
and a missing {% endfor %} tag), I get a TemplateSyntaxError.
However, if I set TEMPLATE_DEBUG=False, I get the behavior George
described -- non-rendering of the inline, rather than the error.
So - what exactly is the problem here? Is this just a case of not
having DEBUG turned on?
Yours,
Russ Magee %-)