Opened 6 years ago

Closed 6 years ago

Last modified 5 years ago

#15603 closed (fixed)

Don't send HTML emails by default

Reported by: Karen Tracey Owned by: Adrian Holovaty
Component: Core (Other) Version: 1.3-rc
Severity: Keywords: blocker
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

In my experience (gmail accounts) the HTML error traceback emails new with 1.3 are much harder to read than the previous plaintext emails. They also expose far more information (potentially sensitive local variable values) than the previous emails. Proposal is to not send HTML by default, but make it easy to configure for HTML inclusion if that is what is really wanted. Attached patch does this. To get HTML included in the email you'd change your logger config to:

LOGGING = { 
    'version': 1, 
    'disable_existing_loggers': False, 
    'handlers': { 
        'mail_admins': { 
            'level': 'ERROR', 
            'class': 'django.utils.log.AdminEmailHandler', 
            'include_html': True, 
        } 
    }, 
    'loggers': { 
        'django.request':{ 
            'handlers': ['mail_admins'], 
            'level': 'ERROR', 
            'propagate': True, 
        }, 
    } 
}

Only change is the inclusion of the include_html: True on the mail_admins handler definition.

Attachments (1)

logger.diff (2.8 KB) - added by Karen Tracey 6 years ago.
Updated patch to include doc

Download all attachments as: .zip

Change History (11)

comment:1 Changed 6 years ago by Karen Tracey

Owner: changed from nobody to Karen Tracey
Triage Stage: UnreviewedAccepted

Changed 6 years ago by Karen Tracey

Attachment: logger.diff added

Updated patch to include doc

comment:2 Changed 6 years ago by Karen Tracey

I updated the patch to include some doc. It doesn't go into any detail about pitfalls of sending HTML email, though, nor recommend external packages in place of this built-in mechanism...two things I think were mentioned as things that should be noted in the doc, so suggestions for proper wording for that kind of stuff would be helpful (I had a hard time figuring out how to word that properly...)

comment:3 Changed 6 years ago by Russell Keith-Magee

Keywords: blocker added

We need to address this before we cut 1.3 final.

comment:4 Changed 6 years ago by Karen Tracey

Owner: Karen Tracey deleted

I'm going to have to leave shortly so removing myself as owner.

comment:5 Changed 6 years ago by Adrian Holovaty

Owner: set to Adrian Holovaty

Agreed this is something we need to do. I'll take a look at this patch and commit.

comment:6 in reply to:  5 Changed 6 years ago by Jannis Leidel

Replying to adrian:

Agreed this is something we need to do. I'll take a look at this patch and commit.

Yeah, I believe linking to http://djangopackages.com/grids/g/error-handling/ would be useful in case we try point to more complex error handling tools.

comment:7 Changed 6 years ago by Jacob

I don't think that's quite the right target for a link -- Lumberjack and slow-log don't really replace emails the way Sentry does. We probably should create an "error email replacements" grid. Better name, though, please.

comment:8 Changed 6 years ago by Adrian Holovaty

In [15849]:

Corrected email --> e-mail in topics/logging.txt. Refs #15603

comment:9 Changed 6 years ago by Adrian Holovaty

Resolution: fixed
Status: newclosed

In [15850]:

Fixed #15603 -- Changed the traceback error e-mails not to use HTML by default. It's now configurable with an 'include_html' parameter to AdminEmailHandler. Thanks, kmtracey

comment:10 Changed 5 years ago by Jacob

milestone: 1.3

Milestone 1.3 deleted

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