Code

Opened 3 years ago

Closed 3 years ago

#15187 closed (fixed)

CommonMiddleware should only send broken link emails if DEBUG is False

Reported by: dancarroll Owned by: dancarroll
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 dancarroll 3 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 3 years ago by dancarroll

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to dancarroll
  • Patch needs improvement unset
  • Status changed from new to assigned

comment:2 Changed 3 years ago by dancarroll

  • Component changed from Uncategorized to Core framework
  • Keywords common middleware added

Changed 3 years ago by dancarroll

comment:3 Changed 3 years ago by dancarroll

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

comment:4 Changed 3 years ago by dancarroll

  • Has patch set

comment:5 Changed 3 years ago by russellm

  • Triage Stage changed from Unreviewed to Ready 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 3 years ago by russellm

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

(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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.