Safer error handling when parsing stack traces to prevent the entire process from taking a shit, should fix #2

Author: mattrobenolt

lib/parsers.js

@@ -15,8 +15,16 @@ module.exports.parseError = function parseError(err, kwargs, cb) {
     utils.parseStack(err.stack, function(e, frames) {
         kwargs['message'] = err.name+': '+(err.message || '<no message>');
         kwargs['sentry.interfaces.Exception'] = {type:err.name, value:err.message};
-        kwargs['sentry.interfaces.Stacktrace'] = {frames:frames};
-        kwargs['culprit'] = (frames[0].filename || 'unknown file').replace(process.cwd()+'/', '')+':'+(frames[0]['function'] || 'unknown function');
+        if(frames) {
+            kwargs['sentry.interfaces.Stacktrace'] = {frames:frames};
+            kwargs['culprit'] = (frames[0].filename || 'unknown file').replace(process.cwd()+'/', '')+':'+(frames[0]['function'] || 'unknown function');
+        }
+        if(err) {
+            kwargs['sentry.interfaces.Message'] = {
+                message: err,
+                params: []
+            };
+        }
         cb(kwargs);
     });
 };

lib/utils.js

@@ -99,46 +99,54 @@ module.exports.parseStackBetter = function parseStackBetter(err, cb) {
 };
 
 module.exports.parseStack = function parseStack(stack, cb) {
-    // grab all lines except the first
-    var lines = stack.split('\n').slice(1), callbacks=lines.length, frames=[], cache={};
+    try {
+        // grab all lines except the first
+        var lines = stack.split('\n').slice(1), callbacks=lines.length, frames=[], cache={};
+        
+        if(lines.length === 0) {
+            throw new Error('No lines to parse!');
+        }
 
-    lines.forEach(function(line, index) {
-        var data = line.match(/^\s*at (?:(.+(?: \[\w\s+\])?) )?\(?(.+?)(?::(\d+):(\d+))?\)?$/).slice(1),
-            frame = {
-                filename: data[1],
-                lineno: ~~data[2]
-            };
+        lines.forEach(function(line, index) {
+            var data = line.match(/^\s*at (?:(.+(?: \[\w\s+\])?) )?\(?(.+?)(?::(\d+):(\d+))?\)?$/).slice(1),
+                frame = {
+                    filename: data[1],
+                    lineno: ~~data[2]
+                };
 
-        // only set the function key if it exists
-        if(data[0]) {
-            frame['function'] = data[0];
-        }
-        // internal Node files are not full path names. Ignore them.
-        if(frame.filename[0] === '/' || frame.filename[0] === '.') {
-            // check if it has been read in first
-            if(frame.filename in cache) {
-                parseLines(cache[frame.filename]);
-                if(--callbacks === 0) cb(null, frames);
-            } else {
-                fs.readFile(frame.filename, function(err, file) {
-                    if(!err) {
-                        file = file.toString().split('\n');
-                        cache[frame.filename] = file;
-                        parseLines(file);
-                    }
-                    frames[index] = frame;
+            // only set the function key if it exists
+            if(data[0]) {
+                frame['function'] = data[0];
+            }
+            // internal Node files are not full path names. Ignore them.
+            if(frame.filename[0] === '/' || frame.filename[0] === '.') {
+                // check if it has been read in first
+                if(frame.filename in cache) {
+                    parseLines(cache[frame.filename]);
                     if(--callbacks === 0) cb(null, frames);
-                });
+                } else {
+                    fs.readFile(frame.filename, function(err, file) {
+                        if(!err) {
+                            file = file.toString().split('\n');
+                            cache[frame.filename] = file;
+                            parseLines(file);
+                        }
+                        frames[index] = frame;
+                        if(--callbacks === 0) cb(null, frames);
+                    });
+                }
+            } else {
+                frames[index] = frame;
+                if(--callbacks === 0) cb(null, frames);
             }
-        } else {
-            frames[index] = frame;
-            if(--callbacks === 0) cb(null, frames);
-        }
 
-        function parseLines(lines) {
-            frame.pre_context = lines.slice(Math.max(0, frame.lineno-(LINES_OF_CONTEXT+1)), frame.lineno-1);
-            frame.context_line = lines[frame.lineno-1];
-            frame.post_context = lines.slice(frame.lineno, frame.lineno+LINES_OF_CONTEXT);
-        }
-    });
+            function parseLines(lines) {
+                frame.pre_context = lines.slice(Math.max(0, frame.lineno-(LINES_OF_CONTEXT+1)), frame.lineno-1);
+                frame.context_line = lines[frame.lineno-1];
+                frame.post_context = lines.slice(frame.lineno, frame.lineno+LINES_OF_CONTEXT);
+            }
+        });
+    } catch(e) {
+        cb(new Error('Can\'t parse stack trace:\n' + stack));
+    }
 };

package.json

@@ -2,7 +2,7 @@
   "name": "raven",
   "description": "A standalone (Node.js) client for Sentry",
   "keywords": ["raven", "sentry", "python"],
-  "version": "0.2.2",
+  "version": "0.2.3-dev",
   "repository": "git://github.com/mattrobenolt/raven-node.git",
   "author": "Matt Robenolt <[email protected]>",
   "main": "index",

test/raven.utils.js

@@ -1,7 +1,8 @@
 var raven = require('../')
   , fs = require('fs')
   , glob = require('glob')
-  , path = require('path');
+  , path = require('path')
+  , should = require('should');
 
 describe('raven.utils', function() {
   describe('#constructChecksum()', function(){
@@ -136,6 +137,14 @@ describe('raven.utils', function() {
       });
     });
 
+    it('should throw an error parsing an invalid stack', function(done){
+      raven.utils.parseStack('wtf?', function(err, frames){
+        should.exist(err);
+        err.should.be.an.instanceof(Error);
+        done();
+      });
+    });
+
     var stacks = glob.sync(__dirname + '/fixtures/stacks/*.txt');
     var results = glob.sync(__dirname + '/fixtures/stacks/*.json');
     stacks.forEach(function(stackname, index) {
@@ -144,6 +153,7 @@ describe('raven.utils', function() {
       it('should parse stack with '+path.basename(stackname, '.txt').replace(/_/g, ' '), function(done) {
         raven.utils.parseStack(stack, function(err, frames){
           frames.should.eql(result);
+          should.not.exist(err);
           done();
         });
       });