Opened 13 years ago

Closed 13 years ago

#15187 closed (fixed)

CommonMiddleware should only send broken link emails if DEBUG is False

Reported by: Dan Carroll Owned by: Dan Carroll
Component: Core (Other) Version: 1.2
Severity: Keywords: common middleware
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

According to the documentation on error reporting (see here: http://docs.djangoproject.com/en/dev/howto/error-reporting/), broken link emails are only sent if the following conditions are true (and CommonMiddleware is enabled):

  • DEBUG is False
  • SEND_BROKEN_LINK_EMAILS is True

Right now, CommonMiddleware.py only checks the second condition:

if settings.SEND_BROKEN_LINK_EMAILS:
    ...

In this case, I believe the best fix is to keep Django in line with the documentation, since broken link emails are unnecessary in an environment where DEBUG = True. Here is a quick look at the fix:

if settings.SEND_BROKEN_LINK_EMAILS and not settings.DEBUG:
    ...

Attachments (1)

15187.diff (686 bytes ) - added by Dan Carroll 13 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 by Dan Carroll, 13 years ago

Owner: changed from nobody to Dan Carroll
Status: newassigned

comment:2 by Dan Carroll, 13 years ago

Component: UncategorizedCore framework
Keywords: common middleware added

by Dan Carroll, 13 years ago

Attachment: 15187.diff added

comment:3 by Dan Carroll, 13 years ago

Patch added, so fix is ready. No test was added, since this should be very low risk and low impact.

comment:4 by Dan Carroll, 13 years ago

Has patch: set

comment:5 by Russell Keith-Magee, 13 years ago

Triage Stage: UnreviewedReady for checkin

Low impact isn't really the measure here -- regardless of impact, every change *could* regress behavior, so it should be tested.

However, in this case, the behavior is something that is difficult to accurately test for regression, because debug is turned off for testing purposes. *That* is a good reason to accept the patch without a test.

comment:6 by Russell Keith-Magee, 13 years ago

Resolution: fixed
Status: assignedclosed

(In [15363]) Fixed #15187 -- Ensure that missing page emails aren't sent when running under debug. Thanks to Dan Carroll for the report and patch.

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