Skip to content

Commit

Permalink
chakrashim: bug fixes for Error APIs
Browse files Browse the repository at this point in the history
1. We parse error stack in chakra_shim to make it simliar to that
of v8. However we were missing the case if stack messages has
`\n`. Fixed it.
Fixes: nodejs#78

2. Fixed Error.captureStackTrace

Since `chakra_shim.js` that patches `Error.captureStackTrace` runs
under strict mode, it was not able to get caller information. Having
caller information is much reliable in captureStackTrace than
matching using `function.name`. Removed `use strict` and disabled
eslint rule for `use strict`.
This helped in matching caller exactly regardless of name of form
`o.p.q` in `Error.captureStackTrace`.

Fixes: nodejs#75

3. Added missing functions on StackFrame

Added following missing methods on StackFrame:
* getFunction
* getMethodName
* getTypeName
* isConstructor
* isToplevel
* isNative

Implemented `getFunction`.

`getFunction` currently works if `.stack` is called from same
callsite where `new Error()` or `Error.captureStackTrace()` was
called. However currently we don't record and store the callstack
when these functions were called and so we lose the information
of `.caller` when called later while populating `e.stack`.

If we save the callstack, we would be holding references to all the
functions in the callstack which doesn't seem right.

I have added a TODO to solve this, but wanted to send this out
so we get better coverage on node compatibility.

Fixes : nodejs#70

PR-URL: nodejs/node-chakracore#85
Reviewed-By: Jianchun Xu <Jianchun.Xu@microsoft.com>
  • Loading branch information
kunalspathak committed Jun 28, 2016
1 parent fa65467 commit 32b85f9
Showing 1 changed file with 69 additions and 26 deletions.
95 changes: 69 additions & 26 deletions deps/chakrashim/lib/chakra_shim.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
// IN THE SOFTWARE.
'use strict';

/* eslint-disable strict */
(function(keepAlive) {
// Save original builtIns
var
Expand All @@ -42,13 +42,29 @@
var global = this;

// Simulate V8 JavaScript stack trace API
function StackFrame(funcName, fileName, lineNumber, columnNumber) {
function StackFrame(func, funcName, fileName, lineNumber, columnNumber) {
this.column = columnNumber;
this.lineNumber = lineNumber;
this.scriptName = fileName;
this.functionName = funcName;
this.function = func;
}

StackFrame.prototype.getFunction = function() {
// TODO: Fix if .stack is called from different callsite
// from where Error() or Error.captureStackTrace was called
return this.function;
};

StackFrame.prototype.getTypeName = function() {
//TODO : Fix this
return this.functionName;
};

StackFrame.prototype.getMethodName = function() {
return this.functionName;
};

StackFrame.prototype.getFunctionName = function() {
return this.functionName;
};
Expand All @@ -70,6 +86,21 @@
return false;
};

StackFrame.prototype.isToplevel = function() {
// TODO
return false;
};

StackFrame.prototype.isNative = function() {
// TODO
return false;
};

StackFrame.prototype.isConstructor = function() {
// TODO
return false;
};

StackFrame.prototype.toString = function() {
return (this.functionName || 'Anonymous function') + ' (' +
this.scriptName + ':' + this.lineNumber + ':' + this.column + ')';
Expand All @@ -89,13 +120,32 @@
// Parse 'stack' string into StackTrace frames. Skip top 'skipDepth' frames,
// and optionally skip top to 'startName' function frames.
function parseStack(stack, skipDepth, startName) {
var splittedStack = stack.split('\n');
splittedStack.splice(0, skipDepth + 1); // also skip top name/message line
var stackSplitter = /\)\s*at/;
var reStackDetails = /\s(?:at\s)?(.*)\s\((.*)/;
var fileDetailsSplitter = /:(\d+)/;

var curr = parseStack;
var splittedStack = stack.split(stackSplitter);
var errstack = [];

for (var i = 0; i < splittedStack.length; i++) {
var parens = /\(/.exec(splittedStack[i]);
var funcName = splittedStack[i].substr(6, parens.index - 7);
// parseStack has 1 frame lesser than skipDepth. So skip calling .caller
// once. After that, continue calling .caller
if (skipDepth != 1 && curr) {
try {
curr = curr.caller;
} catch (e) {
curr = undefined; // .caller might not be allowed in curr's context
}
}

if (skipDepth-- > 0) {
continue;
}

var func = curr;
var stackDetails = reStackDetails.exec(splittedStack[i]);
var funcName = stackDetails[1];

if (startName) {
if (funcName === startName) {
Expand All @@ -107,28 +157,14 @@
funcName = null;
}

var location = splittedStack[i].substr(parens.index + 1,
splittedStack[i].length - parens.index - 2);
var fileDetails = stackDetails[2].split(fileDetailsSplitter);

var fileName = location;
var lineNumber = 0;
var columnNumber = 0;
var fileName = fileDetails[0];
var lineNumber = fileDetails[1] ? fileDetails[1] : 0;
var columnNumber = fileDetails[3] ? fileDetails[3] : 0;

var colonPattern = /:[0-9]+/g;
var firstColon = colonPattern.exec(location);
if (firstColon) {
fileName = location.substr(0, firstColon.index);

var secondColon = colonPattern.exec(location);
if (secondColon) {
lineNumber = parseInt(location.substr(firstColon.index + 1,
secondColon.index - firstColon.index - 1), 10);
columnNumber = parseInt(location.substr(secondColon.index + 1,
location.length - secondColon.index), 10);
}
}
errstack.push(
new StackFrame(funcName, fileName, lineNumber, columnNumber));
new StackFrame(func, funcName, fileName, lineNumber, columnNumber));
}
return errstack;
}
Expand Down Expand Up @@ -190,7 +226,7 @@

var funcSkipDepth = findFuncDepth(func);
var startFuncName = (func && funcSkipDepth < 0) ? func.name : undefined;
skipDepth += Math.max(funcSkipDepth, 0);
skipDepth += Math.max(funcSkipDepth - 1, 0);

var currentStackTrace;
function ensureStackTrace() {
Expand All @@ -216,6 +252,13 @@
// this Chakra runtime would reset stack at throw time.
Reflect_apply(oldStackDesc.set, e, [value]);
}

// To retain overriden stackAccessors below,notify Chakra runtime to not
// reset stack for this error object.
if (e !== err) {
Reflect_apply(oldStackDesc.set, err, ['']);
}

Object_defineProperty(err, 'stack', {
get: stackGetter, set: stackSetter, configurable: true, enumerable: false
});
Expand Down

0 comments on commit 32b85f9

Please sign in to comment.