unreported JS exception bugs

[Gavin Sharp]

Seeing the filing of bug 895340 pushed me over the edge, because I knew we
had many other similar bugs on file about unreported JS exceptions.

I ended up filing https://bugzilla.mozilla.org/show_bug.cgi?id=895548, a
tracking bug from which I could link a bunch of other related bugs that I
found.

I think many of them are probably related to the same underlying issues,
and it would be great if someone more familiar with those issues and
related code (bholley, mrbkap, bz?) could help clean up that bug list.

It would also be useful if people who know of similar issues linked them to
that tracking bug.

These bugs often make debugging chrome JS a real pain. I realize they
aren't all necessarily easy to fix, but I bet a bit of effort could go a
long way to improving the lives of front-end developers and add-on authors.

Gavin

[Benjamin Smedberg]

On 7/18/2013 7:11 PM, Gavin Sharp wrote:
> Seeing the filing of bug 895340 pushed me over the edge, because I knew we
> had many other similar bugs on file about unreported JS exceptions.
>
> I ended up filing https://bugzilla.mozilla.org/show_bug.cgi?id=895548, a
> tracking bug from which I could link a bunch of other related bugs that I
> found.
>
> I think many of them are probably related to the same underlying issues,
> and it would be great if someone more familiar with those issues and
> related code (bholley, mrbkap, bz?) could help clean up that bug list.
>

This is all related to a bug from relatively long ago, bug 393627. In
this bug, people were complaining that in the following case, reporting
an error is wrong:

C++ (main) -> JS code A -> C++ B -> JS code C throws exception

If the C++ code B just sees the exception nsresult and immediately
rethrows to JS code A, then we don't want to report that because the JS
code might catch it.

The result of this is that if there is *any* JS code on the stack, we
don't report errors from deeper JS code. This is always the case in
XPCShell. See in particular the logic around
http://hg.mozilla.org/mozilla-central/annotate/af4e3ce8c487/js/xpconnect/src/XPCWrappedJSClass.cpp#l1001

I argued against fixing bug 415498 at the time, and because it was so
contentious, there is an "out": if you set MOZ_REPORT_ALL_JS_EXCEPTIONS
you will get the old behavior (and the possibility of some
false-positives). You can also call
nsIXPConnect.setReportAllJSExceptions to set this at runtime.

Other sordid history in this tale includes bug 415498, where the
original code was found to be buggy and was eating error reports thrown
from event listeners. My opinion is still that we should rip out most or
all of this mechanism, and replace it with an opt-in mechanism for
methods that actually do exception passthrough.

--BDS

[Boris Zbarsky]

On 7/18/13 9:27 PM, Benjamin Smedberg wrote:
> My opinion is still that we should rip out most or
> all of this mechanism, and replace it with an opt-in mechanism for
> methods that actually do exception passthrough.

For what it's worth, that's how the mechanism for calling JS from C++
via WebIDL bindings works: Exceptions in JS are reported unless the C++
caller explicitly opts in to rethrowing them. And in that case we
assert if it doesn't check whether it needs to rethrow, even if there is
no exception thrown, so the exceptions won't disappear due to
inattentiveness.

The xpidl story is a bit more complicated because it tries to make it
transparent whether you're calling C++ or JS, so you can't just put the
opt-in in the function signature like we do for WebIDL. But maybe we
could just set up a simple RAII class for doing such opt-in...

-Boris

[Bobby Holley]

On Thu, Jul 18, 2013 at 4:11 PM, Gavin Sharp <ga...@gavinsharp.com> wrote:

> Seeing the filing of bug 895340 pushed me over the edge, because I knew we
> had many other similar bugs on file about unreported JS exceptions.
>
> I ended up filing https://bugzilla.mozilla.org/show_bug.cgi?id=895548, a
> tracking bug from which I could link a bunch of other related bugs that I
> found.
>
> I think many of them are probably related to the same underlying issues,
> and it would be great if someone more familiar with those issues and
> related code (bholley, mrbkap, bz?) could help clean up that bug list.
>

The current exception handling mechanism in XPConnect is, in my assessment,
broken and fragile beyond repair. I'm incrementally simplifying a lot of
related machinery (with the end goal of removing JSContexts altogether),
after which point it will be much simpler to put together a sane and
predictable exception mechanism. We have plans for this, in another
off-list thread.

That is to say, I'm working on the big picture, and not particularly
inclined to work on specific cases of the current general brokenness in the
mean time.

bholley