Opened 6 years ago

Closed 6 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: UI/UX:

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 6 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 6 years ago by Dan Carroll

Needs documentation: unset
Needs tests: unset
Owner: changed from nobody to Dan Carroll
Patch needs improvement: unset
Status: newassigned

comment:2 Changed 6 years ago by Dan Carroll

Component: UncategorizedCore framework
Keywords: common middleware added

Changed 6 years ago by Dan Carroll

Attachment: 15187.diff added

comment:3 Changed 6 years ago by Dan Carroll

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

comment:4 Changed 6 years ago by Dan Carroll

Has patch: set

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

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 Changed 6 years ago by Russell Keith-Magee

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