Exception swallowing in urls.py + admin.autodiscover() == a lot of frustration for developers

3 views
Skip to first unread message

Jonas Pfeil

unread,
Aug 24, 2008, 1:53:37 AM8/24/08
to Django developers
Dear Django developers,

The code importing urls.py has a flaw that is easy to fix but has the
potential to cause a lot of frustration especially for inexperienced
developers -- which I do hope will try out Django in large numbers
soon :)

The problem is this: When importing urls.py _any_ exception is caught
and then re-raised as an ImproperlyConfigured exception, not giving
the traceback. This was not that big of an issue until we started to
tell people to put admin.autodiscover() in their urls.py. Now a lot of
code can get executed from there and if you get an exception you have
no clue where it originated. This is especially true if you port a
large project to 1.0. I, at least, keep running into it ... but don't
take my word for it. I just found a thread [1] on django users on the
issue (#7524). To cite Karen Tracey:

> I agree that the way ImproperlyConfigured is swallowing traceback
> information is not good. It may not have been much of an issue before, but
> with admin.autodiscover() now recommended to be placed in urls.py, we are
> seeing a lot of confusion resulting from real errors in admin.py being
> hidden and the message seeming to imply something is wrong in urls.py. I
> don't have time right now, but you might want to search the tracker and see
> if there are any already-logged tickets about this less-than-helpful
> behavior and if not, open one.

Luckily the solution is quite easy. Just let the exception bubbly up
(see my patch [2]). It's changing nothing. We don't add any value by
wrapping the exception in an ImproperlyConfigured one because we do
not know anything about the error in this particular case.

Django swallows exceptions at multiple places and it is clear that we
need a unified solution. But in this case I think it's special. The
code does the wrong thing here.

Cheers,

Jonas

P.S.: I should have brought this up here earlier instead of causing a
somewhat lengthy discussion on IRC, sorry for that. I hope James
doesn’t hate me after this ;)

[1] http://groups.google.de/group/django-users/browse_thread/thread/487d8c3c28eee5f8
[2] http://code.djangoproject.com/attachment/ticket/7524/01-url-import-exception.patch

James Bennett

unread,
Aug 24, 2008, 2:26:07 AM8/24/08
to django-d...@googlegroups.com
On Sun, Aug 24, 2008 at 12:53 AM, Jonas Pfeil <jonas...@gmx.de> wrote:
> P.S.: I should have brought this up here earlier instead of causing a
> somewhat lengthy discussion on IRC, sorry for that. I hope James
> doesn't hate me after this ;)

