From 32b85f9f0fb70efd1c6dda088ae24357d266b595 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Thu, 23 Jun 2016 17:15:31 -0700 Subject: [PATCH] chakrashim: bug fixes for Error APIs 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: #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: #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 : #70 PR-URL: nodejs/node-chakracore#85 Reviewed-By: Jianchun Xu --- deps/chakrashim/lib/chakra_shim.js | 95 ++++++++++++++++++++++-------- 1 file changed, 69 insertions(+), 26 deletions(-) diff --git a/deps/chakrashim/lib/chakra_shim.js b/deps/chakrashim/lib/chakra_shim.js index 05f8304952f4e5..660a0a12566f69 100644 --- a/deps/chakrashim/lib/chakra_shim.js +++ b/deps/chakrashim/lib/chakra_shim.js @@ -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 @@ -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; }; @@ -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 + ')'; @@ -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) { @@ -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; } @@ -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() { @@ -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 });