Should url() respect string_if_invalid?

Django Internals
1

You can use the string_if_invalid setting of the DjangoTemplates back-end to instruct Django on how to handle missing template variables.

You can use this, for example, to force Django to raise an exception when a template calls an undefined variable, which is useful when running unit tests.

In a project I use the url template tag with variable assignment ({% url 'path' as var %}).

The url tag has two different behaviors when it fails to reverse a path:

  1. When called directly, it raises a NoReverseMatch.

  2. When written to a variable, it returns an empty string.

This logic lives in the render() method of django.template.defaulttags.URLNode.

Personally, I expected the url method to defer to string_if_invalid (or something similar) before suppressing the error entirely when using as var.

What do you think about this?

This started off as a Django Discord thread in the #testing channel, for those who are interested in that.

NB. I am currently unsure how much of this is a Django Core issue, and how much it is a pytest-django one.

(For pytest-django users, see the setting FAIL_INVALID_TEMPLATE_VARS, or --fail-on-template-vars flag).

1 Like
2

Personally, I would have expected this to raise the NoReverseMatch exception, regardless of string_if_invalid.

3

@CodenameTim +1. The history for this line doesn’t state how this decision came to be; the if-statement itself has been in this place for at least 16 years (coming from SVN).

My guess is that this was done to avoid crashes, just like calling non-existing template variables doesn’t crash templates by default.

4

<opinion>
I think it’s right as it is, both based on the code and how it’s documented in the url tag.

I think the key difference is that in the as var version, you’re not (necessarily) rendering that variable. That tag does not return a string to be injected into the template being rendered.

The string_if_invalid is defined as:

the output, as a string, that the template system should use for invalid (e.g. misspelled) variables.

But there’s no output here.

</opinion>

Side note: The docs explain why this option exists:

This {% url ... as var %} syntax will not cause an error if the view is missing. In practice you’ll use this to link to views that are optional

{% url 'some-url-name' as the_url %}
{% if the_url %}
  <a href="{{ the_url }}">Link to optional stuff</a>
{% endif %}
2 Likes
5

Thanks, Ken! I get that it’s a feature and not a bug, but I would still be interested in a way to allow this method to fail even when using asvar.

For code readability reasons, I often declare URLs as variables on their own line, so they can be printed as a variable in an href. It also allows me to do checks in the HTML to see if the aria-current attribute needs to be added anywhere (when building navigations). In such situations, the URLs are not optional.

6

My initial reactions, depending upon exactly what you would want the semantics to be, makes me think of a couple of different options.

  • If you want the template to “work”, but flag the variables if the url tag doesn’t find a match, you could use the default filter.
    Using the example above:
    {{ the_url|default:"No url found" }}

  • If you want the template to raise an error, you could create your own version of the default filter to raise an error instead of returning a value.
    e.g.:

@register.filter(is_safe=False)
def validate(value):
    """If value is unavailable, raise exception"""
    if not value:
        raise NoReverseMatch
    return value

which would then be used as:
{{ the_url|validate }}

  • You could create your own version of the url tag that creates a custom URLNode subclass that raises the NoReverseMatch exception regardless of the status of the asvar attribute of that class. (It’s significantly more work/code than the previous, but potentially keeps your templates looking “cleaner” and flags the error at the tag instead of at the reference to the resolved url.)
    The URLNode subclass could look something like this:
class URLValidNode(URLNode):
    def render(self, context):
        url = super().render(context)
        if not url:
            raise NoReverseMatch
        return url

Note: None of these code snippets have in any way been tested - I’m just floating ideas that I think might be worth investigating.

1 Like
7

Thanks again. I was hoping for something that works exactly like the Pytest configuration does (so, plug-and-play).

The custom template tag is my favorite suggestion so far, and I will consider it.

This is the code that cb109 suggested on Discord, which is very similar to your last suggestion:

from django.template.defaulttags import URLNode

orig_url_render = URLNode.render

def patched_url_render(self, context):
	result = orig_url_render(self, context)
	if result == '':
		msg = 'URL tag result should not be an empty string'
		raise ValueError(msg)
	return result

URLNode.render = patched_url_render
8

I help people with this issue all the time. In my opinion the entire “templates shouldn’t crash” idea is a bad idea through and through. It just leads to code that stays broken for a long time and no one notices in the best case, and in the worst case it leads to beginners being frustrated because there are silent failures until they give up on Django.

The suggestion from cb109 won’t help a lot imo, as it will still be extremely confusing. “Should not be an empty string” when the user misspelled a variable isn’t a good error message.

There is only one thing that will save you when the code is broken: a crash.