Opened 6 years ago

Closed 5 years ago

Last modified 4 years ago

#10863 closed Uncategorized (fixed)

Email full stack traces like in the debug error pages

Reported by: boxed Owned by: brodie
Component: Core (Other) Version: master
Severity: Normal Keywords: stack, errors, email
Cc: boxed@…, aom, david, obeattie 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 6 years ago.
10863.2.diff (6.4 KB) - added by robhudson 5 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 5 years ago.
10863-2-html-error-email.diff (10.4 KB) - added by brodie 5 years ago.

Download all attachments as: .zip

Change History (21)

Changed 6 years ago by boxed

comment:1 Changed 6 years ago by boxed

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Version changed from 1.0 to SVN

comment:2 Changed 6 years ago by Alex

  • Triage Stage changed from Unreviewed to Design decision needed

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

comment:3 Changed 6 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 6 years ago by aom

  • Cc aom 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 6 years ago by david

  • Cc david added

comment:6 Changed 6 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 6 years ago by russellm

  • Needs documentation set
  • Needs tests set
  • Patch needs improvement set
  • Triage Stage changed from Design decision needed to Accepted

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 5 years ago by obeattie

  • Cc obeattie added

Changed 5 years ago by robhudson

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

comment:9 Changed 5 years ago by robhudson

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 5 years ago by robhudson

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 5 years ago by russellm

@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 5 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 5 years ago by brodie

Changed 5 years ago by brodie

comment:13 Changed 5 years ago by brodie

  • milestone set to 1.3
  • Needs tests unset
  • Owner changed from nobody to brodie
  • 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 5 years ago by russellm

  • Resolution set to fixed
  • Status changed from new to closed

(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 4 years ago by Tuttle

  • Easy pickings unset
  • Severity set to Normal
  • Type set to 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 4 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

comment:17 Changed 4 years ago by foxwhisper

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