Be explicit about which reverse lookup failed after r8211

[mrts]

Now that {% url foo %} does not fail silently any more, it's really
important to report which reverse lookup failed.

Traceback (most recent call last):
...
File "/usr/lib/python2.5/site-packages/django/core/urlresolvers.py",
line 291, in reverse
raise NoReverseMatch
NoReverseMatch

isn't particularly helpful :). I expect #django will be swarming with
questions "how can I find out which url tag is failing" real soon.

I was hit by this recently and the fix is extremely simple [1]. After
the patch attached to [1], the output is as follows:

Traceback (most recent call last):
...
File "/usr/lib/python2.5/site-packages/django/core/urlresolvers.py",
line 291, in reverse
raise NoReverseMatch("Reverse for '%s' not found." % lookup_view)

NoReverseMatch: Reverse for 'foo-bar' not found.

A quick rgrep foo-bar . after that and you're done.

[1] http://code.djangoproject.com/ticket/8177

[mrts]

And note that I'm +42 on {% url %} not failing silently.

[Julien Phalip]

+1 for this.

Although the URL tag currently only takes hard coded strings as
parameters, you still can wrap it with a higher level template tag to
enable passing the view as a variable:

{% mucustom_url foo.view %}

In that case, it is currently impossible to know which view caused the
exception to be raised...

[koenb]

On 9 aug, 09:49, mrts <m...@mrts.pri.ee> wrote:
> And note that I'm +42 on {% url %} not failing silently.
>

I disagree.
This really breaks my development cycle.
I tend to provide url tags with references to not yet developped views
in my templates (often in different apps not yet developped).
Checking that those links have been filled correctly is something I
always do in my tests. This means that while I have not yet finished
those other parts, my tests may fail, but at least my pages "work" in
the development server. When I finish the other views, there is no
need to go back to look for all references in my templates.

With the new behavior, I have to either create placeholder views
(which means start mixing work on different apps) or create
placeholder texts in my templates (which means going back to replace
them afterwards), neither of which I would like to do.

To me it would be more usefull to show NoReverseMatch as warnings and
not as errors.

Koen

[Mike Scott]

Koen:

Then thats a failed test isn't it? Its not working correctly, and it
SHOULD break. Except in this case you expect that? I don't see the
problem with that. If you don't want it to work, comment it out.

[mrts]

I scratched where it itched when providing the patch in #8177.
Admittedly I should have looked around core.urlresolvers a bit more,
fixing other places as well. I have done that now, the improved patch
is attached to #8221 (http://code.djangoproject.com/attachment/ticket/
8221/improved_exceptions_for_reverse2.diff ).

On Aug 9, 10:38 am, mrts <m...@mrts.pri.ee> wrote:

[Scott Moonen]

One other thing I've found a bit odd.  I have an app import path of appname.views.*, and a project import path of apps.myproject.settings.  I had a broken url tag (I was inadvertently providing a value of None for a parameter, causing the reverse regex match to fail).  What was interesting was that the exception that surfaced in my view indicated that there was no reverse match for apps.appname.views.x instead of appname.views.x

I wasted a bit of time trying to figure out how in the world 'apps' got added to the view import path that was being looked up.  I had recently done a little bit of directory structure so it seemed like the problem might be my fault.  Turns out that it is the fault of django.template.defaulttags.URLNode.render().  It catches a NoReverseMatch on the original view path and then tries to do the lookup by augmenting the requested path with the first component of the settings module path.  It was this second reverse lookup that failed.

Do we know if any code is relying on this augmentation behavior?  It seems a little odd, especially since in many cases the string I'm providing for a reverse lookup is a name rather than a view path.

Assuming that we keep the augmentation behavior, it would be more usable to have URLNode.render() re-raise the original NoReverseMatch exception if the second call to reverse() fails similarly.

If folks agree I will open a ticket,

  -- Scott

--
http://scott.andstuff.org/ | http://truthadorned.org/

[Julien Phalip]

> Turns out that it is the fault of
> django.template.defaulttags.URLNode.render().  It catches a NoReverseMatch
> on the original view path and then tries to do the lookup by augmenting the
> requested path with the first component of the settings module path.  It was
> this second reverse lookup that failed.

Good catch! This might partly explain some of the weird stuff
happening in #8251.
http://code.djangoproject.com/ticket/8251

[Yuri Baburov]

Yes, i did the following quick fix on r8338, because first try-except
can catch *REAL* bug issue as it was in my case: it swallowed error
that "NoReverseMatch: Not enough positional arguments passed in" and
nothing worked.

