Angular and reactNative data callback should return updated data. (#891)

* ensure angular and react-native wrap callbacks without altering Raven API
* abstract setCallback wrapping into single function

Author: graingert

plugins/angular.js

@@ -5,6 +5,8 @@
  */
 'use strict';
 
+var wrappedCallback = require('../src/utils').wrappedCallback;
+
 // See https://github.com/angular/angular.js/blob/v1.4.7/src/minErr.js
 var angularPattern = /^\[((?:[$a-zA-Z0-9]+:)?(?:[$a-zA-Z0-9]+))\] (.*?)\n?(\S+)$/;
 var moduleName = 'ngRaven';
@@ -39,11 +41,9 @@ function angularPlugin(Raven, angular) {
         .provider('Raven',  RavenProvider)
         .config(['$provide', ExceptionHandlerProvider]);
 
-    Raven.setDataCallback(function(data, original) {
-        angularPlugin._normalizeData(data);
-
-        original && original(data);
-    });
+    Raven.setDataCallback(wrappedCallback(function(data) {
+        return angularPlugin._normalizeData(data);
+    }));
 }
 
 angularPlugin._normalizeData = function (data) {
@@ -63,6 +63,8 @@ angularPlugin._normalizeData = function (data) {
             data.extra.angularDocs = matches[3].substr(0, 250);
         }
     }
+
+    return data;
 };
 
 angularPlugin.moduleName = moduleName;

plugins/react-native.js

@@ -19,6 +19,8 @@
  */
 'use strict';
 
+var wrappedCallback = require('../src/utils').wrappedCallback;
+
 // Example React Native path format (iOS):
 // /var/containers/Bundle/Application/{DEVICE_ID}/HelloWorld.app/main.jsbundle
 
@@ -60,9 +62,9 @@ function reactNativePlugin(Raven, options) {
     Raven.setTransport(reactNativePlugin._transport);
 
     // Use data callback to strip device-specific paths from stack traces
-    Raven.setDataCallback(function(data) {
-        reactNativePlugin._normalizeData(data, options.pathStrip)
-    });
+    Raven.setDataCallback(wrappedCallback(function(data) {
+        return reactNativePlugin._normalizeData(data, options.pathStrip);
+    }));
 
     // Check for a previously persisted payload, and report it.
     reactNativePlugin._restorePayload()
@@ -224,6 +226,7 @@ reactNativePlugin._normalizeData = function (data, pathStripRe) {
             frame.filename = normalizeUrl(frame.filename, pathStripRe);
         });
     }
+    return data;
 };
 
 module.exports = reactNativePlugin;

src/raven.js

@@ -26,6 +26,13 @@ var _window = typeof window !== 'undefined' ? window
 var _document = _window.document;
 var _navigator = _window.navigator;
 
+
+function keepOriginalCallback(original, callback) {
+    return isFunction(callback) ?
+    function (data) { return callback(data, original) } :
+    callback;
+}
+
 // First, check for JSON support
 // If there is no JSON, we no-op the core features of Raven
 // since JSON is required to encode the payload
@@ -573,10 +580,8 @@ Raven.prototype = {
      */
     setDataCallback: function(callback) {
         var original = this._globalOptions.dataCallback;
-        this._globalOptions.dataCallback = isFunction(callback)
-          ? function (data) { return callback(data, original); }
-          : callback;
-
+        this._globalOptions.dataCallback =
+          keepOriginalCallback(original, callback);
         return this;
     },
 
@@ -589,10 +594,8 @@ Raven.prototype = {
      */
     setBreadcrumbCallback: function(callback) {
         var original = this._globalOptions.breadcrumbCallback;
-        this._globalOptions.breadcrumbCallback = isFunction(callback)
-          ? function (data) { return callback(data, original); }
-          : callback;
-
+        this._globalOptions.breadcrumbCallback =
+          keepOriginalCallback(original, callback);
         return this;
     },
 
@@ -605,10 +608,8 @@ Raven.prototype = {
      */
     setShouldSendCallback: function(callback) {
         var original = this._globalOptions.shouldSendCallback;
-        this._globalOptions.shouldSendCallback = isFunction(callback)
-            ? function (data) { return callback(data, original); }
-            : callback;
-
+        this._globalOptions.shouldSendCallback =
+          keepOriginalCallback(original, callback);
         return this;
     },
 

src/utils.js

@@ -15,7 +15,20 @@ function isError(value) {
   }
 }
 
+function wrappedCallback(callback) {
+    function dataCallback(data, original) {
+      var normalizedData = callback(data) || data;
+      if (original) {
+          return original(normalizedData) || normalizedData;
+      }
+      return normalizedData;
+    }
+
+    return dataCallback;
+}
+
 module.exports = {
     isObject: isObject,
-    isError: isError
+    isError: isError,
+    wrappedCallback: wrappedCallback
 };

test/utils.test.js

@@ -3,6 +3,7 @@
 'use strict';
 
 var Raven = require('../src/raven');
+var wrappedCallback = require('../src/utils').wrappedCallback;
 var utils = Raven.utils;
 var RavenConfigError = require('../src/configError');
 
@@ -290,4 +291,62 @@ describe('utils', function () {
             });
         });
     });
+
+    describe('wrappedCallback', function () {
+        it('should return data from callback', function () {
+            var expected = 'yup';
+            var cb = wrappedCallback(function() {
+              return expected;
+            });
+            assert.equal(cb(), expected);
+        });
+
+        it('should return mutated data from callback', function () {
+            var cb = wrappedCallback(function(data) {
+                data.mutated = true;
+            });
+            assert.deepEqual(cb({}), {mutated: true});
+        });
+
+        it('should return data from original', function () {
+            var expected = 'yup';
+            var cb = wrappedCallback(function(data) {
+                return 'nope';
+            });
+
+            function original() {
+                return expected;
+            }
+            assert.equal(cb({}, original), expected);
+        });
+
+        it('should return mutated data from original', function () {
+            var cb = wrappedCallback(function(data) {
+                data.mutatedSomeMore = true;
+            });
+
+            function original(data) {
+                data.mutated = true;
+            }
+            assert.deepEqual(cb({}, original), {
+                mutated: true,
+                mutatedSomeMore: true
+            });
+        });
+
+        it('should call callback and original in the right order', function () {
+            var cb = wrappedCallback(function(data) {
+                return data + 'callback first, ';
+            });
+
+            function original(data) {
+                return data + 'then the original.';
+            }
+
+            assert.equal(
+                cb('it will run the ', original),
+                'it will run the callback first, then the original.'
+            );
+        });
+    });
 });