Opened 14 years ago

Closed 9 years ago

#12995 closed New feature (fixed)

"source" exception attribute no longer handled properly by debug exception handler

Reported by: Glenn Maynard Owned by: nobody
Component: Template system Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


The "source" attribute on exceptions handled by the debug view was very handy; I set it in my own custom templating system (which coexists with Django's) and it automatically showed up in errors generated by Django's, complete with line numbers, just like they do for when Django templates set that attribute. Duck typing made this work implicitly, which was extremely useful--that's how duck typing is supposed to work.

r12586 broke this, adding an isinstance check which makes it so only errors from Django's own templates can use this feature. This doesn't make sense; duck typing was the correct approach here.

Please revert r12586; that was an unnecessarily destructive change. If you're having namespace collisions, then use a less generic name ("source_with_lines" or something like that).

Change History (13)

comment:1 by Alex Gaynor, 14 years ago

Triage Stage: UnreviewedAccepted
Version: 1.1SVN

This change isn't going to be reverted wholesale. Also, how is changing to the attribute "source_with_lines" any less backwards incompatible for your usecase? I am accepting this ticket as custom templating systems should have a way of providing debug info.

comment:2 by Glenn Maynard, 14 years ago

I don't care about backwards-compatibility here; I care about being able to do this at all. I can change my source attribute to eg. "source_with_lines" without significant problems, but I can't make all of my exceptions derive from TemplateSyntaxError. (It applies the source attribute to the existing exception and re-throws it without changing the exception type, simply augmenting the exception in-place as it passes out of the template. IMO, this is what Django's templating should be doing, too, to avoid destroying the original exception.)

comment:3 by Russell Keith-Magee, 14 years ago

If you read up on the history of the original bug, the change to avoid the source attribute was done specifically to support non-Django template languages (in particular Jinja2), and to avoid clashes with other libraries (like PyXML) that use a source attribute but have nothing to do with the templating system.

So - we're not going to simply revert here.

However, I agree that a simple type check isn't ideal -- [and I've said as much on django-dev]. Patches implementing a more sophisticated approach to template exception handling are welcome.

comment:4 by Glenn Maynard, 14 years ago

That's exactly why I suggested renaming the attribute to a less generic name than "source". I don't see any problems with "source_with_lines" (other than sounding a little awkward). If you're still concerned about namespace collisions--it's inherent with duck typing--try "django_source".

comment:5 by James Bennett, 14 years ago

Is this a duplicate of #12992?

in reply to:  5 comment:6 by Karen Tracey, 14 years ago

Replying to ubernostrum:

Is this a duplicate of #12992?

No. That one's about the file name not being known with the new loaders (from r11862). This one is about the source lines only being shown if the exception raised is a Django TemplateSyntaxError (due to r12586), breaking debugging for other templating systems.

comment:7 by Armin Ronacher, 14 years ago

I think the correct solution would be adding a hook into the DebugView to inject your own code there.

comment:8 by Glenn Maynard, 14 years ago

Storing source information in the exception is cleaner. It means that any templating engine supporting the interface can supply source context to any exception renderer supporting it, without the user code sitting between the two needing to manually ferry the information across with a hook. (Not to suggest that the interface should be made public as it is; it should also allow getting template source for each stack frame.)

Another reason (the original reason I noticed this) is discussed in ticket #11461. DebugNodeList shouldn't be destroying the original exception to attach information; it should simply be attaching the data to the existing exception. r12586 makes cleanly fixing that impossible.

comment:9 by Luke Plant, 13 years ago

Type: New feature

comment:10 by Luke Plant, 13 years ago

Severity: Normal

comment:11 by Aymeric Augustin, 12 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:12 by Aymeric Augustin, 12 years ago

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:13 by Aymeric Augustin, 9 years ago

Resolution: fixed
Status: newclosed

This was fixed in [4397c587]. See also #16770.

django_template_source was chosen as a "less generic name".

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