Last Comment Bug 642639 - errUtils.js' logException should dump exceptions via Components.utils.reportError too for fancy mozmill logging
: errUtils.js' logException should dump exceptions via Components.utils.reportE...
Product: MailNews Core
Classification: Components
Component: Testing Infrastructure (show other bugs)
: Trunk
: All All
-- normal (vote)
: Thunderbird 18.0
Assigned To: :aceman
Depends on:
Blocks: 1015774
  Show dependency treegraph
Reported: 2011-03-17 16:54 PDT by Andrew Sutherland [:asuth]
Modified: 2014-05-25 14:39 PDT (History)
7 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

patch (1.30 KB, patch)
2012-09-28 13:10 PDT, :aceman
bugmail: review+
Details | Diff | Splinter Review
patch v2 (1.30 KB, patch)
2012-10-03 11:57 PDT, :aceman
acelists: review+
Details | Diff | Splinter Review

Description User image Andrew Sutherland [:asuth] 2011-03-17 16:54:41 PDT
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.)
Comment 1 User image :aceman 2012-09-24 03:12:03 PDT
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.
Comment 2 User image Andrew Sutherland [:asuth] 2012-09-24 21:42:09 PDT
I am not working on this.  It would be great if you want to pursue this!
Comment 3 User image :aceman 2012-09-28 13:10:42 PDT
Created attachment 666033 [details] [diff] [review]
Comment 4 User image Andrew Sutherland [:asuth] 2012-10-03 11:31:44 PDT
Comment on attachment 666033 [details] [diff] [review]

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.
Comment 5 User image :aceman 2012-10-03 11:57:07 PDT
Created attachment 667583 [details] [diff] [review]
patch v2
Comment 6 User image Ryan VanderMeulen [:RyanVM] 2012-10-03 14:59:55 PDT

Note You need to log in before you can comment on or make changes to this bug.