Closed Bug 642639 Opened 13 years ago Closed 12 years ago

errUtils.js' logException should dump exceptions via Components.utils.reportError too for fancy mozmill logging

Categories

(MailNews Core :: Testing Infrastructure, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 18.0

People

(Reporter: asuth, Assigned: aceman)

References

Details

Attachments

(1 file, 1 obsolete file)

We are using logException all over the place.  logException is a noble and great thing, but it only dumps via stdout.  It should also log exceptions via Cu.reportError.  Maybe always, or maybe only if we're not re-throwing.

While my greatest concern is for the fancy mozmill logging, we are using it a lot, and arguably when QA people ask "anything in the error console?" and these failures are hidden from that, we are losing out.

There are other ways we could hook this up to fancy mozmill logging (and we could get fancy about it), but I think a good first step is just the Cu.reportError thing.

The argument against doing this is that the exceptions may be harmless and could needlessly concern users.  The counter-argument is that a harmless error should be explicitly ignored in the exception handler if that is the case; the fact that the developer wanted to see it dumped suggested they were concerned (or didn't know that logException wouldn't send things to the error console.)
Asuth are you working on this?

The logException function in used in many places outside tests in main code. Let's try it and see what errors show up suddenly.
I am not working on this.  It would be great if you want to pursue this!
Assignee: bugmail → nobody
Status: ASSIGNED → NEW
Assignee: nobody → acelists
Attached patch patch (obsolete) — Splinter Review
Attachment #666033 - Flags: review?(mbanner)
Status: NEW → ASSIGNED
Attachment #666033 - Flags: review?(mbanner) → review?(bugmail)
Comment on attachment 666033 [details] [diff] [review]
patch

Review of attachment 666033 [details] [diff] [review]:
-----------------------------------------------------------------

Chronologically speaking, I think it makes sense to have the message reported first, so that when skimming one will see the message and then the specific exception.  However, either way is a major improvement!  Thanks for doing this.
Attachment #666033 - Flags: review?(bugmail) → review+
Attached patch patch v2Splinter Review
Attachment #666033 - Attachment is obsolete: true
Attachment #667583 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/ac1839e646e5
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite-
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 18.0
Blocks: 1015774
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: