temporary mitigation of bug 551604 #528

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants
@dcrewi
Contributor

dcrewi commented Aug 20, 2012

The file name and line number are included the SyntaxError object thrown from require(). The problem is that the information isn't included in the stack trace nor in the error message. This commit appends that info to the error message. It's sort of a hack, but it gets the job done for now and will hold until something better is implemented.

@ochameau

View changes

packages/api-utils/lib/loader.js
+ try {
+ return source ? Cu.evalInSandbox(source, sandbox, version, uri, line)
+ : loadSubScript(uri, sandbox, encoding);
+ } catch (exc if exc instanceof SyntaxError) {

This comment has been minimized.

@ochameau

ochameau Aug 23, 2012

Member

Hum that's weird, it shouldn't work and it doesn't work for me.
Because of cross-sandboxes/compartments values, exc is never an instance of SyntaxError.
It is an instance of sandbox's SyntaxError.
We should do something like this instead:
} catch (exc if exc.name === "SyntaxError") {

@ochameau

ochameau Aug 23, 2012

Member

Hum that's weird, it shouldn't work and it doesn't work for me.
Because of cross-sandboxes/compartments values, exc is never an instance of SyntaxError.
It is an instance of sandbox's SyntaxError.
We should do something like this instead:
} catch (exc if exc.name === "SyntaxError") {

This comment has been minimized.

@dcrewi

dcrewi Aug 24, 2012

Contributor

I could swear it worked earlier. I judged instanceof to be more proper than checking name, but you're right that it doesn't work.

@dcrewi

dcrewi Aug 24, 2012

Contributor

I could swear it worked earlier. I judged instanceof to be more proper than checking name, but you're right that it doesn't work.

@KWierso

This comment has been minimized.

Show comment
Hide comment
@KWierso

KWierso Dec 13, 2012

Member

Looks like a fix for 551604 already landed.

Member

KWierso commented Dec 13, 2012

Looks like a fix for 551604 already landed.

@KWierso KWierso closed this Dec 13, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment