should nonexistent template filter arguments resolve to string_if_invalid rather than raise VariableDoesNotExist?

284 views
Skip to first unread message

Tim Graham

unread,
May 31, 2017, 11:03:24 AM5/31/17
to Django developers (Contributions to Django itself)
Should nonexistent template filter arguments raise an exception? Current behavior:

{{ value|filter:nonexistent_template_var}}

raises VariableDoesNotExist for nonexistent_template_var.

I guess the proposal would be to make nonexistent_tempatle_var resolve to string_if_invalid. As for me, I think the current behavior is less error prone.

Related tickets:

This was first closed incorrectly as "fixed" by Jacob (actually a different issue was fixed), then amended as wontfix by Karen with the note, "I'm fine with wontfixing that, though it does rather seem to go against "template errors don't raise exceptions" philosophy."

A follow up ticket requesting the same thing as #13167.

Jon Dufresne

unread,
May 31, 2017, 8:10:53 PM5/31/17
to django-d...@googlegroups.com
Just my opinion, but I think raising an exception is more helpful to developers and will result in fewer unnoticed bugs.

More generally, I know the template engine has a history of silently converting unknown variables to string_if_invalid but this is more harmful than helpful in my experience. It continues to be a source of hard to catch bugs on the projects I work on. So I'd prefer we avoid introducing more of this pattern if possible.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/02727bd0-5a67-4cf9-9c3f-b0a0a7ea0a3a%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Shai Berger

unread,
Jun 2, 2017, 2:37:35 AM6/2/17
to django-d...@googlegroups.com
I favor raising an exception in this case, because values in this context are
not coerced to strings (unlike the case in {{ }} references).

IMO, and in order to protect the writers of all 3rd-party filters, the only way
to solve this without raising an exception is to resolve the whole filter
expression to string_if_invalid, rather than just the missing var.

Shai
> > email to django-develop...@googlegroups.com.
> > To post to this group, send email to django-d...@googlegroups.com.
> > Visit this group at https://groups.google.com/group/django-developers.
> > To view this discussion on the web visit https://groups.google.com/d/
> > msgid/django-developers/02727bd0-5a67-4cf9-9c3f-
> > b0a0a7ea0a3a%40googlegroups.com
> > <https://groups.google.com/d/msgid/django-developers/02727bd0-5a67-4cf9-9
> > c3f-b0a0a7ea0a3a%40googlegroups.com?utm_medium=email&utm_source=footer> .

Vlastimil Zíma

unread,
Jun 2, 2017, 11:38:47 AM6/2/17
to Django developers (Contributions to Django itself)
Django now reports invalid variable names by 'VariableDoesNotExist' exception, which is logged. So it is possible to track down invalid variable names. The problem I see here is inconsistency in handling invalid variable names. If invalid variable is rendered or used in an 'if' tag, it is silently turned into a string_if_invalid, if it's as a filter argument, it suddenly raises error.

Shai: Values are sometimes turned into an empty string in case of UnicodeDecodeError, see https://github.com/django/django/blob/master/django/template/base.py#L994-L998

IMO invalid variable should behave the same way regardless of whether it's printed or used as an filter argument.

Vlastik

Dne pátek 2. června 2017 8:37:35 UTC+2 Shai Berger napsal(a):

Shai Berger

unread,
Jun 3, 2017, 10:30:51 AM6/3/17
to django-d...@googlegroups.com
On Friday 02 June 2017 18:38:47 Vlastimil Zíma wrote:
>
> Shai: Values are sometimes turned into an empty string in case of
> UnicodeDecodeError, see
> https://github.com/django/django/blob/master/django/template/base.py#L994-L
> 998
>

That code is within a render() method -- a place where the value is already
going to be turned into a string.

> IMO invalid variable should behave the same way regardless of whether it's
> printed or used as an filter argument.
>

My point was that rendering makes a difference because it already implies a
possible type change.

I stand by my earlier position: Changing a filter argument to a string is odd
and unexpected and we shouldn't do this. We should either keep raising an
exception, or replace the whole filter expression (rather than just the
argument) with string_if_invalid. My preference is towards the exception, but
I also see the value of failing silently here.

Shai.

Tim Graham

