Opened 8 years ago

Closed 6 years ago

Last modified 5 years ago

#10863 closed Uncategorized (fixed)

Email full stack traces like in the debug error pages

Reported by: boxed Owned by: Brodie Rao
Component: Core (Other) Version: master
Severity: Normal Keywords: stack, errors, email
Cc: boxed@…, Mika Marttila, David Larlet, Oliver Beattie Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The emails you get when you turn off debug mode are significantly less helpful than the nice html you get in the browser when the debug mode is on. I suggest simply sending an html mail with exactly the same stuff as in the debug page.

Attachments (4)

10863.diff (2.5 KB) - added by boxed 8 years ago.
10863.2.diff (6.4 KB) - added by Rob Hudson 6 years ago.
Updated patch to apply against trunk, added docs and tests, and API parity
10863-1-error-formatting.diff (3.5 KB) - added by Brodie Rao 6 years ago.
10863-2-html-error-email.diff (10.4 KB) - added by Brodie Rao 6 years ago.

Download all attachments as: .zip

Change History (21)

Changed 8 years ago by boxed

Attachment: 10863.diff added

comment:1 Changed 8 years ago by boxed

Version: 1.0SVN

comment:2 Changed 8 years ago by Alex Gaynor

Triage Stage: UnreviewedDesign decision needed

Perhaps we could send a multipart email so people who want the plaintext tracebacks can still have them.

comment:3 Changed 8 years ago by boxed

Cc: boxed@… added

That's what the patch does actually. The plain text part of the email is unchanged.

comment:4 Changed 7 years ago by Mika Marttila

Cc: Mika Marttila added
Keywords: stack errors email added

Thanks for the patch. I'll see if I get it into our staging or even production server.

comment:5 Changed 7 years ago by David Larlet

Cc: David Larlet added

comment:6 Changed 7 years ago by boxed

Is something happening with this issue? It's rather annoying to have to patch all my installations because this isn't in trunk.

comment:7 Changed 7 years ago by Russell Keith-Magee

Needs documentation: set
Needs tests: set
Patch needs improvement: set
Triage Stage: Design decision neededAccepted

There are many reasons why this ticket hasn't progressed:

  • The ticket hasn't been proposed for inclusion in milestone 1.2
  • The patch doesn't apply to the current trunk.
  • Your patch doesn't have documentation or tests.
  • Your patch doesn't address an obvious API parity issue - ensuring that mail_managers should also have a html_message argument.
  • Your patch doesn't address the fact that the rendering of complex HTML emails is notoriously unreliable. Sending an error email to Gmail, for example, results in almost an unreadable message.

I completely agree that there is plenty of scope to improve the error emails - but this patch isn't there yet (and given the timeframe before the 1.2 feature freeze, I'm going to guess it won't make the cut).

The logging proposal (#12012) is also of interest here. Error handling is currently hard-coded to call mail_admins; with a fully integrated logging backend, it would be possible to configure an SMTP handler to send emails on certain error conditions. Logging won't make 1.2 either, but it's worth keeping this in mind because I intend to make logging a personal priority for 1.3.

comment:8 Changed 7 years ago by Oliver Beattie

Cc: Oliver Beattie added

Changed 6 years ago by Rob Hudson

Attachment: 10863.2.diff added

Updated patch to apply against trunk, added docs and tests, and API parity

comment:9 Changed 6 years ago by Rob Hudson

I've updated the original patch for this to cover most of Russell's points above. The patch currently applies against trunk, I added docs and doctests, the API matches mail_managers. I tested the HTML in Gmail and it actually looks decent -- the toggle "Local vars" don't work, but it's legible and relatively clean looking.

However, after all this, I think this might be a DDN. I can imagine some people prefer "text/plain" error e-mails and some people may prefer a full "text/html" error e-mail, and how do you accomodate both? Perhaps a solution would be to include more details in the "text/plain" error e-mails to get closer to the detail that the HTML debug pages have?

comment:10 Changed 6 years ago by Rob Hudson

PS: Also, if the goal is to get the full traceback and error debug pages as you'd see when DEBUG=True, this is mostly a solved problem in 3rd party space with things like django-sentry and Arecibo.

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

@robhudson; as long as both text/plain and text/html are transmitted, and text/plain is transmitted first, HTML/Text mail support should be able to be handled by any decent mail client. However, it would be nice to see the text support improved so that it contains the same data as the html mail.

There's also some interesting crossover with the #12012 logging proposal that is targeted for 1.3.

comment:12 Changed 6 years ago by cope360

Replying to robhudson:

I've updated the original patch for this to cover most of Russell's points above. [...]

Rob, I see an error in your patch at line 104 of /django/core/mail/__init__.py. settings.ADMINS is used instead of settings.MANAGERS in mail_managers.

Thanks for the patch.

Changed 6 years ago by Brodie Rao

Changed 6 years ago by Brodie Rao

comment:13 Changed 6 years ago by Brodie Rao

milestone: 1.3
Needs tests: unset
Owner: changed from nobody to Brodie Rao
Patch needs improvement: unset

I've attached two patches for this feature.

The first patch cleans up the debug exception template so that pretty printed values retain their whitespace. The second patch is based on boxed's and robhudson's work but updated for Django 1.3 and with some minor improvements.

AdminEmailHandler has been updated to send out HTML emails, and ExceptionReporter has grown a new is_email keyword argument. If is_email is True, JS and Pastebin-related code is omitted from the template. If request is None, request-related info is also omitted.

I've included robhudson's doc updates, but we should probably also update the AdminEmailHandler section of docs/topics/logging.txt to mention this new behavior.

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

Resolution: fixed
Status: newclosed

(In [14844]) Fixed #10863 -- Added HTML support to mail_managers() and mail_admins(), and used this to provide more and prettier detail in error emails. Thanks to boxed for the suggestion, and to Rob Hudson and Brodie Rao for their work on the patch.

comment:15 Changed 5 years ago by Vlada Macek

Easy pickings: unset
Severity: Normal
Type: Uncategorized
UI/UX: unset

Later disabled by default for safety reasons. Related ticket: #15603

Personally I enable the HTML multipart like this:

LOGGING['handlers']['mail_admins']['include_html'] = True

Only enable if you know what you're doing (when you have safe mail path where unauthorized people can't read sensitive content).

comment:16 Changed 5 years ago by Jacob

milestone: 1.3

Milestone 1.3 deleted

comment:17 Changed 5 years ago by Cal Leeming

Guys - thought I should throw something in. We have been using middleware combined with ReportBug() (see http://djangosnippets.org/snippets/2191/ ) which we wrote last year.

However, we had an issue where someone was able to DoS the server by triggering thousands of large debug emails, which flooded the mail server, and kept connections hanging.

As such - we implemented flood control via means of memcache (using cache backend).

I would *HIGHLY* suggest that this patch should include something similar before it is included in the core. (we can provide if necessary??)

But in all fairness, django debug emails should have this protection anyway - despite whether it is html or plain text (should this be raised as a separate ticket??).

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