DebugNodeList breaks exceptions
|Reported by:||Glenn Maynard||Owned by:||nobody|
|Cc:||glenn@…, john.firebaugh@…, hanne.moa@…, physicsnick@…, michaelg@…, django@…||Triage Stage:||Design decision needed|
|Has patch:||yes||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
DebugNodeList catches all exceptions, sticks them in a TemplateSyntaxError, and stuffs the original exception in the new exception. I'm not sure why this is done, but it breaks debugging and exception handling.
- It masks the backtrace. Django's ExceptionReporter receives and reports the wrapper exception, never seeing the wrapped exception, so the backtrace starts at DebugNodeList's "raise wrapped" line--the location of the actual error is lost. This makes errors inside templates very hard to debug: the only evidence of the real error is the error string. The same problem will happen with any custom exception reporter.
- It's the wrong exception. If my template calls a filter which raises NameError, it's not a template syntax error; it's a NameError.
- It blocks exceptions which were raised intentionally. If I call a filter which raises IOError, the code calling the template should be able to catch that.
I'm attaching two alternate patches:
#1 (template-dont-wrap-exception.diff): Patch removes the TemplateSyntaxError wrapping; it just attaches the source to all exceptions and re-raises. With this patch, exceptions coming out of template filters, etc. can be caught properly when template debugging is enabled, and are reported correctly by standard exception reporters.
#2 (template-fix-lost-traceback.diff): Includes the original tb when re-raising the exception. This is the correct way to raise exceptions like this, when that's really what you want to do. This fixes backtraces only, which is the most annoying problem. It doesn't fix the problems caused by changing the exception type. I include this because it's straightforwardly committable (as I don't know the original reason for the wrapped exception), but I strongly prefer #1.
(related: ticket #11368)
Change History (24)
comment:1 Changed 7 years ago by
|Patch needs improvement:||unset|
|Triage Stage:||Unreviewed → Design decision needed|