unread,
Jun 19, 2017, 2:10:54 PM6/19/17
to Django developers (Contributions to Django itself)
I think that to convince me to change the current behavior, you would have to present a compelling use case for relying on nonexistent variables as filter arguments. Wouldn't this behavior typically just hide bugs?

On a related note, I proposed removing the logging of undefined template variables here: https://groups.google.com/d/topic/django-developers/zdULZcmAWNw/discussion

allejj...@gmail.com

unread,
Jun 19, 2017, 5:48:36 PM6/19/17
to Django developers (Contributions to Django itself)
But if the current behaviour is left intact it goes against the definitions of filters like 'default' and 'default_if_none'.

If one reads the code snippet example in the ticket, the exception is raised even when the filter shouldn't be invoked.

So, for me the correct behaviour should be not raise any exceptions (maybe a warning could be used) in cases when filters used are 'default' or 'default_if_none', and are applied to a variable with a valid value that not trigger these filters.

Only we should focus to resolve (or raise VariableDoesNotExiste exception if applies) the variables in other filters, e.g if we have more chaining filters with variables like:
 
Template('{{ foo|default:notreal1|date:notreal2 }}').render(Context({'foo': '', notreal2: 'as'})).

Tim Graham

unread,
Jun 19, 2017, 6:58:56 PM6/19/17
to Django developers (Contributions to Django itself)
Could you describe your use case in more detail?

allejj...@gmail.com

unread,
Jun 21, 2017, 12:02:16 AM6/21/17
to Django developers (Contributions to Django itself)
# Some examples here for illustrate better my point of view:
from django.template import Context, Template

# CASE_1: shouldn't raise an exception because 'foo' variable exists in the context and 'default' filter
# shouldn't be invoked, 
Template('{{ foo|default:notreal }}').render(Context({'foo': ''}))       #so, the output should be ''

# CASE_2: should raise an exception because 'foo' variable not exist in the context and 'default' filter
# will be invoked with non existent variable as argument, 
Template('{{ foo|default:notreal }}').render(Context({}))       #so, the output should be the exception VariableDoesNotExist

# CASE_3: should raise an exception because although 'foo' variable exists in the context and 'default' filter
# shouldn't be invoked, the filter 'center' is called with non existent variable as argument, 
Template('{{ foo|default:notreal|center:notreal2  }}').render(Context({'foo': '', }))       #so, the output should be the exception VariableDoesNotExist but for 'notreal2' variable

# CASE_4: shouldn't raise an exception because 'foo' variable exists in the context and 'default' filter
# shouldn't be invoked and also the filter 'center' is called with existent variable as argument, 
Template('{{ foo|default:notreal|center:notreal2 }}').render(Context({'foo': 'django', 'notreal2':'15' }))       #so, the output should be '     django    '

# and the same reasoning and analogous examples should be derived for 'default_is_none' filter, only having in mind the definition of that filter

Tim Graham

unread,
Jun 22, 2017, 11:15:53 AM6/22/17
to Django developers (Contributions to Django itself)
My question is why is it problematic to put "notreal/notreal2" in the context, even if they're unused in some situations?

allejj...@gmail.com

unread,
Jun 23, 2017, 12:01:22 PM6/23/17
to Django developers (Contributions to Django itself)
IMHO I think it's more consistent with the definition of certain filters make the system somehow "lazy" and only evaluate filter variables only when are used.

Shai Berger

unread,
Jun 24, 2017, 1:44:47 PM6/24/17
to django-d...@googlegroups.com
On Friday 23 June 2017 19:01:21 allejj...@gmail.com wrote:
> IMHO I think it's more consistent with the definition of certain filters
> make the system somehow "lazy" and only evaluate filter variables only when
> are used.
>

Like everything else, it's a tradeoff: Making conditional filters "lazy" or
"short-circuit" may be consistent with their conditional semantics, but making
all filter arguments evaluated by the template engine creates more consistency
among filters and tags in general. Further, giving elements control over the
evaluation creates the temptation to take some strings verbatim from the
arguments -- which was the way the url tag used to work, and which was
painstakingly fixed at 1.5.

However, that whole discussion is somewhat tangential to the topic of this
thread -- if you want to open a discussion on "should filter arguments always
be evaluated", I think it would be more productive to open a new thread.

Shai.
Reply all
Reply to author
Forward
0 new messages