Opened 17 years ago

Closed 14 years ago

#5140 closed (worksforme)

url templatetag should raise exception on invalid path

Reported by: oliver.andrich@… Owned by: Adrian Holovaty
Component: Template system Version: dev
Severity: Keywords: url templatetag
Cc: Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I just stumbled across the fact, that the url templatetag silently accepts any path to a view - no matter if it exists or not. In the module django.template.defaulttags I have seen, that after processing the templatetag an URLNode is generatet and afterwards rendered.

    def render(self, context):
        from django.core.urlresolvers import reverse, NoReverseMatch
        args = [arg.resolve(context) for arg in self.args]
        kwargs = dict([(smart_str(k,'ascii'), v.resolve(context)) for k, v in self.kwargs.items()])
        try:
            return reverse(self.view_name, args=args, kwargs=kwargs)
        except NoReverseMatch:
            try:
                project_name = settings.SETTINGS_MODULE.split('.')[0]
                return reverse(project_name + '.' + self.view_name, args=args, kwargs=kwargs)
            except NoReverseMatch:
                return ''

The last try-catch-block which catches the second NoReverseMatch exception is causing the troubles. At least in my eyes.
I would find it much more convenient if I actually get the exception when I cause such an error. This would have saved my some time today, as I created such an error during my Django studies.

Attachments (2)

url_template_tag_raises_exception_on_invalid_path.patch (1.0 KB ) - added by oliver.andrich@… 17 years ago.
A newbies patch for this ticket.
propagate_exception_if_debug.patch (729 bytes ) - added by Kenneth Arnold 17 years ago.
Propagate the NoReverseMatch exception if the reverse lookup fails

Download all attachments as: .zip

Change History (10)

by oliver.andrich@…, 17 years ago

A newbies patch for this ticket.

comment:1 by Malcolm Tredinnick, 17 years ago

Resolution: wontfix
Status: newclosed

Template tags should never raise exceptions. That must always fail quietly. That is a design feature and there is a lot of code and templates that rely on that (it saves in duplication in templates in a number of places, for example).

comment:3 by orutherfurd <orutherfurd+django@…>, 17 years ago

What about letting the exception percolate up when in DEBUG mode, or based on a config knob? It's a pity that this, otherwise very useful, tag silently returns nothing -- in my apps, it's always a mistake when lookup fails.

comment:4 by Kenneth Arnold, 17 years ago

Resolution: wontfix
Status: closedreopened

Agreed with orutherfurd. I just cleaned up my urls, and a quick-and-dirty patch for this bug is greatly helping me track down my mistakes.

My patch just propagates the exception if settings.TEMPLATE_DEBUG. Better solutions might be:

  1. Use another setting, something like FAIL_ON_TEMPLATE_ERRORS. As mtredinnick says, this might catch too much.
  2. Return an object that compares like a string (so you can use the result in an if tag), but raises an exception when rendering.

But these are only concerns for other template errors; url lookups failing is always a mistake. (I am willing to be proven wrong there.)

by Kenneth Arnold, 17 years ago

Propagate the NoReverseMatch exception if the reverse lookup fails

comment:5 by Chris Beaven, 17 years ago

Has patch: set
Triage Stage: UnreviewedDesign decision needed

comment:6 by sorl, 17 years ago

Correct me if I am wrong but the docs don't say Templae tags can't raise exceptions. I am also +1 to raise an error on NoReverseMatch. Debugging is almost impossible as is.

comment:7 by sorl, 17 years ago

Just quoting the docs:

render() should never raise TemplateSyntaxError or any other exception. It should fail silently, just as template filters should.

So this is the problem.

comment:8 by Alex Gaynor, 14 years ago

Resolution: worksforme
Status: reopenedclosed

The URL tag is now noisy.

Note: See TracTickets for help on using tickets.
Back to Top