Skip to content

Commit 73015b4

Browse files
mattrobenoltmattrobenolt
committed
Safer error handling when parsing stack traces to prevent the entire process from taking a shit, should fix #2 
1 parent 128bde0 commit 73015b4

File tree

4 files changed

+67
-41
lines changed

4 files changed

+67
-41
lines changed

‎lib/parsers.js

+10-2
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,16 @@ module.exports.parseError = function parseError(err, kwargs, cb) {
1515
utils.parseStack(err.stack, function(e, frames) {
1616
kwargs['message'] = err.name+': '+(err.message || '<no message>');
1717
kwargs['sentry.interfaces.Exception'] = {type:err.name, value:err.message};
18-
kwargs['sentry.interfaces.Stacktrace'] = {frames:frames};
19-
kwargs['culprit'] = (frames[0].filename || 'unknown file').replace(process.cwd()+'/', '')+':'+(frames[0]['function'] || 'unknown function');
18+
if(frames) {
19+
kwargs['sentry.interfaces.Stacktrace'] = {frames:frames};
20+
kwargs['culprit'] = (frames[0].filename || 'unknown file').replace(process.cwd()+'/', '')+':'+(frames[0]['function'] || 'unknown function');
21+
}
22+
if(err) {
23+
kwargs['sentry.interfaces.Message'] = {
24+
message: err,
25+
params: []
26+
};
27+
}
2028
cb(kwargs);
2129
});
2230
};

‎lib/utils.js

+45-37
Original file line numberDiff line numberDiff line change
@@ -99,46 +99,54 @@ module.exports.parseStackBetter = function parseStackBetter(err, cb) {
9999
};
100100

101101
module.exports.parseStack = function parseStack(stack, cb) {
102-
// grab all lines except the first
103-
var lines = stack.split('\n').slice(1), callbacks=lines.length, frames=[], cache={};
102+
try {
103+
// grab all lines except the first
104+
var lines = stack.split('\n').slice(1), callbacks=lines.length, frames=[], cache={};
105+
106+
if(lines.length === 0) {
107+
throw new Error('No lines to parse!');
108+
}
104109

105-
lines.forEach(function(line, index) {
106-
var data = line.match(/^\s*at (?:(.+(?: \[\w\s+\])?) )?\(?(.+?)(?::(\d+):(\d+))?\)?$/).slice(1),
107-
frame = {
108-
filename: data[1],
109-
lineno: ~~data[2]
110-
};
110+
lines.forEach(function(line, index) {
111+
var data = line.match(/^\s*at (?:(.+(?: \[\w\s+\])?) )?\(?(.+?)(?::(\d+):(\d+))?\)?$/).slice(1),
112+
frame = {
113+
filename: data[1],
114+
lineno: ~~data[2]
115+
};
111116

112-
// only set the function key if it exists
113-
if(data[0]) {
114-
frame['function'] = data[0];
115-
}
116-
// internal Node files are not full path names. Ignore them.
117-
if(frame.filename[0] === '/' || frame.filename[0] === '.') {
118-
// check if it has been read in first
119-
if(frame.filename in cache) {
120-
parseLines(cache[frame.filename]);
121-
if(--callbacks === 0) cb(null, frames);
122-
} else {
123-
fs.readFile(frame.filename, function(err, file) {
124-
if(!err) {
125-
file = file.toString().split('\n');
126-
cache[frame.filename] = file;
127-
parseLines(file);
128-
}
129-
frames[index] = frame;
117+
// only set the function key if it exists
118+
if(data[0]) {
119+
frame['function'] = data[0];
120+
}
121+
// internal Node files are not full path names. Ignore them.
122+
if(frame.filename[0] === '/' || frame.filename[0] === '.') {
123+
// check if it has been read in first
124+
if(frame.filename in cache) {
125+
parseLines(cache[frame.filename]);
130126
if(--callbacks === 0) cb(null, frames);
131-
});
127+
} else {
128+
fs.readFile(frame.filename, function(err, file) {
129+
if(!err) {
130+
file = file.toString().split('\n');
131+
cache[frame.filename] = file;
132+
parseLines(file);
133+
}
134+
frames[index] = frame;
135+
if(--callbacks === 0) cb(null, frames);
136+
});
137+
}
138+
} else {
139+
frames[index] = frame;
140+
if(--callbacks === 0) cb(null, frames);
132141
}
133-
} else {
134-
frames[index] = frame;
135-
if(--callbacks === 0) cb(null, frames);
136-
}
137142

138-
function parseLines(lines) {
139-
frame.pre_context = lines.slice(Math.max(0, frame.lineno-(LINES_OF_CONTEXT+1)), frame.lineno-1);
140-
frame.context_line = lines[frame.lineno-1];
141-
frame.post_context = lines.slice(frame.lineno, frame.lineno+LINES_OF_CONTEXT);
142-
}
143-
});
143+
function parseLines(lines) {
144+
frame.pre_context = lines.slice(Math.max(0, frame.lineno-(LINES_OF_CONTEXT+1)), frame.lineno-1);
145+
frame.context_line = lines[frame.lineno-1];
146+
frame.post_context = lines.slice(frame.lineno, frame.lineno+LINES_OF_CONTEXT);
147+
}
148+
});
149+
} catch(e) {
150+
cb(new Error('Can\'t parse stack trace:\n' + stack));
151+
}
144152
};

‎package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"name": "raven",
33
"description": "A standalone (Node.js) client for Sentry",
44
"keywords": ["raven", "sentry", "python"],
5-
"version": "0.2.2",
5+
"version": "0.2.3-dev",
66
"repository": "git://github.com/mattrobenolt/raven-node.git",
77
"author": "Matt Robenolt <[email protected]>",
88
"main": "index",

‎test/raven.utils.js

+11-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
var raven = require('../')
22
, fs = require('fs')
33
, glob = require('glob')
4-
, path = require('path');
4+
, path = require('path')
5+
, should = require('should');
56

67
describe('raven.utils', function() {
78
describe('#constructChecksum()', function(){
@@ -136,6 +137,14 @@ describe('raven.utils', function() {
136137
});
137138
});
138139

140+
it('should throw an error parsing an invalid stack', function(done){
141+
raven.utils.parseStack('wtf?', function(err, frames){
142+
should.exist(err);
143+
err.should.be.an.instanceof(Error);
144+
done();
145+
});
146+
});
147+
139148
var stacks = glob.sync(__dirname + '/fixtures/stacks/*.txt');
140149
var results = glob.sync(__dirname + '/fixtures/stacks/*.json');
141150
stacks.forEach(function(stackname, index) {
@@ -144,6 +153,7 @@ describe('raven.utils', function() {
144153
it('should parse stack with '+path.basename(stackname, '.txt').replace(/_/g, ' '), function(done) {
145154
raven.utils.parseStack(stack, function(err, frames){
146155
frames.should.eql(result);
156+
should.not.exist(err);
147157
done();
148158
});
149159
});

0 commit comments

Comments
 (0)