Merge pull request #3444 from benjiwheeler/join-flow-sentry-wrap

Add Sentry to Join Flow; set Sentry tags on various ErrorBoundaries

src/components/errorboundary/errorboundary.jsx

@@ -17,6 +17,10 @@ class ErrorBoundary extends React.Component {
     componentDidCatch (error, errorInfo) {
         // Display fallback UI
         Sentry.withScope(scope => {
+            scope.setTag('project', 'scratch-www');
+            if (this.props.componentName) {
+                scope.setTag('component', this.props.componentName);
+            }
             Object.keys(errorInfo).forEach(key => {
                 scope.setExtra(key, errorInfo[key]);
             });
@@ -46,7 +50,8 @@ class ErrorBoundary extends React.Component {
     }
 }
 ErrorBoundary.propTypes = {
-    children: PropTypes.node
+    children: PropTypes.node,
+    componentName: PropTypes.string
 };
 
 module.exports = ErrorBoundary;

src/components/page/www/page.jsx

@@ -10,7 +10,7 @@ const Page = ({
     children,
     className
 }) => (
-    <ErrorBoundary>
+    <ErrorBoundary componentName="Page">
         <div className={classNames('page', className)}>
             <div
                 className={classNames({

src/views/join/join.jsx

@@ -5,9 +5,12 @@ const ErrorBoundary = require('../../components/errorboundary/errorboundary.jsx'
 // Require this even though we don't use it because, without it, webpack runs out of memory...
 const Page = require('../../components/page/www/page.jsx'); // eslint-disable-line no-unused-vars
 
+const initSentry = require('../../lib/sentry.js');
+initSentry();
+
 require('./join.scss');
 const Register = () => (
-    <ErrorBoundary>
+    <ErrorBoundary componentName="Join">
         <div className="join">
             <a
                 aria-label="Scratch"

src/views/preview/embed-view.jsx

@@ -5,7 +5,6 @@ const PropTypes = require('prop-types');
 const connect = require('react-redux').connect;
 const injectIntl = require('react-intl').injectIntl;
 
-const ErrorBoundary = require('../../components/errorboundary/errorboundary.jsx');
 const projectShape = require('./projectshape.jsx').projectShape;
 const NotAvailable = require('../../components/not-available/not-available.jsx');
 const Meta = require('./meta.jsx');
@@ -35,11 +34,9 @@ class EmbedView extends React.Component {
     render () {
         if (this.props.projectNotAvailable || this.state.invalidProject) {
             return (
-                <ErrorBoundary>
+                <div className="preview">
-                    <div className="preview">
+                    <NotAvailable />
-                        <NotAvailable />
+                </div>
-                    </div>
-                </ErrorBoundary>
             );
         }
 

src/views/preview/embed.jsx

@@ -13,7 +13,9 @@ const UnsupportedBrowser = require('./unsupported-browser.jsx');
 if (isSupportedBrowser()) {
     const EmbedView = require('./embed-view.jsx');
     render(
-        <EmbedView.View />,
+        <ErrorBoundary componentName="EmbedView">
+            <EmbedView.View />
+        </ErrorBoundary>,
         document.getElementById('app'),
         {
             preview: previewActions.previewReducer,
@@ -26,5 +28,8 @@ if (isSupportedBrowser()) {
         EmbedView.guiMiddleware
     );
 } else {
-    render(<ErrorBoundary><UnsupportedBrowser /></ErrorBoundary>, document.getElementById('app'));
+    render(
+        <ErrorBoundary componentName="UnsupportedBrowser"><UnsupportedBrowser /></ErrorBoundary>,
+        document.getElementById('app')
+    );
 }

test/unit/components/errorboundary.test.jsx

@@ -0,0 +1,76 @@
+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', () => {
+    let errorBoundaryWrapper;
+
+    const ChildClass = () => (
+        <div>
+            Children here
+        </div>
+    );
+
+    beforeEach(() => {
+        errorBoundaryWrapper = mountWithIntl(
+            <ErrorBoundary
+                componentName="TestEBName"
+            >
+                <ChildClass id="childClass" />
+            </ErrorBoundary>
+        );
+    });
+
+    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 errorBoundaryInstance = errorBoundaryWrapper.instance();
+        errorBoundaryInstance.componentDidCatch('error', {});
+        expect(Sentry.captureException).toHaveBeenCalledWith('error');
+    });
+
+    test('throwing error under ErrorBoundary calls Sentry.withScope()', () => {
+        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 child = errorBoundaryWrapper.find('#childClass');
+        expect(child.exists()).toEqual(true);
+        child.simulateError({});
+        expect(Sentry.setTag).toHaveBeenCalledWith('component', 'TestEBName');
+    });
+});