Skip to content

Commit

Permalink
Merge pull request #852 from rollbar/wj-node-source-map-diagnostic
Browse files Browse the repository at this point in the history
Add diagnostic keys for node source maps
  • Loading branch information
waltjones authored May 12, 2020
2 parents 942445a + 4efc4c8 commit 38d813d
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 13 deletions.
21 changes: 14 additions & 7 deletions src/server/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,14 @@ function extractContextLines(frame, fileLines) {
};
}

function mapPosition(position) {
function mapPosition(position, diagnostic) {
return stackTrace.mapSourcePosition({
source: position.source,
line: position.line,
column: position.column
});
},
diagnostic
);
}

function parseFrameLine(line, callback) {
Expand All @@ -145,7 +147,7 @@ function parseFrameLine(line, callback) {
column: Math.floor(data[3]) - 1
};
if (this.useSourceMaps) {
position = mapPosition(position);
position = mapPosition(position, this.diagnostic);
}

frame = {
Expand Down Expand Up @@ -270,10 +272,10 @@ function gatherContexts(frames, callback) {
*/


exports.parseException = function (exc, options, callback) {
exports.parseException = function (exc, options, item, callback) {
var multipleErrs = getMultipleErrors(exc.errors);

return exports.parseStack(exc.stack, options, function (err, stack) {
return exports.parseStack(exc.stack, options, item, function (err, stack) {
var message, clss, ret, firstErr, jadeMatch, jadeData;

if (err) {
Expand Down Expand Up @@ -309,7 +311,7 @@ exports.parseException = function (exc, options, callback) {
};


exports.parseStack = function (stack, options, callback) {
exports.parseStack = function (stack, options, item, callback) {
var lines, _stack = stack;

// Some JS frameworks (e.g. Meteor) might bury the stack property
Expand All @@ -320,8 +322,13 @@ exports.parseStack = function (stack, options, callback) {
// grab all lines except the first
lines = (_stack || '').split('\n').slice(1);

if (options.nodeSourceMaps) {
item.diagnostic.node_source_maps = {};
item.diagnostic.node_source_maps.source_mapping_urls = {};
}

// Parse out all of the frame and filename info
async.map(lines, parseFrameLine.bind({ useSourceMaps: options.nodeSourceMaps }), function (err, frames) {
async.map(lines, parseFrameLine.bind({ useSourceMaps: options.nodeSourceMaps, diagnostic: item.diagnostic }), function (err, frames) {
if (err) {
return callback(err);
}
Expand Down
4 changes: 3 additions & 1 deletion src/server/sourceMap/stackTrace.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ function retrieveSourceMap(source) {
};
}

exports.mapSourcePosition = function mapSourcePosition(position) {
exports.mapSourcePosition = function mapSourcePosition(position, diagnostic) {
var sourceMap = sourceMapCache[position.source];
if (!sourceMap) {
// Call the (overrideable) retrieveSourceMap function to get the source map.
Expand All @@ -122,6 +122,7 @@ exports.mapSourcePosition = function mapSourcePosition(position) {
url: urlAndMap.url,
map: new SourceMapConsumer(urlAndMap.map)
};
diagnostic.node_source_maps.source_mapping_urls[position.source] = urlAndMap.url;

// Load all sources stored inline with the source map into the file cache
// to pretend like they are already loaded. They may not exist on disk.
Expand All @@ -139,6 +140,7 @@ exports.mapSourcePosition = function mapSourcePosition(position) {
url: null,
map: null
};
diagnostic.node_source_maps.source_mapping_urls[position.source] = 'not found';
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/server/transforms.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ function handleItemWithError(item, options, callback) {
}
callback(null, item);
};
async.eachSeries(errors, _buildTraceData(chain, options), cb);
async.eachSeries(errors, _buildTraceData(chain, options, item), cb);
}

function addRequestData(item, options, callback) {
Expand Down Expand Up @@ -224,9 +224,9 @@ function _isJsonContentType(req) {
return req.headers && req.headers['content-type'] && req.headers['content-type'].includes('json');
}

function _buildTraceData(chain, options) {
function _buildTraceData(chain, options, item) {
return function(ex, cb) {
parser.parseException(ex, options, function (err, errData) {
parser.parseException(ex, options, item, function (err, errData) {
if (err) {
return cb(err);
}
Expand Down
3 changes: 2 additions & 1 deletion test/server.parser.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,15 @@ vows.describe('parser')
'parseStack': {
'a valid stack trace': {
topic: function() {
var item = { diagnostic: {} }
var stack = 'ReferenceError: foo is not defined\n' +
' at MethodClass.method.<anonymous> (app/server.js:2:4)\n' +
' at /app/node_modules/client.js:321:23\n' +
' at (/app/node_modules/client.js:321:23)\n' +
' at MethodClass.method.(anonymous) (app/server.js:62:14)\n' +
' at MethodClass.method (app/server.ts:52:4)\n' +
' at MethodClass.method (app/server.js:62:14)\n';
p.parseStack(stack, {}, this.callback);
p.parseStack(stack, {}, item, this.callback);
},
'should parse valid js frame': function(err, frames) {
var frame = frames[0];
Expand Down
10 changes: 9 additions & 1 deletion test/server.transforms.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -275,11 +275,19 @@ vows.describe('transforms')
assert.isTrue(addItem.called);
if (addItem.called) {
var frame = addItem.getCall(0).args[0].body.trace_chain[0].frames.pop();
console.log(frame);
assert.ok(frame.filename.includes('src/index.ts'));
assert.equal(frame.lineno, 10);
assert.equal(frame.colno, 22);
assert.equal(frame.code, " var error = <Error> new CustomError('foo');");

var sourceMappingURLs = addItem.getCall(0).args[0].notifier.diagnostic.node_source_maps.source_mapping_urls;
var urls = Object.keys(sourceMappingURLs);
assert.ok(urls[0].includes('index.js'));
assert.ok(sourceMappingURLs[urls[0]].includes('index.js.map'));
assert.ok(urls[1].includes('server.transforms.test.js'));
assert.ok(sourceMappingURLs[urls[1]].includes('not found'));
assert.ok(urls[2].includes('timers.js'));
assert.ok(sourceMappingURLs[urls[2]].includes('not found'));
}
addItem.reset();
},
Expand Down

0 comments on commit 38d813d

Please sign in to comment.