24688 - runtime errors in wrapped JS are not made obvious

Status: RESOLVED FIXED

(Core :: XPConnect, defect, P3)

Description

[John Bandhauer]

In calling wrapped JS objects xpconnect captures JS errors, builds an exception, 
and returns an xpcom error code. One can programatically get the most recent 
exception (with error report object) on the current thread using 
nsIXPConnect::GetPendingException(). It is not currrently printed. I can fix 
that.

We are going to need a global error reporter service. printf is not going to cut 
it in the long run. The DOM error reporter only works on DOM contexts. I assume 
that there is a bug about sending its output to a window. The JS loader and 
xpconnect should use an error reporter service to also get the data to a window 
console rather than the OS console that is not always going to be there.

I'm thinking about the short term here... In debug builds I can add code to 
printf errors and uncaught exceptions from calls to wrapped JS objects. This is 
not a production solution.

Comment 1

[John Bandhauer]

I added the (debug build only) printf. We need to do better.

Comment 2

[John Bandhauer]

mccabe offered to add code to send these errors to his new console service.

Comment 3

[Mike McCabe]

Attachment: diffs to make xpcwrappedjsclass error reporter log to the console

Comment 4

[Mike McCabe]

Marking 47013 as a dependency, and nominating nsbeta3.  Fix in hand.

This fix adds code to an XPConnect error reporter to register that error with
the JS Console.  So far as I can see, it's a clean addition.  Without this
patch, XUL/js developers won't see exceptions in JS code called from C++ unless
they're running debug builds.  Life would be better if this were not so.

Comment 5

[Patrick C. Beard]

This will be crucial for JS developers. Marking beta3+.

Comment 6

[Mike McCabe]

*** Bug 13187 has been marked as a duplicate of this bug. ***

Comment 7

[Mike McCabe]

Added 13187 as dup - possible extra issue to check is what JS Component errors
during registration time look like.

Comment 8

[Mike McCabe]

I'm making another stab at this, logging the exception in a somewhat different
place, at John's suggestion.

I've also subsumed xpcJSErrorReport.cpp into nsScriptError.cpp.

Diff attached

Comment 9

[Mike McCabe]

Attachment: log exceptions to the console and turn xpcJSErrorReport into nsScriptError.

Comment 10

[Mike McCabe]

Fix checked in.

Comment 11

[Mike McCabe]

Ach, I think this is causing trouble.

I'm seeing an unfortunate behavior in the JavaScript console. Whenever the JS
console tries to QI (from JavaScript) one of the console messages to an
nsIScriptError, it might throw an exception (which is OK) but - due to the new
code, this exception logs another error, which ends up executing JS console
code, which tries to do a QI...

We need to short circuit this somewhere, or have a way to learn something more
about whether an exception will be uncaught.  Ideas requested.

Mike

Comment 12

[John Bandhauer]

Yuk!

The JS console code could protect itself from re-entrance with a flag.

We could consider having the code in xpcwrappedjsclass only log a console 
message on JS errors and not on throws from the JS code. That would sort of 
suck.

Comment 13

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

I guess I'm missing something here -- are we really reporting caught-by-JS
exceptions as errors?  That would seem a little wrong -- there are lots of
correct programs that catch errors as part of interaction with their
environment, and they shouldn't be filling the error log.

If we're only logging uncaught exceptions, though, why not just have the JS
Console catch the possible exception from QI?