From 46ef423c5d75a24084d1de839807978dfdca6359 Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Wed, 9 Oct 2019 22:29:12 -0400 Subject: [PATCH 1/9] set sentry environment in www --- src/lib/sentry.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib/sentry.js b/src/lib/sentry.js index b16cd03a9..1afdb53a4 100644 --- a/src/lib/sentry.js +++ b/src/lib/sentry.js @@ -5,6 +5,7 @@ const initSentry = () => { Sentry.init({ dsn: `${process.env.SENTRY_DSN}`, + environment: 'www', // Do not collect global onerror, only collect specifically from React error boundaries. // TryCatch plugin also includes errors from setTimeouts (i.e. the VM) integrations: integrations => integrations.filter(i => From 0f8eb638cc5d0547ac74f6192cab4c4699a06640 Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Wed, 9 Oct 2019 22:30:35 -0400 Subject: [PATCH 2/9] add support in ErrorBoundary for setting errorboundary tag --- src/components/errorboundary/errorboundary.jsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/components/errorboundary/errorboundary.jsx b/src/components/errorboundary/errorboundary.jsx index 90bd7b368..22cc94351 100644 --- a/src/components/errorboundary/errorboundary.jsx +++ b/src/components/errorboundary/errorboundary.jsx @@ -17,6 +17,9 @@ class ErrorBoundary extends React.Component { componentDidCatch (error, errorInfo) { // Display fallback UI Sentry.withScope(scope => { + if (this.props.name) { + scope.setTag('errorboundary', this.props.name); + } Object.keys(errorInfo).forEach(key => { scope.setExtra(key, errorInfo[key]); }); @@ -46,7 +49,8 @@ class ErrorBoundary extends React.Component { } } ErrorBoundary.propTypes = { - children: PropTypes.node + children: PropTypes.node, + name: PropTypes.string }; module.exports = ErrorBoundary; From 54f30fc944e88db314d38c80f9e10d34320a490f Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Wed, 9 Oct 2019 22:30:59 -0400 Subject: [PATCH 3/9] set errorboundary name tag everywhere ErrorBoundary is used --- src/components/page/www/page.jsx | 2 +- src/views/join/join.jsx | 5 ++++- src/views/preview/embed-view.jsx | 2 +- src/views/preview/embed.jsx | 5 ++++- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/components/page/www/page.jsx b/src/components/page/www/page.jsx index d20e8fda6..19142005a 100644 --- a/src/components/page/www/page.jsx +++ b/src/components/page/www/page.jsx @@ -10,7 +10,7 @@ const Page = ({ children, className }) => ( - +
( - +
+
diff --git a/src/views/preview/embed.jsx b/src/views/preview/embed.jsx index f458948be..8ef61e37e 100644 --- a/src/views/preview/embed.jsx +++ b/src/views/preview/embed.jsx @@ -26,5 +26,8 @@ if (isSupportedBrowser()) { EmbedView.guiMiddleware ); } else { - render(, document.getElementById('app')); + render( + , + document.getElementById('app') + ); } From fbbf4678786b05e9b8a659c8304b66040383ce5a Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Wed, 9 Oct 2019 22:48:11 -0400 Subject: [PATCH 4/9] =?UTF-8?q?set=20=E2=80=98www=E2=80=99=20as=20tag,=20r?= =?UTF-8?q?ather=20than=20environment?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/components/errorboundary/errorboundary.jsx | 1 + src/lib/sentry.js | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/errorboundary/errorboundary.jsx b/src/components/errorboundary/errorboundary.jsx index 22cc94351..aa86631f6 100644 --- a/src/components/errorboundary/errorboundary.jsx +++ b/src/components/errorboundary/errorboundary.jsx @@ -17,6 +17,7 @@ class ErrorBoundary extends React.Component { componentDidCatch (error, errorInfo) { // Display fallback UI Sentry.withScope(scope => { + scope.setTag('project', 'www'); if (this.props.name) { scope.setTag('errorboundary', this.props.name); } diff --git a/src/lib/sentry.js b/src/lib/sentry.js index 1afdb53a4..b16cd03a9 100644 --- a/src/lib/sentry.js +++ b/src/lib/sentry.js @@ -5,7 +5,6 @@ const initSentry = () => { Sentry.init({ dsn: `${process.env.SENTRY_DSN}`, - environment: 'www', // Do not collect global onerror, only collect specifically from React error boundaries. // TryCatch plugin also includes errors from setTimeouts (i.e. the VM) integrations: integrations => integrations.filter(i => From cd73afee1bb826970c1dd520cdffa40b1a4c90b7 Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Wed, 9 Oct 2019 23:05:23 -0400 Subject: [PATCH 5/9] =?UTF-8?q?use=20=E2=80=98scratch-www=E2=80=99=20rathe?= =?UTF-8?q?r=20than=20=E2=80=98www=E2=80=99=20in=20report=20to=20sentry?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/components/errorboundary/errorboundary.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/errorboundary/errorboundary.jsx b/src/components/errorboundary/errorboundary.jsx index aa86631f6..75d60b8ac 100644 --- a/src/components/errorboundary/errorboundary.jsx +++ b/src/components/errorboundary/errorboundary.jsx @@ -17,7 +17,7 @@ class ErrorBoundary extends React.Component { componentDidCatch (error, errorInfo) { // Display fallback UI Sentry.withScope(scope => { - scope.setTag('project', 'www'); + scope.setTag('project', 'scratch-www'); if (this.props.name) { scope.setTag('errorboundary', this.props.name); } From 487ca236be83940cffa06870524b6e8e665d74c8 Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Thu, 10 Oct 2019 06:53:30 -0400 Subject: [PATCH 6/9] Add errorboundary tests --- test/unit/components/errorboundary.test.jsx | 94 +++++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 test/unit/components/errorboundary.test.jsx diff --git a/test/unit/components/errorboundary.test.jsx b/test/unit/components/errorboundary.test.jsx new file mode 100644 index 000000000..bcae8c46d --- /dev/null +++ b/test/unit/components/errorboundary.test.jsx @@ -0,0 +1,94 @@ +import React from 'react'; +const {mountWithIntl} = require('../../helpers/intl-helpers.jsx'); + +jest.mock('@sentry/browser', () => { + const setExtra = jest.fn(); + const setTag = jest.fn(); + + const makeScope = (setExtraParam, setTagParam) => { + const thisScope = { + setExtra: setExtraParam, + setTag: setTagParam + }; + return thisScope; + }; + const Sentry = { + captureException: jest.fn(), + lastEventId: function () { + return 0; + }, + setExtra: setExtra, + setTag: setTag, + withScope: jest.fn(cb => { + cb(makeScope(setExtra, setTag)); + }) + }; + return Sentry; +}); + +const Sentry = require('@sentry/browser'); +import ErrorBoundary from '../../../src/components/errorboundary/errorboundary.jsx'; + +describe('ErrorBoundary', () => { + test('calling ErrorBoundary\'s componentDidCatch() calls Sentry.withScope()', () => { + const errorBoundaryWrapper = mountWithIntl( + +
+ Children here +
+
+ ); + const errorBoundaryInstance = errorBoundaryWrapper.instance(); + errorBoundaryInstance.componentDidCatch('error', {}); + expect(Sentry.withScope).toHaveBeenCalled(); + }); + + test('calling ErrorBoundary\'s componentDidCatch() calls Sentry.captureException()', () => { + const errorBoundaryWrapper = mountWithIntl( + +
+ Children here +
+
+ ); + const errorBoundaryInstance = errorBoundaryWrapper.instance(); + errorBoundaryInstance.componentDidCatch('error', {}); + expect(Sentry.captureException).toHaveBeenCalledWith('error'); + }); + + test('throwing error under ErrorBoundary calls Sentry.withScope()', () => { + const ChildClass = () => ( +
+ Children here +
+ ); + const errorBoundaryWrapper = mountWithIntl( + + + + ); + const child = errorBoundaryWrapper.find('#childClass'); + expect(child.exists()).toEqual(true); + child.simulateError({}, {}); + expect(Sentry.withScope).toHaveBeenCalled(); + }); + + test('ErrorBoundary with name prop causes Sentry to setTag with that name', () => { + const ChildClass = () => ( +
+ Children here +
+ ); + const errorBoundaryWrapper = mountWithIntl( + + + + ); + const child = errorBoundaryWrapper.find('#childClass'); + expect(child.exists()).toEqual(true); + child.simulateError({}); + expect(Sentry.setTag).toHaveBeenCalledWith('errorboundary', 'testEB'); + }); +}); From 91a05d636f61784b508170a4f01a64fec1e28b8d Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Thu, 10 Oct 2019 13:15:35 -0400 Subject: [PATCH 7/9] redo sentry tag key and values, move ErrorBoundary around embed --- src/components/errorboundary/errorboundary.jsx | 2 +- src/components/page/www/page.jsx | 2 +- src/views/join/join.jsx | 2 +- src/views/preview/embed-view.jsx | 9 +++------ src/views/preview/embed.jsx | 6 ++++-- 5 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/components/errorboundary/errorboundary.jsx b/src/components/errorboundary/errorboundary.jsx index 75d60b8ac..41a641421 100644 --- a/src/components/errorboundary/errorboundary.jsx +++ b/src/components/errorboundary/errorboundary.jsx @@ -19,7 +19,7 @@ class ErrorBoundary extends React.Component { Sentry.withScope(scope => { scope.setTag('project', 'scratch-www'); if (this.props.name) { - scope.setTag('errorboundary', this.props.name); + scope.setTag('component', this.props.name); } Object.keys(errorInfo).forEach(key => { scope.setExtra(key, errorInfo[key]); diff --git a/src/components/page/www/page.jsx b/src/components/page/www/page.jsx index 19142005a..150c03c52 100644 --- a/src/components/page/www/page.jsx +++ b/src/components/page/www/page.jsx @@ -10,7 +10,7 @@ const Page = ({ children, className }) => ( - +
( - +
-
- -
- +
+ +
); } diff --git a/src/views/preview/embed.jsx b/src/views/preview/embed.jsx index 8ef61e37e..8285ffa04 100644 --- a/src/views/preview/embed.jsx +++ b/src/views/preview/embed.jsx @@ -13,7 +13,9 @@ const UnsupportedBrowser = require('./unsupported-browser.jsx'); if (isSupportedBrowser()) { const EmbedView = require('./embed-view.jsx'); render( - , + + + , document.getElementById('app'), { preview: previewActions.previewReducer, @@ -27,7 +29,7 @@ if (isSupportedBrowser()) { ); } else { render( - , + , document.getElementById('app') ); } From 46660a2ff9bb2c5c8a699791ac3ad35ac04b409b Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Thu, 10 Oct 2019 13:31:25 -0400 Subject: [PATCH 8/9] revised errorboundary tests --- test/unit/components/errorboundary.test.jsx | 54 +++++++-------------- 1 file changed, 18 insertions(+), 36 deletions(-) diff --git a/test/unit/components/errorboundary.test.jsx b/test/unit/components/errorboundary.test.jsx index bcae8c46d..d781f38ac 100644 --- a/test/unit/components/errorboundary.test.jsx +++ b/test/unit/components/errorboundary.test.jsx @@ -30,43 +30,37 @@ const Sentry = require('@sentry/browser'); import ErrorBoundary from '../../../src/components/errorboundary/errorboundary.jsx'; describe('ErrorBoundary', () => { - test('calling ErrorBoundary\'s componentDidCatch() calls Sentry.withScope()', () => { - const errorBoundaryWrapper = mountWithIntl( - -
- Children here -
+ let errorBoundaryWrapper; + + const ChildClass = () => ( +
+ Children here +
+ ); + + beforeEach(() => { + errorBoundaryWrapper = mountWithIntl( + + ); + }); + + test('calling ErrorBoundary\'s componentDidCatch() calls Sentry.withScope()', () => { const errorBoundaryInstance = errorBoundaryWrapper.instance(); errorBoundaryInstance.componentDidCatch('error', {}); expect(Sentry.withScope).toHaveBeenCalled(); }); test('calling ErrorBoundary\'s componentDidCatch() calls Sentry.captureException()', () => { - const errorBoundaryWrapper = mountWithIntl( - -
- Children here -
-
- ); const errorBoundaryInstance = errorBoundaryWrapper.instance(); errorBoundaryInstance.componentDidCatch('error', {}); expect(Sentry.captureException).toHaveBeenCalledWith('error'); }); test('throwing error under ErrorBoundary calls Sentry.withScope()', () => { - const ChildClass = () => ( -
- Children here -
- ); - const errorBoundaryWrapper = mountWithIntl( - - - - ); const child = errorBoundaryWrapper.find('#childClass'); expect(child.exists()).toEqual(true); child.simulateError({}, {}); @@ -74,21 +68,9 @@ describe('ErrorBoundary', () => { }); test('ErrorBoundary with name prop causes Sentry to setTag with that name', () => { - const ChildClass = () => ( -
- Children here -
- ); - const errorBoundaryWrapper = mountWithIntl( - - - - ); const child = errorBoundaryWrapper.find('#childClass'); expect(child.exists()).toEqual(true); child.simulateError({}); - expect(Sentry.setTag).toHaveBeenCalledWith('errorboundary', 'testEB'); + expect(Sentry.setTag).toHaveBeenCalledWith('component', 'TestEBName'); }); }); From 226134bc8168c84a947a3b4454209b8fc371b29c Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Fri, 11 Oct 2019 16:25:19 -0400 Subject: [PATCH 9/9] renamed ErrorBoundary prop name to componentName --- src/components/errorboundary/errorboundary.jsx | 6 +++--- src/components/page/www/page.jsx | 2 +- src/views/join/join.jsx | 2 +- src/views/preview/embed.jsx | 4 ++-- test/unit/components/errorboundary.test.jsx | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/components/errorboundary/errorboundary.jsx b/src/components/errorboundary/errorboundary.jsx index 41a641421..a1c320e36 100644 --- a/src/components/errorboundary/errorboundary.jsx +++ b/src/components/errorboundary/errorboundary.jsx @@ -18,8 +18,8 @@ class ErrorBoundary extends React.Component { // Display fallback UI Sentry.withScope(scope => { scope.setTag('project', 'scratch-www'); - if (this.props.name) { - scope.setTag('component', this.props.name); + if (this.props.componentName) { + scope.setTag('component', this.props.componentName); } Object.keys(errorInfo).forEach(key => { scope.setExtra(key, errorInfo[key]); @@ -51,7 +51,7 @@ class ErrorBoundary extends React.Component { } ErrorBoundary.propTypes = { children: PropTypes.node, - name: PropTypes.string + componentName: PropTypes.string }; module.exports = ErrorBoundary; diff --git a/src/components/page/www/page.jsx b/src/components/page/www/page.jsx index 150c03c52..77f3712ba 100644 --- a/src/components/page/www/page.jsx +++ b/src/components/page/www/page.jsx @@ -10,7 +10,7 @@ const Page = ({ children, className }) => ( - +