Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Improve script error reporting #1396

Closed
arantius opened this Issue · 5 comments

2 participants

@arantius
Collaborator

This issue is to track a particular idea I have. If the idea turns out to be no good, then there will be no fix/change for it.

Via any mechanism (likely a JS module), make a file with as little code as possible, all on one line, which actually calls evalInSandbox(). Given that the file has exactly one line, we can make more/better assumptions about the meaning of any line numbers we might get back from thrown/caught errors, and thus hopefully report them better and with less special case handling code.

@arantius arantius referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@arantius arantius referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@arantius arantius referenced this issue from a commit in arantius/greasemonkey
@arantius arantius First-line-eval implementation.
Partially works.  The real problem is async callbacks.  Maybe doc'ed as it is, this is the best we can do?

Refs #1396
cc2d356
@arantius arantius referenced this issue from a commit in arantius/greasemonkey
@arantius arantius More for someone to read, if they can follow the error.
Fixes #1396
d0aec33
@arantius arantius closed this in d0aec33
@sizzlemctwizzle

Couldn't we use the last two arguments for Components.utils.evalInSandbox detailed here?

So the call in modules/runScript.js would look something like:

Components.utils.evalInSandbox(code, sandbox, maxJSVersion, filename, 1);

We could loop through all the @requires and eval each one individually, that way those filenames and line numbers are preserved as well. Then finally we would eval the script. Is there some reason this wouldn't work?

@arantius
Collaborator

Wow, how did I miss that?

Requires are the issue. Today, they're all evaluated inside the same anonymous function and thus cannot be broken up. We tried taking it out in the past and it causes unpredictable issues.

@arantius arantius reopened this
@sizzlemctwizzle

Today, they're all evaluated inside the same anonymous function and thus cannot be broken up.
We could execute them all inside separate anonymous functions and bind their this to a single shared object using call. Of course this would mean we would have to decide that requires and the main script couldn't share local variables. I'd be willing to give this up for very accurate error reporting myself.

Or we could try executing the script and requires without an anonymous function and if we catch one of the tell-tail errors from one of them we could try re-executing it inside an anonymous function. I believe this is the method Scriptish uses.

Either that or we can just keep doing what were doing and use the fifth argument on evalInSandbox to force the line count to start at 1.

@arantius
Collaborator
@arantius
Collaborator

Turned out to be a bit more involved than I first hoped, especially considering the scope issues with requires. So I added #1404 to do this bit, and for now we'll leave this issue solved as-is.

@arantius arantius closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.