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)
Change History (17)
by , 16 years ago
Attachment: | core-urlresolvers.patch added |
---|
comment:1 by , 15 years ago
Triage Stage: | Unreviewed → Design decision needed |
---|
comment:2 by , 15 years ago
comment:3 by , 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 , 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 , 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
- The owning module exists, but has no attribute for the view.
- The owning module does not exist.
- 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 , 13 years ago
Attachment: | django-10802.diff added |
---|
by , 13 years ago
Attachment: | django-10802.2.diff added |
---|
comment:6 by , 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 , 13 years ago
Triage Stage: | Design decision needed → Ready for checkin |
---|
First patch from bpeschier looks good to me.
comment:8 by , 13 years ago
Patch needs improvement: | set |
---|
Doesn't apply to trunk after the i18n urls landed.
by , 13 years ago
Attachment: | django-10802.3.diff added |
---|
comment:11 by , 12 years ago
Component: | Core (Other) → Core (URLs) |
---|---|
Has patch: | unset |
Keywords: | urlresolver ViewDoesNotExistError AttributeError ImportError added |
Resolution: | fixed |
Status: | closed → reopened |
Type: | Bug → Uncategorized |
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 , 12 years ago
Triage Stage: | Ready for checkin → Unreviewed |
---|
comment:13 by , 12 years ago
Has patch: | set |
---|---|
Resolution: | → fixed |
Status: | reopened → closed |
Type: | Uncategorized → Bug |
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)
Any updates? 10 months is a long time.