Opened 16 years ago

Closed 12 years ago

#10802 closed Bug (fixed)

urlresolvers RegexURLResolver._get_callback should not catch ImportError and AttributeError

Reported by: Ionel Cristian Maries Owned by: nobody
Component: Core (URLs) Version: dev
Severity: Normal Keywords: urlresolver, ViewDoesNotExistError, AttributeError, ImportError
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Because we lose valuable traceback information - it should be very clear where is the source of the import error.

Attachments (4)

core-urlresolvers.patch (938 bytes ) - added by Ionel Cristian Maries 16 years ago.
django-10802.diff (6.3 KB ) - added by Bas Peschier 13 years ago.
django-10802.2.diff (5.1 KB ) - added by Bas Peschier 13 years ago.
django-10802.3.diff (6.3 KB ) - added by Bas Peschier 13 years ago.

Download all attachments as: .zip

Change History (17)

by Ionel Cristian Maries, 16 years ago

Attachment: core-urlresolvers.patch added

comment:1 by Alex Gaynor, 15 years ago

Triage Stage: UnreviewedDesign decision needed

comment:2 by Ionel Cristian Maries, 15 years ago

Any updates? 10 months is a long time.

comment:3 by Chris Beaven, 14 years ago

Severity: Normal
Type: Bug

Indeed it is a long time. Bring it up in the django developers group and see if you can get things moving.

comment:4 by Graham Dumpleton, 14 years ago

FWIW, I supplied the following code just recently to a mod_wsgi user so they could dump out the original traceback and thus work out where original source of problem is:

import traceback
import sys

def dump_exception(callable):
   def wrapper(*args, **kwargs):
       try:
           return callable(*args, **kwargs)
       except:
           traceback.print_exception(*sys.exc_info())
   return wrapper

import django.core.urlresolvers

urlresolvers.get_callable = dump_exception(urlresolvers.get_callable)

This was added into the WSGI script file, with the result being that traceback was then output to error log.

The Python agent for New Relic RPM I am working on will also instrument get_callable in a similar way so as to capture exception details within a view for which traceback is otherwise lost.

One other area where tracebacks are lost is in importing of the DJANGO_SETTINGS_MODULE. That only happens once on startup and is a PITA to wrap like above as there is no intermediary function that can wrap and so one would have to use byte code level instrumentation.

comment:5 by Bas Peschier, 13 years ago

Easy pickings: unset
UI/UX: unset

Ok, so this was brought up recently on django-developers [1]. Attached above is a patch which will raise ViewDoesNotExist, but only if

  1. The owning module exists, but has no attribute for the view.
  2. The owning module does not exist.
  3. The "view" does exists, but is not callable.

In case of an ImportError due to other reasons, it passes it on.

The patch moves ViewDoesNotExist to the get_callable function where it actually can detect it correctly.

[1]: http://groups.google.com/group/django-developers/browse_thread/thread/62bc422bd34e247e

by Bas Peschier, 13 years ago

Attachment: django-10802.diff added

by Bas Peschier, 13 years ago

Attachment: django-10802.2.diff added

comment:6 by Bas Peschier, 13 years ago

Hmmm, trac screwed up there for a minute, I tried to overwrite the patch because of missing added files. The first patch is the correct one.

comment:7 by Russell Keith-Magee, 13 years ago

Triage Stage: Design decision neededReady for checkin

First patch from bpeschier looks good to me.

comment:8 by Jannis Leidel, 13 years ago

Patch needs improvement: set

Doesn't apply to trunk after the i18n urls landed.

by Bas Peschier, 13 years ago

Attachment: django-10802.3.diff added

comment:9 by Bas Peschier, 13 years ago

Patch needs improvement: unset

Updated trunk and rediffed.

comment:10 by Jannis Leidel, 13 years ago

Resolution: fixed
Status: newclosed

In [16420]:

Fixed #10802 -- Handle ImportErrors and AttributeErrors gracefully when raised by the URL resolver system during startup. Many thanks, IonelMaries and Bas Peschier.

comment:11 by rod333@…, 12 years ago

Component: Core (Other)Core (URLs)
Has patch: unset
Keywords: urlresolver ViewDoesNotExistError AttributeError ImportError added
Resolution: fixed
Status: closedreopened
Type: BugUncategorized

The traceback would be very useful in certain situations. For example:
http://pastebin.com/zs7Z2y4W

In this case, home2 would get a ViewDoesNotExist error for app.views.home2 - even if it has nothing to do with the form! That makes it very difficult to find the bug.

I don't mean to be annoying when I reopen a ticket like this. I'm genuinely curious - what's the logic behind making hiding the callback here?

comment:12 by anonymous, 12 years ago

Triage Stage: Ready for checkinUnreviewed

comment:13 by Tomek Paczkowski, 12 years ago

Has patch: set
Resolution: fixed
Status: reopenedclosed
Type: UncategorizedBug

rod333 it's generally bad to reopen ticket after it has been closed by core dev, doubly so when it was closed after commit, triply, if ticket was closed over year ago.
I'm re-closing this ticket. If you think there is a problem with current committed implementation, please open new one.

(Please don't take it personally, I'm not trying to be an ass, I'm just trying to make things more manageable on trac)

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