Adjust wrapSubscription to handle invalid errors

lib/hijack/wrap_subscription.js

@@ -43,28 +43,30 @@ wrapSubscription = function(subscriptionProto) {
 
   var originalError = subscriptionProto.error;
   subscriptionProto.error = function(err) {
-    var kadiraInfo = Kadira._getInfo();
+    if (err) {
+      var kadiraInfo = Kadira._getInfo();
 
-    if(kadiraInfo && this._subscriptionId == kadiraInfo.trace.id) {
-      Kadira.tracer.endLastEvent(kadiraInfo.trace);
+      if(kadiraInfo && this._subscriptionId == kadiraInfo.trace.id) {
+        Kadira.tracer.endLastEvent(kadiraInfo.trace);
 
-      var errorForApm = _.pick(err, 'message', 'stack');
-      Kadira.tracer.event(kadiraInfo.trace, 'error', {error: errorForApm});
-      var trace = Kadira.tracer.buildTrace(kadiraInfo.trace);
+        var errorForApm = _.pick(err, 'message', 'stack');
+        Kadira.tracer.event(kadiraInfo.trace, 'error', {error: errorForApm});
+        var trace = Kadira.tracer.buildTrace(kadiraInfo.trace);
 
-      Kadira.models.pubsub._trackError(this._session, this, trace);
+        Kadira.models.pubsub._trackError(this._session, this, trace);
 
-      // error tracking can be disabled and if there is a trace
-      // trace should be avaialble all the time, but it won't
-      // if something wrong happened on the trace building
-      if(Kadira.options.enableErrorTracking && trace) {
-        Kadira.models.error.trackError(err, trace);
+        // error tracking can be disabled and if there is a trace
+        // trace should be avaialble all the time, but it won't
+        // if something wrong happened on the trace building
+        if(Kadira.options.enableErrorTracking && trace) {
+          Kadira.models.error.trackError(err, trace);
+        }
       }
-    }
 
-    // wrap error stack so Meteor._debug can identify and ignore it
-    err.stack = {stack: err.stack, source: 'subscription'};
-    originalError.call(this, err);
+      // wrap error stack so Meteor._debug can identify and ignore it
+      err.stack = {stack: err.stack, source: 'subscription'};
+      originalError.call(this, err);
+    }
   };
 
   var originalDeactivate = subscriptionProto._deactivate;
@@ -91,4 +93,4 @@ wrapSubscription = function(subscriptionProto) {
       return res;
     };
   });
-};
+};

package.js

@@ -1,6 +1,6 @@
 Package.describe({
   "summary": "Performance Monitoring for Meteor",
-  "version": "3.1.1",
+  "version": "3.1.2",
   "git": "https://github.com/meteor/meteor-apm-agent.git",
   "name": "mdg:meteor-apm-agent"
 });
@@ -12,7 +12,8 @@ var npmModules = {
   "evloop-monitor": "0.1.0",
   "pidusage": "0.1.1",
   "lru-cache": "4.0.0",
-  "json-stringify-safe": "5.0.1"
+  "json-stringify-safe": "5.0.1",
+  "uuid": "3.3.2"
 };
 
 Npm.depends(npmModules);

tests/client/error_reporters/meteor_debug.js

@@ -1,11 +1,12 @@
+var uuid = Npm.require('uuid');
 
 Tinytest.add(
   'Client Side - Error Manager - Reporters - meteor._debug - with zone',
   TestWithErrorTracking(function (test) {
     hijackKadiraSendErrors(mock_KadiraSendErrors);
     test.equal(typeof Meteor._debug, 'function');
     var errorSent = false;
-    var message = Meteor.uuid();
+    var message = uuid();
 
     // set window.zone as nothing
     var originalZone = window.zone;
@@ -15,7 +16,7 @@ Tinytest.add(
     test.equal(errorSent, false);
     restoreKadiraSendErrors();
 
-    // cleajr 
+    // cleajr
     window.zone = originalZone;
     function mock_KadiraSendErrors(data) {
       errorSent = true;
@@ -30,7 +31,7 @@ Tinytest.add(
     test.equal(typeof Meteor._debug, 'function');
     var errorSent = false;
     var originalZone = window.zone;
-    var message = Meteor.uuid();
+    var message = uuid();
     window.zone = undefined;
 
     try {
@@ -61,7 +62,7 @@ Tinytest.add(
     test.equal(typeof Meteor._debug, 'function');
     var errorSent = false;
     var originalZone = window.zone;
-    var message = Meteor.uuid();
+    var message = uuid();
     window.zone = undefined;
 
     try {

tests/client/error_reporters/window_error.js

@@ -1,11 +1,12 @@
+var uuid = Npm.require('uuid');
 
 Tinytest.addAsync(
   'Client Side - Error Manager - Reporters - window.onerror - with all args',
   TestWithErrorTrackingAsync(function (test, next) {
     hijackKadiraSendErrors(mock_KadiraSendErrors);
     test.equal(typeof window.onerror, 'function');
     var error = new Error('test-error');
-    var message = Meteor.uuid();
+    var message = uuid();
     window.onerror(message, '_url', 1, 1, error);
 
     function mock_KadiraSendErrors(error) {
@@ -27,7 +28,7 @@ Tinytest.addAsync(
   TestWithErrorTrackingAsync(function (test, next) {
     hijackKadiraSendErrors(mock_KadiraSendErrors);
     test.equal(typeof window.onerror, 'function');
-    var message = Meteor.uuid();
+    var message = uuid();
     window.onerror(message, '_url', 1, 1);
 
     function mock_KadiraSendErrors(error) {

tests/hijack/subscriptions.js

@@ -260,3 +260,44 @@ Tinytest.add(
     CloseClient(client);
   }
 );
+
+Tinytest.add(
+  'Subscriptions - wrapSubscription - handle undefined errors',
+  function (test) {
+    var subscription = {
+      error: function (error) {
+        // Do nothing
+      }
+    };
+
+    wrapSubscription(subscription);
+
+    // Valid error should work
+    var error1 = new Error('Oh no!');
+    test.isUndefined(error1.stack.source);
+    subscription.error(error1);
+    test.isNotUndefined(error1.stack.source);
+
+    // Invalid error should be ignored
+    var getInfo = Kadira._getInfo;
+    Kadira._getInfo = function () {
+      return {
+        trace: {
+          id: 'abc123',
+          events: [{
+            key: 'value',
+          }],
+        },
+      };
+    };
+
+    subscription._subscriptionId = 'abc123';
+    try {
+      subscription.error(undefined);
+    } catch (error) {
+      test.fail('Invalid errors should not throw an exception');
+    } finally {
+      Kadira._getInfo = getInfo;
+    }
+  }
+);