Skip to content

Commit

Permalink
Bug 762556 - Error stack should contain column number. r=jorendorff
Browse files Browse the repository at this point in the history
  • Loading branch information
ConradIrwin committed Feb 19, 2014
1 parent 25ab080 commit 58ebc84
Show file tree
Hide file tree
Showing 10 changed files with 40 additions and 19 deletions.
12 changes: 8 additions & 4 deletions addon-sdk/source/lib/toolkit/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,13 +137,16 @@ var parseStack = iced(function parseStack(stack) {
if (line) {
let atIndex = line.indexOf("@");
let columnIndex = line.lastIndexOf(":");
let fileName = sourceURI(line.slice(atIndex + 1, columnIndex));
let lineNumber = parseInt(line.slice(columnIndex + 1));
let lineIndex = line.lastIndexOf(":", columnIndex - 1);
let fileName = sourceURI(line.slice(atIndex + 1, lineIndex));
let lineNumber = parseInt(line.slice(lineIndex + 1, columnIndex));
let columnNumber = parseInt(line.slice(columnIndex + 1));
let name = line.slice(0, atIndex).split("(").shift();
frames.unshift({
fileName: fileName,
name: name,
lineNumber: lineNumber
lineNumber: lineNumber,
columnNumber: columnNumber
});
}
return frames;
Expand All @@ -155,7 +158,8 @@ var serializeStack = iced(function serializeStack(frames) {
return frames.reduce(function(stack, frame) {
return frame.name + "@" +
frame.fileName + ":" +
frame.lineNumber + "\n" +
frame.lineNumber + ":" +
frame.columnNumber + "\n" +
stack;
}, "");
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ let observer = {
reflow: function (start, end) {
// Gather information about the current code path.
let path = (new Error().stack).split("\n").slice(1).map(line => {
return line.replace(/:\d+$/, "");
return line.replace(/:\d+:\d+$/, "");
}).join("|");
let pathWithLineNumbers = (new Error().stack).split("\n").slice(1).join("|");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ let observer = {
// Gather information about the current code path.
let stack = new Error().stack;
let path = stack.split("\n").slice(1).map(line => {
return line.replace(/:\d+$/, "");
return line.replace(/:\d+:\d+$/, "");
}).join("|");
let pathWithLineNumbers = (new Error().stack).split("\n").slice(1).join("|");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ function runTests()
method: "display",
code: error1,
result: error1 + openComment +
"Exception: Ouch!\n@" + scratchpad.uniqueName + ":1" + closeComment,
"Exception: Ouch!\n@" + scratchpad.uniqueName + ":1:1" + closeComment,
label: "error display output"
},
{
Expand Down Expand Up @@ -78,7 +78,7 @@ function runTests()
method: "run",
code: error1,
result: error1 + openComment +
"Exception: Ouch!\n@" + scratchpad.uniqueName + ":1" + closeComment,
"Exception: Ouch!\n@" + scratchpad.uniqueName + ":1:1" + closeComment,
label: "error run output"
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ function runTests()
method: "display",
code: error,
result: error + openComment + "Exception: Ouch!\n@" +
scratchpad.uniqueName + ":1" + closeComment,
scratchpad.uniqueName + ":1:1" + closeComment,
label: "error display output",
},
{
Expand All @@ -57,7 +57,7 @@ function runTests()
method: "run",
code: error,
result: error + openComment + "Exception: Ouch!\n@" +
scratchpad.uniqueName + ":1" + closeComment,
scratchpad.uniqueName + ":1:1" + closeComment,
label: "error run output",
},
{
Expand Down
6 changes: 3 additions & 3 deletions js/src/jit-test/tests/basic/cross-context-stack-1.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ function f() {
}
f();
assertEq(stack,
"h@@evaluate:2\n" +
"@@evaluate:4\n");
"h@@evaluate:2:1\n" +
"@@evaluate:4:2\n");

function k() {
evaluate("stack = Error().stack", {newContext: true});
}
k();
assertEq(stack, "@@evaluate:1\n");
assertEq(stack, "@@evaluate:1:1\n");
4 changes: 2 additions & 2 deletions js/src/jit-test/tests/basic/offThreadCompileScript-02.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
// Test offThreadCompileScript option handling.

offThreadCompileScript('Error()');
assertEq(!!runOffThreadScript().stack.match(/^@<string>:1\n/), true);
assertEq(!!runOffThreadScript().stack.match(/^@<string>:1:1\n/), true);

offThreadCompileScript('Error()',
{ fileName: "candelabra", lineNumber: 6502 });
assertEq(!!runOffThreadScript().stack.match(/^@candelabra:6502\n/), true);
assertEq(!!runOffThreadScript().stack.match(/^@candelabra:6502:1\n/), true);

var element = {};
offThreadCompileScript('Error()', { element: element }); // shouldn't crash
Expand Down
12 changes: 9 additions & 3 deletions js/src/jsexn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -237,9 +237,15 @@ ComputeStackString(JSContext *cx)
if (!sb.appendInflated(cfilename, strlen(cfilename)))
return nullptr;

/* Finally, : followed by the line number and a newline. */
uint32_t line = PCToLineNumber(script, i.pc());
if (!sb.append(':') || !NumberValueToStringBuffer(cx, NumberValue(line), sb) ||
uint32_t column = 0;
uint32_t line = PCToLineNumber(script, i.pc(), &column);
// Now the line number
if (!sb.append(':') || !NumberValueToStringBuffer(cx, NumberValue(line), sb))
return nullptr;

// Finally, : followed by the column number (1-based, as in other browsers)
// and a newline.
if (!sb.append(':') || !NumberValueToStringBuffer(cx, NumberValue(column + 1), sb) ||
!sb.append('\n'))
{
return nullptr;
Expand Down
10 changes: 10 additions & 0 deletions js/src/tests/js1_8_5/extensions/column-numbers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
actual = 'No Error';
expected = /column-numbers\.js:4:5/;
try {
throw new Error("test");
}
catch(ex) {
actual = ex.stack;
print('Caught exception ' + ex.stack);
}
reportMatch(expected, actual, 'column number present');
3 changes: 2 additions & 1 deletion testing/marionette/marionette-common.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ this.createStackMessage = function createStackMessage(error, fnName, pythonFile,
let trace, msg;
if (typeof(error) == "object" && 'name' in error && 'stack' in error) {
let stack = error.stack.split("\n");
let line = stack[0].substr(stack[0].lastIndexOf(':') + 1);
let match = stack[0].match(/:(\d+):\d+$/);
let line = match ? parseInt(match[1]) : 0;
msg = error.name + ('message' in error ? ": " + error.message : "");
trace = python_stack +
"\ninline javascript, line " + line +
Expand Down

0 comments on commit 58ebc84

Please sign in to comment.