I stand by what I said in the dev channel yesterday: this is one
instance of a more general issue (there are lots of places where --
because we need to import or check something -- we can end up
"swallowing" exceptions in this fashion), one which will require time
and thought to deal with, and which should not be tackled piecemeal.
Given that, and the fact that there's a lot of much more critical work
to do, I think the appropriate time to do that is **after** Django
1.0. This is simple prioritization (and, as others have pointed out to
you, the fact that there hasn't been a big flood of bug
reports/support problems related to this probably indicates it's not
as critical as you're making it out to be).

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

Karen Tracey

unread,
Aug 24, 2008, 3:11:25 AM8/24/08
to django-d...@googlegroups.com
On Sun, Aug 24, 2008 at 2:26 AM, James Bennett <ubern...@gmail.com> wrote:

I stand by what I said in the dev channel yesterday: this is one
instance of a more general issue (there are lots of places where --
because we need to import or check something -- we can end up
"swallowing" exceptions in this fashion), one which will require time
and thought to deal with, and which should not be tackled piecemeal.

I don't understand this argument.  At some point fixing this general issue is going to have to involve a piecemeal change of each instance where exceptions are currently swallowed.  (Or at least each instance where the swallowing/transforming is causing problems.)  How can this not be eventually fixed in a piecemeal fashion?  It sounds like maybe you want to design something consistent to do instead and not do anything until that consistent design is developed...is that what you're getting at?


Given that, and the fact that there's a lot of much more critical work
to do, I think the appropriate time to do that is **after** Django
1.0. This is simple prioritization (and, as others have pointed out to
you, the fact that there hasn't been a big flood of bug
reports/support problems related to this probably indicates it's not
as critical as you're making it out to be).

Except we did, in fact, see a definite spate of reports of  this problem after newforms-admin landed in trunk with the recommendation to put admin.autodiscover() in urls.py.  (How big is a spate compared to a flood? I don't know, and honestly I didn't keep a count or anything, and have zero interest in doing any archive searching to come up with one.)  My sense is we saw a good deal of resulting confusion.  If we don't do something to address this particular case of exceptions getting swallowed, I expect we'll see a bigger echo of that spate when 1.0 goes out and people who don't track trunk start updating. 

This one, I think, is worth fixing sooner rather than later.  I don't even know if the others are worth worrying about, since I can't say I recall any people on the users list running into trouble with other cases of this exception-swallowing behavior. This one I definitely have seen causing problems, triggered by newforms-admin causing a lot more code to get executed when urls.py is loaded.

Karen

James Bennett

unread,
Aug 24, 2008, 4:31:14 AM8/24/08
to django-d...@googlegroups.com
On Sun, Aug 24, 2008 at 2:11 AM, Karen Tracey <kmtr...@gmail.com> wrote:
> I don't understand this argument. At some point fixing this general issue
> is going to have to involve a piecemeal change of each instance where
> exceptions are currently swallowed. (Or at least each instance where the
> swallowing/transforming is causing problems.) How can this not be
> eventually fixed in a piecemeal fashion? It sounds like maybe you want to
> design something consistent to do instead and not do anything until that
> consistent design is developed...is that what you're getting at?

Yes. I'd rather have thought put into A) whether it's worth doing
something (after all, this is really more of a Python thing -- Python
has "except", not "except but only when the exception was in the last
stack frame") and B) what that thing is. I don't want to end up with a
patchwork of different solution because different cases ended up with
different people passionately arguing about how best to solve them.

> This one, I think, is worth fixing sooner rather than later. I don't even
> know if the others are worth worrying about, since I can't say I recall any
> people on the users list running into trouble with other cases of this
> exception-swallowing behavior. This one I definitely have seen causing
> problems, triggered by newforms-admin causing a lot more code to get
> executed when urls.py is loaded.

Then we need a *general* solution, that can and must be applied to all
the various places where we do stuff like this (template tag loading
also has the potential to "swallow" exceptions, as do other areas of
URL resolution, as does middleware loading, as do... well, lots of
stuff in Django). But again I think this comes down to prioritization;
it's really less of a bug in Django than it is an attempt to help
people rescue their own broken code (since the root of the issue is
that somebody will have broken code somewhere and the "real" exception
is masked when something else incidentally bumps against it), and we
still have plenty of genuine bugs where things in Django don't work
properly. So I still think this is a post-1.0 thing.

Jacob Kaplan-Moss

unread,
Aug 24, 2008, 10:24:58 AM8/24/08
to django-d...@googlegroups.com
Quick note: Malcolm and I are in Portland in the only place in the
city sans wifi. We've talked about this and the other exc swallowing
issue and I have some thoughts. Please hold until I'm in a more
civilized location and can actually use a keyboard bigger than a few
stamps.

Jacob

Sent from my iPhone

On Aug 24, 2008, at 1:31 AM, "James Bennett" <ubern...@gmail.com>
wrote:

>

Karen Tracey

unread,
Aug 26, 2008, 4:38:28 PM8/26/08
to django-d...@googlegroups.com
On Sun, Aug 24, 2008 at 10:24 AM, Jacob Kaplan-Moss <jacob.ka...@gmail.com> wrote:

Quick note: Malcolm and I are in Portland in the only place in the
city sans wifi. We've talked about this and the other exc swallowing
issue and I have some thoughts. Please hold until I'm in a more
civilized location and can actually use a keyboard bigger than a few
stamps.

So it's a couple of days later...got time to update with your thoughts?  I'm not trying to be a pain, I just spent some time investigating #8569 where this exception-swallowing behavior completely obscures the real origin of the error (which is obscure enough with the actual traceback).  I know how to get rid of it so it wasn't any big deal for me to patch the code to remove the exception swallowing, but I fear if 1.0 ships like this we'll be getting a lot of reports of ImproperlyConfigured exceptions with tantalizingly vague error messages and no real traceback informaiton to be able to guide people as to whether it's a bug in their code or a bug in Django.

Karen

Jacob Kaplan-Moss

unread,
Aug 26, 2008, 4:55:30 PM8/26/08
to django-d...@googlegroups.com
On Tue, Aug 26, 2008 at 3:38 PM, Karen Tracey <kmtr...@gmail.com> wrote:
> So it's a couple of days later...got time to update with your thoughts?

Yeah, I'm sorry; I lost track of this.

Essentially I think that James is right that a systematic fix would be
better, but I don't see what that might look like.

Unfortunately, I'm really not sure exactly what we can do -- there's
some places where perhaps we could not raise ImproperlyConfigured, but
the problem is that sometimes those messages are *more* helpful, and
other times less. It's not entirely clear (to me) what a fix, even a
half-assed one, would look like

I started going down the path of having ImproperlyConfigured take an
``original_exception`` argument and displaying that original exception
from manage.py, but that's pretty fiddly.

So I think we need to do something along the lines of what's in
#7524... it's far, far from perfect, but it's probably the only way to
go to avoid a lot of frustration.

Jacob

Karen Tracey

unread,
Aug 26, 2008, 5:13:51 PM8/26/08
to django-d...@googlegroups.com
On Tue, Aug 26, 2008 at 4:55 PM, Jacob Kaplan-Moss <jacob.ka...@gmail.com> wrote:
So I think we need to do something along the lines of what's in
#7524... it's far, far from perfect, but it's probably the only way to
go to avoid a lot of frustration.


So should #7524 get moved back to a 1.0 milestone? FWIW it is the 2nd patch on that ticket I used to enable me to see what was the real issue with the problem I was looking at earlier today, and that ticket has at least 2 dups with nearly identical patches, so that particular case of wrapping the original exception in ImproperlyConfigured does seem to be causing a fair number of headaches.  Doing something to address it, alone, before 1.0 ships would be a good idea, IMO.

Karen

Thomas Guettler

unread,
Aug 28, 2008, 9:26:59 AM8/28/08
to django-d...@googlegroups.com
Jacob Kaplan-Moss schrieb:
On Tue, Aug 26, 2008 at 3:38 PM, Karen Tracey <kmtr...@gmail.com> wrote:
  
So it's a couple of days later...got time to update with your thoughts?
    
Yeah, I'm sorry; I lost track of this.

Essentially I think that James is right that a systematic fix would be
better, but I don't see what that might look like.

Unfortunately, I'm really not sure exactly what we can do -- there's
some places where perhaps we could not raise ImproperlyConfigured, but
the problem is that sometimes those messages are *more* helpful, and
other times less. It's not entirely clear (to me) what a fix, even a
half-assed one, would look like

  

> I started going down the path of having ImproperlyConfigured take an
> ``original_exception`` argument and displaying that original exception
> from manage.py, but that's pretty fiddly.
I think catching any exception an raising ImporperlyConfigured is a good
solution. But the original exception should not be lost. I wrote a patch several
month ago. The original exception gets  displayed in the debug view. manage.py
is not handled:

http://code.djangoproject.com/attachment/ticket/6537/6537.diff


So I think we need to do something along the lines of what's in
#7524... it's far, far from perfect, but it's probably the only way to
go to avoid a lot of frustration.

  
Yes, I think this is a show stopper which should be solved before 1.0.

 Thomas

-- 
Thomas Guettler, http://www.thomas-guettler.de/
E-Mail: guettli (*) thomas-guettler + de
Reply all
Reply to author
Forward
0 new messages