Refactor parseStack to break out file-reading and line-parsing into s…

…erial steps
getsentry · May 17, 2017 · 1e883b4 · 1e883b4
1 parent 1573458
commit 1e883b4
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 54 deletions.
diff --git a/lib/utils.js b/lib/utils.js
@@ -6,7 +6,6 @@ var transports = require('./transports');
 var path = require('path');
 var lsmod = require('lsmod');
 var stacktrace = require('stack-trace');
-var Promise = require('promise/domains');
 
 var ravenVersion = require('../package.json').version;
 
@@ -142,84 +141,77 @@ function getModule(filename, base) {
   return file;
 }
 
-function parseLines(lines, frame) {
-  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 readSourceFiles(filenames, cb) {
+  // we're relying on filenames being de-duped already
+  if (filenames.length === 0) return setTimeout(cb, 0, {});
 
-function readFilePromise(filename) {
-  return new Promise(function (resolve, reject) {
-    fs.readFile(filename, function (err, file) {
-      return err ? reject(err) : resolve(file);
+  var sourceFiles = {};
+  var numFilesToRead = filenames.length;
+  return filenames.forEach(function (filename) {
+    fs.readFile(filename, function (readErr, file) {
+      if (!readErr) sourceFiles[filename] = file.toString().split('\n');
+      if (--numFilesToRead === 0) cb(sourceFiles);
     });
   });
 }
 
 function parseStack(err, cb) {
-  var frames = [],
-      cache = {};
-
-  if (!err) {
-    return cb(frames);
-  }
+  if (!err) return cb([]);
 
   var stack = stacktrace.parse(err);
-
-  // check to make sure that the stack is what we need it to be.
   if (!stack || !Array.isArray(stack) || !stack.length || !stack[0].getFileName) {
-    // lol, stack is fucked
-    return cb(frames);
+    // the stack is not the useful thing we were expecting :/
+    return cb([]);
   }
 
-  var callbacks = stack.length;
-
-  // Sentry requires the stack trace to be from oldest to newest
+  // Sentry expects the stack trace to be oldest -> newest, v8 provides newest -> oldest
   stack.reverse();
 
-  return stack.forEach(function (line, index) {
+  var frames = [];
+  var filesToRead = {};
+  stack.forEach(function (line) {
     var frame = {
-          filename: line.getFileName() || '',
-          lineno: line.getLineNumber(),
-          colno: line.getColumnNumber(),
-          'function': getFunction(line),
-        },
-        isInternal = line.isNative() ||
-          frame.filename[0] !== '/' &&
-          frame.filename[0] !== '.' &&
-          frame.filename.indexOf(':\\') !== 1;
+      filename: line.getFileName() || '',
+      lineno: line.getLineNumber(),
+      colno: line.getColumnNumber(),
+      'function': getFunction(line),
+    };
+
+    var isInternal = line.isNative() ||
+      frame.filename[0] !== '/' &&
+      frame.filename[0] !== '.' &&
+      frame.filename.indexOf(':\\') !== 1;
 
     // in_app is all that's not an internal Node function or a module within node_modules
     // note that isNative appears to return true even for node core libraries
     // see https://github.com/getsentry/raven-node/issues/176
     frame.in_app = !isInternal && frame.filename.indexOf('node_modules/') === -1;
 
     // Extract a module name based on the filename
-    if (frame.filename) frame.module = getModule(frame.filename);
-
-    // internal Node files are not full path names. Ignore them.
-    if (isInternal) {
-      frames[index] = frame;
-      if (--callbacks === 0) cb(frames);
-      return;
+    if (frame.filename) {
+      frame.module = getModule(frame.filename);
+      if (!isInternal) filesToRead[frame.filename] = true;
     }
 
-    if (!cache[frame.filename]) {
-      cache[frame.filename] = readFilePromise(frame.filename)
-        .then(function (file) {
-          return file.toString().split('\n');
-        }, function () {
-          return null;
-        });
-    }
+    frames.push(frame);
+  });
 
-    cache[frame.filename].then(function (file) {
-      if (file) {
-        parseLines(file, frame);
+  return readSourceFiles(Object.keys(filesToRead), function (sourceFiles) {
+    frames.forEach(function (frame) {
+      if (frame.filename && sourceFiles[frame.filename]) {
+        var lines = sourceFiles[frame.filename];
+        try {
+          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) {
+          // anomaly, being defensive in case
+          // unlikely to ever happen in practice but can definitely happen in theory
+        }
       }
-      frames[index] = frame;
-      if (--callbacks === 0) cb(frames);
     });
+
+    cb(frames);
   });
 }
 

diff --git a/package.json b/package.json
@@ -31,7 +31,6 @@
     "cookie": "0.3.1",
     "json-stringify-safe": "5.0.1",
     "lsmod": "1.0.0",
-    "promise": "^7.1.1",
     "stack-trace": "0.0.9",
     "timed-out": "4.0.1",
     "uuid": "3.0.0"