Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#25373 closed New feature (fixed)

Add logging for {% include %} exceptions when template.debug = False

Reported by: Nick Johnson Owned by: Nick Johnson
Component: Template system Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Exceptions raised from ModelAdmin fields during the rendering of the admin detail view for a model are silently caught and there's no simple way to log them without putting the template renderer in debug mode.

As a fix I'd like the ability to log these exceptions via the normal logging facilities, something akin to this change:

The problematic exception catching line is here

example with reproduction steps

Attachments (1)

template_import_logging.diff (3.0 KB) - added by Nick Johnson 5 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 Changed 5 years ago by Tim Graham

Component: contrib.adminTemplate system
Easy pickings: unset
Summary: Django Admin silently catches exceptions without a chance to log themAdd logging for {% include %} exceptions when template.debug = False
Triage Stage: UnreviewedAccepted
Type: BugNew feature

Updated title to reflect that the issue is really about the template system and the {% include %} template tag, not the admin. As the docs note:

When debug mode is turned on, an exception like TemplateDoesNotExist or TemplateSyntaxError will be raised; otherwise {% include %} silences any exception that happens while rendering the included template and returns an empty string.

comment:2 Changed 5 years ago by Nick Johnson

if you want these logged via the django.template logger I can put a patch together with a fix and a regression test for review

comment:3 Changed 5 years ago by Tim Graham

I didn't have an alternate implementation in mind.

comment:4 Changed 5 years ago by Francisco Saldaña

Owner: changed from nobody to Francisco Saldaña
Status: newassigned

comment:5 Changed 5 years ago by Francisco Saldaña

Owner: Francisco Saldaña deleted
Status: assignednew

comment:6 Changed 5 years ago by Nick Johnson

Owner: set to Nick Johnson
Status: newassigned

Changed 5 years ago by Nick Johnson

comment:7 Changed 5 years ago by Nick Johnson

Has patch: set

comment:8 Changed 5 years ago by Nick Johnson

After some further digging it appears the django.template logger added in the commit I originally referenced will capture exceptions bubbled through this function, however it will also catch things like missing variables in templates and expected fallthrough behavior. All of this is at a level of DEBUG which seems appropriate.

In order to be able to log only unexpected exceptions, I'm proposing adding WARNING level logging to any uncaught exceptions during import templatetag rendering. See attached patch for changes and regression test.

comment:9 Changed 5 years ago by Nick Johnson

fot ease of review I've created a PR here: https://github.com/django/django/pull/5285

comment:10 Changed 5 years ago by Simon Charette

Patch needs improvement: set

Left some comment on the PR.

comment:11 Changed 5 years ago by Nick Johnson

Patch needs improvement: unset

comment:12 Changed 5 years ago by Simon Charette

Triage Stage: AcceptedReady for checkin

Given a docs review.

comment:13 Changed 5 years ago by Tim Graham

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

Left a few comments for improvement.

comment:14 Changed 5 years ago by Nick Johnson

Patch needs improvement: unset

should be all cleaned up

comment:15 Changed 5 years ago by Tim Graham

Patch needs improvement: set

comment:16 Changed 5 years ago by Nick Johnson

Patch needs improvement: unset

Should be all cleaned up again. docs and isort are broken on CI right now, but they're broken for all PRs and are not caused by my change.

comment:17 Changed 5 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 392f6484:

Fixed #25373 -- Added warning logging for exceptions during {% include %} tag rendering.

comment:18 Changed 5 years ago by Tim Graham <timograham@…>

In b1f60460:

Refs #25373 -- Doc'd logging of exceptions during {% include %} rendering.

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