Closed Bug 1015774 Opened 5 years ago Closed 5 years ago

errUtils.js::logException shows the same exception to Error console twice

Categories

(MailNews Core :: Backend, defect, trivial)

defect
Not set
trivial

Tracking

(thunderbird34 fixed)

RESOLVED FIXED
Thunderbird 34.0
Tracking Status
thunderbird34 --- fixed

People

(Reporter: aceman, Assigned: aceman)

References

Details

(Keywords: polish)

Attachments

(1 file)

Bug 714733 added Components.utils.reportError() to 
errUtils.js::dumpException().

Bug 642639 added Components.utils.reportError() to errUtils.js::logException() which also calls dumpException so the same message is output twice. This bug was filed sooner bug when fixing it I didn't notice 714733 already inserted the reportError() into dumpException().

So what can we do now?

Florian, do you remember why you needed to insert the reportError? Maybe the IM code should use logException instead of dumpException? There should probably be proper comments on when to use which function.
(In reply to :aceman from comment #0)

> Florian, do you remember why you needed to insert the reportError?

Not at all, but I guess "dumpException" was the logical place to add a dump() call.

> Maybe the
> IM code should use logException instead of dumpException? There should
> probably be proper comments on when to use which function.

The only usage of this stuff as part of the IM patch is in mail/components/im/content/imsearch.xml, and the function called there is actually logException, not dumpException.
It appears to me that dump* functions are used for lowlevel reporting (into a log or console) and logException and reportError are used to also show it to the user in Error console.

So are you OK if in this bug I remove the reportError from dumpException? I will leave the dump() in there.
Assignee: nobody → acelists
Status: NEW → ASSIGNED
(In reply to :aceman from comment #2)

> So are you OK if in this bug I remove the reportError from dumpException? I
> will leave the dump() in there.

Assuming the question is directed to me, sure. I have no emotional attachment to this reportError call I added so long ago that I can't even remember. Please do whatever makes the most sense to you :-).
Yes, thanks. If you call logException from IM, nothing will change for you. You should still get it reported to Error console via at least one reportError.
Attached patch patchSplinter Review
Attachment #8428855 - Flags: review?(neil)
Attachment #8428855 - Flags: review?(neil) → review+
Keywords: checkin-needed
I landed this the other day, as I needed to give the tree a click:

https://hg.mozilla.org/comm-central/rev/3045e237ddd7
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 34.0
You need to log in before you can comment on or make changes to this bug.