diff --git a/django/template/defaulttags.py b/django/template/defaulttags.py
index 489faa2..03f4669 100644
--- a/django/template/defaulttags.py
+++ b/django/template/defaulttags.py
@@ -363,10 +363,13 @@ class URLNode(Node):
for k, v in self.kwargs.items()])
try:
return reverse(self.view_name, args=args, kwargs=kwargs)
- except NoReverseMatch:
- project_name = settings.SETTINGS_MODULE.split('.')[0]
- return reverse(project_name + '.' + self.view_name,
- args=args, kwargs=kwargs)
+ except NoReverseMatch, e1:
+ try:
+ project_name = settings.SETTINGS_MODULE.split('.')[0]
+ return reverse(project_name + '.' + self.view_name,
+ args=args, kwargs=kwargs)
+ except NoReverseMatch, e2:
+ raise e1

class WidthRatioNode(Node):
def __init__(self, val_expr, max_expr, max_width):

I'm afraid you guys would have to remove too smart functionality
trying to add settings path, or i'd suggest to make 2 different
exceptions: one for NoReverseMatch (long-circuit), second for
ReverseMatchFailed (short-circuit).

--
Best regards, Yuri V. Baburov, ICQ# 99934676, Skype: yuri.baburov,
MSN: bu...@live.com

[mrts]

It is quite common to be hit by the insufficiently verbose reporting
that #8221 and #7524 fix -- e.g. see the duplicates that have popped
up.

As I already said, #8221 is only needed because the patch I provided
in #8177 and that got commited fixed only the most burning issue I was
directly hit with. Now that I looked around in urlresolvers, I found
other places where similar improvements were needed. #8221 covers all
them and supersedes #8177.

I personally think both should make it into 1.0, but James seems to
oppose, so can we discuss this a bit further?

Regards,
Mart Sõmermaa

[Malcolm Tredinnick]

On Sat, 2008-08-23 at 10:46 -0700, mrts wrote:
[...]

> I personally think both should make it into 1.0, but James seems to
> oppose, so can we discuss this a bit further?

The ticket is open. It will either be committed, postponed or closed as
a dupe of something else. Let's leave it at that and get some other work
done in the interim.

Malcolm

[mrts]

http://code.djangoproject.com/ticket/7524 is tagged as post-1.0.
http://code.djangoproject.com/ticket/8221 was closed as duplicate of
#7524, which it is not.

On Aug 23, 9:40 pm, Malcolm Tredinnick <malc...@pointy-stick.com>
wrote:

[James Bennett]

On Sat, Aug 23, 2008 at 2:43 PM, mrts <mr...@mrts.pri.ee> wrote:
> http://code.djangoproject.com/ticket/7524 is tagged as post-1.0.
> http://code.djangoproject.com/ticket/8221 was closed as duplicate of
> #7524, which it is not.

In triaging, I'm trying to take the position that all of the various
"Django looks for an exception and accidentally swallows some other
exception" tickets are really the same issue and should all be dealt
with together, rather than being attacked piecemeal.

--
"Bureaucrat Conrad, you are technically correct -- the best kind of correct."

[mrts]

On Aug 24, 4:40 am, "James Bennett" <ubernost...@gmail.com> wrote:

> On Sat, Aug 23, 2008 at 2:43 PM, mrts <m...@mrts.pri.ee> wrote:
> >http://code.djangoproject.com/ticket/8221 was closed as duplicate of
> > #7524, which it is not.
>
> In triaging, I'm trying to take the position that all of the various
> "Django looks for an exception and accidentally swallows some other
> exception" tickets are really the same issue and should all be dealt
> with together, rather than being attacked piecemeal.

That argument pertains to #7524 and to only *half* of #8221 -- the
except (ImportError, AttributeError): raise NoReverseMatch("Error
importing '%s': %s." % (viewname, e)) bits in there. A general
agreement is indeed needed (e.g. what to do with the original
backtrace), but IMHO this should be in scope for 1.0.

The other half is different, although in the same spirit of "try to
provide all available information that helps to solve the problem" --
raise NoReverseMatch("Reverse for '%s' with arguments '%s' and keyword
arguments '%s' not found." % (lookup_view, args, kwargs)). This helped
me to track down an invalid slug that had slipped into an article
database. The article index view was dying with NoReverseMatch and I
had no clue why (everything *looked* OK) -- found it only after
patching my Django checkout with this.