From 999628315be63d2cf0e6e848009787f34c52736d Mon Sep 17 00:00:00 2001 From: Eli White Date: Mon, 5 Dec 2016 19:15:57 -0800 Subject: [PATCH 1/5] Adding a shouldSendBreadcrumbCallback --- src/raven.js | 22 +++++++ test/raven.test.js | 134 ++++++++++++++++++++++++++++++++++++++++++ typescript/raven.d.ts | 3 + 3 files changed, 159 insertions(+) diff --git a/src/raven.js b/src/raven.js index 0210cd43e85a..bb5b734571ef 100644 --- a/src/raven.js +++ b/src/raven.js @@ -413,6 +413,12 @@ Raven.prototype = { timestamp: now() / 1000 }, obj); + if (isFunction(this._globalOptions.shouldSendBreadcrumbCallback)) { + if (!this._globalOptions.shouldSendBreadcrumbCallback(crumb)) { + return this; + } + } + this._breadcrumbs.push(crumb); if (this._breadcrumbs.length > this._globalOptions.maxBreadcrumbs) { this._breadcrumbs.shift(); @@ -546,6 +552,22 @@ Raven.prototype = { return this; }, + /* + * Set the shouldSendBreadcrumbCallback option + * + * @param {function} callback The callback to run which allows + * introspecting breadcrumbs before sending + * @return {Raven} + */ + setShouldSendBreadcrumbCallback: function(callback) { + var original = this._globalOptions.shouldSendBreadcrumbCallback; + this._globalOptions.shouldSendBreadcrumbCallback = isFunction(callback) + ? function (data) { return callback(data, original); } + : callback; + + return this; + }, + /** * Override the default HTTP transport mechanism that transmits data * to the Sentry server. diff --git a/test/raven.test.js b/test/raven.test.js index da9ee46b8d02..5cf67edc30e1 100644 --- a/test/raven.test.js +++ b/test/raven.test.js @@ -1961,6 +1961,41 @@ describe('Raven (public API)', function() { }); }); + describe('.setShouldSendBreadcrumbCallback', function() { + it('should set the globalOptions.shouldSendBreadcrumbCallback attribute', function() { + var foo = sinon.stub(); + Raven.setShouldSendBreadcrumbCallback(foo); + + // note that shouldSendBreadcrumbCallback creates a callback/closure around + // foo, so can't test for equality - just verify that calling the wrapper + // also calls foo + Raven._globalOptions.shouldSendBreadcrumbCallback(); + assert.isTrue(foo.calledOnce); + }); + + it('should clear globalOptions.shouldSendBreadcrumbCallback with no arguments', function() { + var foo = function(){}; + Raven._globalOptions.shouldSendBreadcrumbCallback = foo; + Raven.setShouldSendBreadcrumbCallback(); + assert.isUndefined(Raven._globalOptions.shouldSendBreadcrumbCallback); + }); + + it('should generate a wrapper that passes the prior callback as the 2nd argument', function () { + var foo = sinon.stub(); + var bar = sinon.spy(function(data, orig) { + assert.equal(orig, foo); + foo(); + }); + Raven._globalOptions.shouldSendBreadcrumbCallback = foo; + Raven.setShouldSendBreadcrumbCallback(bar); + Raven._globalOptions.shouldSendBreadcrumbCallback({ + 'a': 1 // "data" + }); + assert.isTrue(bar.calledOnce); + assert.isTrue(foo.calledOnce); + }); + }); + describe('.setShouldSendCallback', function() { it('should set the globalOptions.shouldSendCallback attribute', function() { var foo = sinon.stub(); @@ -2216,6 +2251,105 @@ describe('Raven (public API)', function() { { message: 'lol', timestamp: 0.1 } ]); }); + + describe('shouldSendBreadcrumbCallback', function() { + it('should filter the breadcrumb if it returns false', function() { + Raven.setShouldSendBreadcrumbCallback(function() { + return false; + }); + + Raven.captureBreadcrumb({ + type: 'http', + data: { + url: 'http://example.org/api/0/auth/', + status_code: 200 + } + }); + + assert.strictEqual(Raven._breadcrumbs.length, 0); + }); + + it('should not filter the breadcrumb if it returns true', function() { + Raven.setShouldSendBreadcrumbCallback(function() { + return true; + }); + + Raven.captureBreadcrumb({ + type: 'http', + data: { + url: 'http://example.org/api/0/auth/', + status_code: 200 + } + }); + + assert.deepEqual(Raven._breadcrumbs[0], { + type: 'http', + timestamp: 0.1, + data: { + url: 'http://example.org/api/0/auth/', + status_code: 200 + } + }); + }); + + it('should call the callback with the breadcrumb object', function() { + var callback = this.sinon.stub().returns(true); + Raven.setShouldSendBreadcrumbCallback(callback); + + Raven.captureBreadcrumb({ + type: 'http', + data: { + url: 'http://example.org/api/0/auth/', + status_code: 200 + } + }); + + assert.isTrue(callback.calledWith({ + type: 'http', + timestamp: 0.1, + data: { + url: 'http://example.org/api/0/auth/', + status_code: 200 + } + })); + }); + + it('should enable filtering one breadcrumb but not another', function() { + function callback(data) { + if (data.data.url === 'example') { + return false; + } + + return true; + } + + Raven.setShouldSendBreadcrumbCallback(callback); + Raven.captureBreadcrumb({ + type: 'http', + data: { + url: 'example', + status_code: 200 + } + }); + + Raven.captureBreadcrumb({ + type: 'http', + data: { + url: 'foo', + status_code: 301 + } + }); + + assert.deepEqual(Raven._breadcrumbs, [{ + type: 'http', + timestamp: 0.1, + data: { + url: 'foo', + status_code: 301 + } + }]); + }); + }); }); describe('._captureUrlChange', function () { diff --git a/typescript/raven.d.ts b/typescript/raven.d.ts index a623ad7a4f31..8007a5c720c5 100644 --- a/typescript/raven.d.ts +++ b/typescript/raven.d.ts @@ -213,6 +213,9 @@ interface RavenStatic { /** Specify a callback function that allows you to apply your own filters to determine if the message should be sent to Sentry. */ setShouldSendCallback(data: any, orig?: any): RavenStatic; + /** Specify a callback function that allows you to apply your own filters to determine if a breadcrumb should be sent to Sentry. */ + setShouldSendBreadcrumbCallback(data: any, orig?: any): RavenStatic; + /** Show Sentry user feedback dialog */ showReportDialog(options: Object): void; } From 24514f0d9b016924af26d75f4051f55f14d02263 Mon Sep 17 00:00:00 2001 From: Eli White Date: Tue, 6 Dec 2016 14:09:57 -0800 Subject: [PATCH 2/5] Change to breadcrumbCallback --- src/raven.js | 38 ++++++++++++++----------- test/raven.test.js | 65 ++++++++++++++++++++++++++++--------------- typescript/raven.d.ts | 6 ++-- 3 files changed, 67 insertions(+), 42 deletions(-) diff --git a/src/raven.js b/src/raven.js index bb5b734571ef..f3f46579260b 100644 --- a/src/raven.js +++ b/src/raven.js @@ -413,8 +413,12 @@ Raven.prototype = { timestamp: now() / 1000 }, obj); - if (isFunction(this._globalOptions.shouldSendBreadcrumbCallback)) { - if (!this._globalOptions.shouldSendBreadcrumbCallback(crumb)) { + if (isFunction(this._globalOptions.breadcrumbCallback)) { + var result = this._globalOptions.breadcrumbCallback(crumb); + + if (result) { + crumb = result; + } else { return this; } } @@ -537,33 +541,33 @@ Raven.prototype = { }, /* - * Set the shouldSendCallback option + * Set the breadcrumbCallback option * - * @param {function} callback The callback to run which allows - * introspecting the blob before sending + * @param {function} callback The callback to run which allows filtering + * or mutating breadcrumbs * @return {Raven} */ - setShouldSendCallback: function(callback) { - var original = this._globalOptions.shouldSendCallback; - this._globalOptions.shouldSendCallback = isFunction(callback) - ? function (data) { return callback(data, original); } - : callback; + setBreadcrumbCallback: function(callback) { + var original = this._globalOptions.breadcrumbCallback; + this._globalOptions.breadcrumbCallback = isFunction(callback) + ? function (data) { return callback(data, original); } + : callback; return this; }, /* - * Set the shouldSendBreadcrumbCallback option + * Set the shouldSendCallback option * * @param {function} callback The callback to run which allows - * introspecting breadcrumbs before sending + * introspecting the blob before sending * @return {Raven} */ - setShouldSendBreadcrumbCallback: function(callback) { - var original = this._globalOptions.shouldSendBreadcrumbCallback; - this._globalOptions.shouldSendBreadcrumbCallback = isFunction(callback) - ? function (data) { return callback(data, original); } - : callback; + setShouldSendCallback: function(callback) { + var original = this._globalOptions.shouldSendCallback; + this._globalOptions.shouldSendCallback = isFunction(callback) + ? function (data) { return callback(data, original); } + : callback; return this; }, diff --git a/test/raven.test.js b/test/raven.test.js index 5cf67edc30e1..2d58246177e7 100644 --- a/test/raven.test.js +++ b/test/raven.test.js @@ -1961,23 +1961,23 @@ describe('Raven (public API)', function() { }); }); - describe('.setShouldSendBreadcrumbCallback', function() { - it('should set the globalOptions.shouldSendBreadcrumbCallback attribute', function() { + describe('.setBreadcrumbCallback', function() { + it('should set the globalOptions.breadcrumbCallback attribute', function() { var foo = sinon.stub(); - Raven.setShouldSendBreadcrumbCallback(foo); + Raven.setBreadcrumbCallback(foo); - // note that shouldSendBreadcrumbCallback creates a callback/closure around + // note that breadcrumbCallback creates a callback/closure around // foo, so can't test for equality - just verify that calling the wrapper // also calls foo - Raven._globalOptions.shouldSendBreadcrumbCallback(); + Raven._globalOptions.breadcrumbCallback(); assert.isTrue(foo.calledOnce); }); - it('should clear globalOptions.shouldSendBreadcrumbCallback with no arguments', function() { + it('should clear globalOptions.breadcrumbCallback with no arguments', function() { var foo = function(){}; - Raven._globalOptions.shouldSendBreadcrumbCallback = foo; - Raven.setShouldSendBreadcrumbCallback(); - assert.isUndefined(Raven._globalOptions.shouldSendBreadcrumbCallback); + Raven._globalOptions.breadcrumbCallback = foo; + Raven.setBreadcrumbCallback(); + assert.isUndefined(Raven._globalOptions.breadcrumbCallback); }); it('should generate a wrapper that passes the prior callback as the 2nd argument', function () { @@ -1986,9 +1986,9 @@ describe('Raven (public API)', function() { assert.equal(orig, foo); foo(); }); - Raven._globalOptions.shouldSendBreadcrumbCallback = foo; - Raven.setShouldSendBreadcrumbCallback(bar); - Raven._globalOptions.shouldSendBreadcrumbCallback({ + Raven._globalOptions.breadcrumbCallback = foo; + Raven.setBreadcrumbCallback(bar); + Raven._globalOptions.breadcrumbCallback({ 'a': 1 // "data" }); assert.isTrue(bar.calledOnce); @@ -2252,9 +2252,9 @@ describe('Raven (public API)', function() { ]); }); - describe('shouldSendBreadcrumbCallback', function() { + describe('breadcrumbCallback', function() { it('should filter the breadcrumb if it returns false', function() { - Raven.setShouldSendBreadcrumbCallback(function() { + Raven.setBreadcrumbCallback(function() { return false; }); @@ -2269,9 +2269,10 @@ describe('Raven (public API)', function() { assert.strictEqual(Raven._breadcrumbs.length, 0); }); - it('should not filter the breadcrumb if it returns true', function() { - Raven.setShouldSendBreadcrumbCallback(function() { - return true; + it('should mutate the breadcrumb if it returns an object', function() { + Raven.setBreadcrumbCallback(function(crumb) { + crumb.type = 'whee'; + return crumb; }); Raven.captureBreadcrumb({ @@ -2283,7 +2284,7 @@ describe('Raven (public API)', function() { }); assert.deepEqual(Raven._breadcrumbs[0], { - type: 'http', + type: 'whee', timestamp: 0.1, data: { url: 'http://example.org/api/0/auth/', @@ -2292,9 +2293,29 @@ describe('Raven (public API)', function() { }); }); + it('should enable replacing the breadcrumb', function() { + Raven.setBreadcrumbCallback(function(crumb) { + return { + foo: 'bar' + } + }); + + Raven.captureBreadcrumb({ + type: 'http', + data: { + url: 'http://example.org/api/0/auth/', + status_code: 200 + } + }); + + assert.deepEqual(Raven._breadcrumbs[0], { + foo: 'bar' + }); + }); + it('should call the callback with the breadcrumb object', function() { - var callback = this.sinon.stub().returns(true); - Raven.setShouldSendBreadcrumbCallback(callback); + var callback = this.sinon.stub().returnsArg(0); + Raven.setBreadcrumbCallback(callback); Raven.captureBreadcrumb({ type: 'http', @@ -2320,10 +2341,10 @@ describe('Raven (public API)', function() { return false; } - return true; + return data; } - Raven.setShouldSendBreadcrumbCallback(callback); + Raven.setBreadcrumbCallback(callback); Raven.captureBreadcrumb({ type: 'http', data: { diff --git a/typescript/raven.d.ts b/typescript/raven.d.ts index 8007a5c720c5..312a85e78e13 100644 --- a/typescript/raven.d.ts +++ b/typescript/raven.d.ts @@ -210,12 +210,12 @@ interface RavenStatic { /** Specify a function that allows mutation of the data payload right before being sent to Sentry. */ setDataCallback(data: any, orig?: any): RavenStatic; + /** Specify a callback function that allows you to mutate or filter breadcrumbs when they are captured. */ + setBreadcrumbCallback(data: any, orig?: any): RavenStatic; + /** Specify a callback function that allows you to apply your own filters to determine if the message should be sent to Sentry. */ setShouldSendCallback(data: any, orig?: any): RavenStatic; - /** Specify a callback function that allows you to apply your own filters to determine if a breadcrumb should be sent to Sentry. */ - setShouldSendBreadcrumbCallback(data: any, orig?: any): RavenStatic; - /** Show Sentry user feedback dialog */ showReportDialog(options: Object): void; } From b99fbd0990ecf9c235af125edf0e03590f1fc50e Mon Sep 17 00:00:00 2001 From: Eli White Date: Tue, 6 Dec 2016 14:15:32 -0800 Subject: [PATCH 3/5] Add docs for breadcrumbCallback --- docs/config.rst | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/docs/config.rst b/docs/config.rst index dfae577a06db..25a1e63f8c20 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -159,6 +159,23 @@ Those configuration options are documented below: } } +.. describe:: breadcrumbCallback + + A function that allows filtering or mutating breadcrumb payloads. + Return false to throw away the breadcrumb. + + .. code-block:: javascript + + { + breadcrumbCallback: function(crumb) { + if (crumb.type === 'http') { + return crumb; + } + + return false; + } + } + .. describe:: shouldSendCallback A callback function that allows you to apply your own filters to From 594c073387125d54f4d440cc9a843409fa2e62f6 Mon Sep 17 00:00:00 2001 From: Eli White Date: Tue, 6 Dec 2016 15:25:14 -0800 Subject: [PATCH 4/5] Updating breadcrumb return type checking --- src/raven.js | 4 +-- test/raven.test.js | 73 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 72 insertions(+), 5 deletions(-) diff --git a/src/raven.js b/src/raven.js index f3f46579260b..35bda5083427 100644 --- a/src/raven.js +++ b/src/raven.js @@ -416,9 +416,9 @@ Raven.prototype = { if (isFunction(this._globalOptions.breadcrumbCallback)) { var result = this._globalOptions.breadcrumbCallback(crumb); - if (result) { + if (isObject(result) && !isEmptyObject(result)) { crumb = result; - } else { + } else if (result === false) { return this; } } diff --git a/test/raven.test.js b/test/raven.test.js index 2d58246177e7..ccdf3879367e 100644 --- a/test/raven.test.js +++ b/test/raven.test.js @@ -2269,6 +2269,27 @@ describe('Raven (public API)', function() { assert.strictEqual(Raven._breadcrumbs.length, 0); }); + it('should not filter the breadcrumb if callback returns undefined', function() { + Raven.setBreadcrumbCallback(function() {}); + + Raven.captureBreadcrumb({ + type: 'http', + data: { + url: 'http://example.org/api/0/auth/', + status_code: 200 + } + }); + + assert.deepEqual(Raven._breadcrumbs, [{ + type: 'http', + timestamp: 0.1, + data: { + url: 'http://example.org/api/0/auth/', + status_code: 200 + } + }]); + }); + it('should mutate the breadcrumb if it returns an object', function() { Raven.setBreadcrumbCallback(function(crumb) { crumb.type = 'whee'; @@ -2283,18 +2304,18 @@ describe('Raven (public API)', function() { } }); - assert.deepEqual(Raven._breadcrumbs[0], { + assert.deepEqual(Raven._breadcrumbs, [{ type: 'whee', timestamp: 0.1, data: { url: 'http://example.org/api/0/auth/', status_code: 200 } - }); + }]); }); it('should enable replacing the breadcrumb', function() { - Raven.setBreadcrumbCallback(function(crumb) { + Raven.setBreadcrumbCallback(function() { return { foo: 'bar' } @@ -2313,6 +2334,52 @@ describe('Raven (public API)', function() { }); }); + it('should not replace the breadcrumb if a non object is returned', function() { + Raven.setBreadcrumbCallback(function() { + return 'foo'; + }); + + Raven.captureBreadcrumb({ + type: 'http', + data: { + url: 'http://example.org/api/0/auth/', + status_code: 200 + } + }); + + assert.deepEqual(Raven._breadcrumbs, [{ + type: 'http', + timestamp: 0.1, + data: { + url: 'http://example.org/api/0/auth/', + status_code: 200 + } + }]); + }); + + it('should not replace the breadcrumb if an empty object is returned', function() { + Raven.setBreadcrumbCallback(function() { + return {}; + }); + + Raven.captureBreadcrumb({ + type: 'http', + data: { + url: 'http://example.org/api/0/auth/', + status_code: 200 + } + }); + + assert.deepEqual(Raven._breadcrumbs, [{ + type: 'http', + timestamp: 0.1, + data: { + url: 'http://example.org/api/0/auth/', + status_code: 200 + } + }]); + }); + it('should call the callback with the breadcrumb object', function() { var callback = this.sinon.stub().returnsArg(0); Raven.setBreadcrumbCallback(callback); From 4da8c97f2ec76ba1f3ff3da44edc2aa7d7683a92 Mon Sep 17 00:00:00 2001 From: Eli White Date: Tue, 6 Dec 2016 16:01:53 -0800 Subject: [PATCH 5/5] Fixing test for mutation of a breadcrumb --- test/raven.test.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/raven.test.js b/test/raven.test.js index ccdf3879367e..9e5a9d4cb330 100644 --- a/test/raven.test.js +++ b/test/raven.test.js @@ -2290,10 +2290,9 @@ describe('Raven (public API)', function() { }]); }); - it('should mutate the breadcrumb if it returns an object', function() { + it('should be able to mutate the breadcrumb', function() { Raven.setBreadcrumbCallback(function(crumb) { crumb.type = 'whee'; - return crumb; }); Raven.captureBreadcrumb({