Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#14972 closed (fixed)

AdminEmailHandler breaks when report is missing stack trace information

Reported by: jamstooks Owned by: nobody
Component: Core (Other) Version: 1.3-alpha
Severity: Keywords: blocker
Cc: ben.stookey@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: UI/UX:

Description

If a log message doesn't have stack trace info then record.exc_info is () and ExceptionReporter __init__() breaks. I assume this is because *exc_info should contain the keyword arguments, but is assigned to ():

# django/utils/log.py line # 89
exc_info = ()
...
reporter = ExceptionReporter(request, is_email=True, *exc_info)
# ERROR: __init__() takes at least 5 non-keyword arguments (2 given)

I'd be happy to build a patch for this in some way (if this is indeed a bug), but I'm not sure if the fix belongs in AdminEmailHandler or ExceptionReporter.

Attachments (2)

adminemailhandler.patch (1.2 KB) - added by jamstooks 4 years ago.
django-14972-exceptionreporter.patch (5.6 KB) - added by bpeschier 4 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 4 years ago by russellm

  • milestone set to 1.3
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

AdmineEmailHandler already has checks for whether the report has exc_info, so the fix should be in ExceptionReporter.

Since this is a new 1.3 feature, this is a 1.3 blocker.

comment:2 Changed 4 years ago by russellm

  • Keywords blocker added

comment:3 Changed 4 years ago by jamstooks

  • Cc ben.stookey@… added
  • Has patch set
  • Summary changed from AdminEmailHandler breaks when logger is missing stack trace information to AdminEmailHandler breaks when report is missing stack trace information

I'm actually thinking that this fix belongs in AdminEmailHandler because there really isn't a need to call ExceptionReporter if there isn't any exception data. Here's a first go at a patch.

When there is no exception data, instead of using ExceptionReporter to create the html_message, I set that to None and use the records msg property to populate the message.

I considered putting the msg text in the subject at first, but since it could be long (if triggered by a logger) I thought it better to put in the body of the email.

Changed 4 years ago by jamstooks

comment:4 Changed 4 years ago by jamstooks

  • Needs tests set

comment:5 Changed 4 years ago by russellm

  • Patch needs improvement set

@jamstooks: It's not quite as simple as you make out. Using your proposed patch users with HTML-enabled mail won't get any of the request dump information. Which also may not exist.

The patch you've proposed also provides no HTML styling for the HTML mail, which kinda defeats the point of having HTML mail in the first place.

Changed 4 years ago by bpeschier

comment:6 Changed 4 years ago by bpeschier

I took a look at the ExceptionReporter and made it deal with a lack of exception or request data. It will display a small message that the data was not supplied to not misinform people.

comment:7 Changed 4 years ago by russellm

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

In [15383]:

Fixed #14972 -- Ensure that the HTML email logger always produces useful output, regardless of whether it has been given an exception or a request. Thanks to jamstooks for the report, and bpeschier for the initial patch.

comment:8 Changed 4 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

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