307985 - Feature request: allow me to catch all exceptions inside evalInSandbox, not just parse-time ones

Status: RESOLVED INACTIVE

(Core :: XPConnect, enhancement)

Description

[Aaron Boodman]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4

I'd like to be able to this:

function handleSandboxException(e) {
}

var sandbox = new Components.utils.Sandbox(location.href);
sandbox.registerExceptionHandler(handleSandboxException);
Components.utils.evalInSandbox("window.setTimeout('throw new Error()', 1000)",
sandbox);

Reproducible: Always

Steps to Reproduce:
1. Run attachment
2 [review]. Note that error is not caught by catch clause, since it didn't occur at
parse-time
3. Appreciate the suckitude that Greasemonkey cannot report script errors well
when they occur in event handlers or timers which get called after the initial
parse of the script.

Actual Results:  
Exception not caught, shows up in JS console: 

Error: foobar
Source File: file:///C:/Documents%20and%20Settings/aa/Desktop/evalInSandbox.html
Line: 42

Expected Results:  
I should have been able to write code like in the details section above,
registering an exception handler to catch all exceptions in the eval'd code, no
matter when it occurs.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Aaron, did you mean to attach a testcase?

Comment 2

[Aaron Boodman]

Attachment: demonstration of problem

Comment 3

[Aaron Boodman]

Attachment: One option for how this feature could work

Allow callers of evalInSandbox to register and error handler on the sandbox.

Comment 4

[Aaron Boodman]

Whoops, sorry. Fixed now.

Comment 5

[Boris Zbarsky [:bzbarsky]]

This might be doable if we can get the error reporter func to call this other function somehow....  One problem is that the error reporter func doesn't get access to the sandbox, just to the JSContext.

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Except that the sandbox object should be the global object on the given cx, no?

Comment 7

[Boris Zbarsky [:bzbarsky]]

Ah, true!  So do we want to do something like this?

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

I don't see why not. It seems like a zero-cost feature that will make evalInSandbox more useful for consumers that want this.

Comment 9

[Boris Zbarsky [:bzbarsky]]

So if we do this, it seems to me that we need to pass the handler to the sandbox constructor and store it in a slot or something.  Storing on the JSObject in a JS property is no good, since the sandboxed script can mess with that, right?

Comment 10

[Boris Zbarsky [:bzbarsky]]

So I looked into doing this.... and the problem is that of course the script is running on the JSContext of the window involved (which I presume in your case would be the window that the script came from, if you're willing to give the sandbox access to it).  So there's no sane way to intercept the error -- the sandbox can't just munge the error reporter on that context or anything...

Comment 11

[Mike Shaver (:shaver emeritus)]

The sandbox should be able to substitute its own reporter and then restore it after evalInSandbox returns, safely and easily.

We can store it in a private slot, though I don't know that we want to expose it as a method on the sandbox object itself.  Would an extra parameter to evalInSandbox and/or the Sandbox constructor work OK?

Comment 12

[Boris Zbarsky [:bzbarsky]]

Mike, the whole point of this bug is that the reporter wants to catch exceptions in code that runs _after_ evalInSandbox returns.  See the setTimeout call in the testcase.  There is no problem with exceptions under evalInSandbox -- you can just try/catch those.

Comment 13

[Mike Shaver (:shaver emeritus)]

Oh, sorry, I was skimming the latter parts of the bug only, and the summary.

("throw" doesn't ever occur at parse time: only syntax errors are really caught and thrown then, which is partly why I was confused.)

I'm not sure that this is something that we can really do, even without the context sharing issue.  If I set the "onclick" attribute of an element from inside evalInSandbox, there's no way for the eventual compiler of the code to know that it came from within an evalInSandbox originally.

I do wonder what blake had in mind in comment #8, now, though!

Comment 14

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to comment #13)
> I do wonder what blake had in mind in comment #8, now, though!

Hrm, rereading this bug, so do I! I wonder if I misread the bug before... I think it's possible, but I'm not sure of the details now.

Comment 15

[Aaron Boodman]

I know it's been awhile, but I was just going through my old bugs...

In response to comment #13:

If you set an onclick attribute to a string that throws an error, I think that is OK to not route back to this proposed handler. It makes sense to me that in that case, you are actually modifying the document, and so any errors you create will be modified by the document's own error handling.

However, if I add a click listener with a function reference to code in the sandbox, and that function throws an error, then the sandbox handler *should* catch it. 

In code:

// does not call sandbox's exception handler
document.body.setAttribute("onclick", "throw new Error('boo!')");

// calls sandbox's exception handler
document.body.addEventListener("click", function() { throw new Error('boo!') }, false);

Does that make this feature request any easier to implement?

Comment 16

[Boris Zbarsky [:bzbarsky]]

No.  In both cases, the sandbox is long gone by the time the click handler executes.

Comment 17

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.