Merge pull request #345 from rollbar/fix-server-errors

Address issue with handleItemWithError possible not calling the callback
rollbar · Jul 27, 2017 · 0a3aff5 · 0a3aff5
2 parents 222e8dc + 27063ca
commit 0a3aff5
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 28 deletions.
diff --git a/sdks/rollbar.js/src/rollbar.js b/sdks/rollbar.js/src/rollbar.js
@@ -77,15 +77,17 @@ Rollbar.prototype._log = function(defaultLevel, item) {
   if (this._sameAsLastError(item)) {
     return;
   }
-  _.wrapRollbarFunction(this.logger, function() {
+  try {
     var callback = null;
     if (item.callback) {
       callback = item.callback;
       delete item.callback;
     }
     item.level = item.level || defaultLevel;
     this.notifier.log(item, callback);
-  }, this)();
+  } catch (e) {
+    this.logger.error(e)
+  }
 };
 
 Rollbar.prototype._defaultLogLevel = function() {

diff --git a/sdks/rollbar.js/src/server/transforms.js b/sdks/rollbar.js/src/server/transforms.js
@@ -73,24 +73,23 @@ function addBody(item, options, callback) {
 
 function handleItemWithError(item, options, callback) {
   if (!item.err) {
-    callback(null, item);
-    return;
+    return callback(null, item);
   }
 
   var err = item.err;
   var errors = [];
   var chain = [];
   do {
     errors.push(err);
-  } while ((err = err.nested) !== undefined);
+    err = err.nested;
+  } while (err !== undefined);
   item.stackInfo = chain;
 
-  var cb = function(err) {
-    if (err) {
+  var cb = function(e) {
+    if (e) {
       item.message = item.err.message || item.err.description || item.message || String(item.err);
       delete item.err;
       delete item.stackInfo;
-      callback(null, item);
     }
     callback(null, item);
   };
@@ -171,7 +170,7 @@ function _buildTraceData(chain) {
         }
       });
 
-      return cb();
+      return cb(null);
     });
   };
 }

diff --git a/sdks/rollbar.js/src/utility.js b/sdks/rollbar.js/src/utility.js
@@ -103,22 +103,6 @@ function isError(e) {
   return isType(e, 'error');
 }
 
-/* wrapRollbarFunction - puts a try/catch around a function, logs caught exceptions to console.error
- *
- * @param f - a function
- * @param ctx - an optional context to bind the function to
- */
-function wrapRollbarFunction(logger, f, ctx) {
-  return function() {
-    var self = ctx || this;
-    try {
-      return f.apply(self, arguments);
-    } catch (e) {
-      logger.error(e);
-    }
-  };
-}
-
 function traverse(obj, func) {
   var k;
   var v;
@@ -344,6 +328,16 @@ function makeUnhandledStackInfo(
   };
 }
 
+function wrapCallback(logger, f) {
+  return function(err, resp) {
+    try {
+      f(err, resp);
+    } catch (e) {
+      logger.error(e);
+    }
+  };
+}
+
 function createItem(args, logger, notifier, requestKeys) {
   var message, err, custom, callback, request;
   var arg;
@@ -360,7 +354,7 @@ function createItem(args, logger, notifier, requestKeys) {
         message ? extraArgs.push(arg) : message = arg;
         break;
       case 'function':
-        callback = wrapRollbarFunction(logger, arg, notifier);
+        callback = wrapCallback(logger, arg);
         break;
       case 'date':
         extraArgs.push(arg);
@@ -377,7 +371,7 @@ function createItem(args, logger, notifier, requestKeys) {
         }
         if (requestKeys && typ === 'object' && !request) {
           for (var j = 0, len = requestKeys.length; j < len; ++j) {
-            if (arg[requestKeys[j]]) {
+            if (arg[requestKeys[j]] !== undefined) {
               request = arg;
               break;
             }
@@ -550,7 +544,6 @@ module.exports = {
   traverse: traverse,
   redact: redact,
   uuid4: uuid4,
-  wrapRollbarFunction: wrapRollbarFunction,
   LEVELS: LEVELS,
   sanitizeUrl: sanitizeUrl,
   addParamsAndAccessTokenToPath: addParamsAndAccessTokenToPath,