Improve log lines file naming on server.

We avoid using Error.prepareStackTrace (which the node source map support also
uses) and do some hacky regexp parsing instead. This way, on the server, we get
the filename/line numbers after source map processing.

On the client we continue to get the compiled version... I guess because source
maps are implemented in the developer tools, not directly into the Error object.

(Probably should have gotten parseStack from tools/buildmessage.js instead.)

unbreaks _getCallerDetails tests.
This commit is contained in:
David Glasser
2013-07-10 17:24:56 -07:00
parent a8ccc8786d
commit 81e456ae81
2 changed files with 29 additions and 28 deletions

View File

@@ -75,11 +75,11 @@ var logInBrowser = function (obj) {
// @returns {Object: { line: Number, file: String }}
Log._getCallerDetails = function () {
var getStack = function () {
var orig = Error.prepareStackTrace;
Error.prepareStackTrace = function(_, stack){ return stack; };
// We do NOT use Error.prepareStackTrace here (a V8 extension that gets us a
// pre-parsed stack) since it's impossible to compose it with the use of
// Error.prepareStackTrace used on the server for source maps.
var err = new Error;
var stack = err.stack;
Error.prepareStackTrace = orig;
return stack;
};
@@ -87,33 +87,37 @@ Log._getCallerDetails = function () {
if (!stack) return {};
var isV8 = false;
var lines = stack;
// check for V8 specifics
if (_.isArray(stack))
isV8 = true;
else
lines = stack.split('\n');
var index = 1;
var line = lines[index];
var lines = stack.split('\n');
// looking for the first line outside the logging package
while ((isV8 ? line.getFileName() || '' : line)
.indexOf('/packages/logging.js') !== -1)
line = lines[++index];
// looking for the first line outside the logging package (or an
// eval if we find that first)
var line;
for (var i = 1; i < lines.length; ++i) {
line = lines[i];
if (line.match(/^\s*at eval \(eval/)) {
return {file: "eval"};
}
// XXX probably wants to be / or .js in case no source maps
if (!line.match(/packages\/logging(?:\/|(?:\.tests)?\.js)/))
break;
}
var details = {};
// The format for FF is functionName@filePath:lineNumber
// For V8 call built-in function
details.line = isV8 ? line.getLineNumber() : line.split(':').slice(-1)[0];
// The format for FF is 'functionName@filePath:lineNumber'
// The format for V8 is 'functionName (packages/logging/logging.js:81)' or
// 'packages/logging/logging.js:81'
var match = /(?:[@(]| at )([^(]+?):([0-9:]+)(?:\)|$)/.exec(line);
if (!match)
return details;
// in case the matched block here is line:column
details.line = match[2].split(':')[0];
// Possible format: https://foo.bar.com/scripts/file.js?random=foobar
// For FF we parse the line, for V8 we call built-in function
// XXX: if you can write the following in better way, please do it
details.file = isV8 ? line.getFileName() || (line.isEval() ? 'eval' : '')
: line.split('@')[1].split(':').slice(0, -1).join(':');
details.file = details.file.split('/').slice(-1)[0].split('?')[0];
// XXX: what about evals?
details.file = match[1].split('/').slice(-1)[0].split('?')[0];
return details;
};

View File

@@ -4,14 +4,11 @@ Tinytest.add("logging - _getCallerDetails", function (test) {
// in Chrome and Firefox, other browsers don't give us an ability to get
// stacktrace.
if ((new Error).stack) {
//test.equal(details, { file: 'logging_test.js', line: 2 });
// XXX: When we have source maps, we should uncomment the test above and
// remove this one
test.isTrue(details.file === 'logging.tests.js');
test.equal(details.file, 'tinytest.js');
// evaled statements shouldn't crash
var code = "Log._getCallerDetails().file";
test.matches(eval(code), /^eval|logging.tests.js$/);
test.matches(eval(code), /^eval|tinytest.js$/);
}
});