Opened 13 years ago

Closed 13 years ago

Last modified 12 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: no UI/UX: no

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 13 years ago.
django-14972-exceptionreporter.patch (5.6 KB ) - added by Bas Peschier 13 years ago.

Download all attachments as: .zip

Change History (10)

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

milestone: 1.3
Triage Stage: UnreviewedAccepted

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 by Russell Keith-Magee, 13 years ago

Keywords: blocker added

comment:3 by jamstooks, 13 years ago

Cc: ben.stookey@… added
Has patch: set
Summary: AdminEmailHandler breaks when logger is missing stack trace informationAdminEmailHandler 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.

by jamstooks, 13 years ago

Attachment: adminemailhandler.patch added

comment:4 by jamstooks, 13 years ago

Needs tests: set

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

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.

by Bas Peschier, 13 years ago

comment:6 by Bas Peschier, 13 years ago

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 by Russell Keith-Magee, 13 years ago

Resolution: fixed
Status: newclosed

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 by Jacob, 12 years ago

milestone: 1.3

Milestone 1.3 deleted

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