Re: unreported JS exception bugs

2013-07-18 Thread Bobby Holley
On Thu, Jul 18, 2013 at 4: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.
>

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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: unreported JS exception bugs

2013-07-18 Thread 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

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: unreported JS exception bugs

2013-07-18 Thread 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

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


unreported JS exception bugs

2013-07-18 Thread 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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform