Merge pull request #1953 from paulkaplan/unsupported-errors

Make error catching and reporting better

src/containers/blocks.jsx

@@ -12,6 +12,7 @@ import Prompt from './prompt.jsx';
 import BlocksComponent from '../components/blocks/blocks.jsx';
 import ExtensionLibrary from './extension-library.jsx';
 import CustomProcedures from './custom-procedures.jsx';
+import errorBoundaryHOC from '../lib/error-boundary-hoc.jsx';
 
 import {connect} from 'react-redux';
 import {updateToolbox} from '../reducers/toolbox';
@@ -415,7 +416,9 @@ const mapDispatchToProps = dispatch => ({
     }
 });
 
-export default connect(
-    mapStateToProps,
-    mapDispatchToProps
-)(Blocks);
+export default errorBoundaryHOC('Blocks')(
+    connect(
+        mapStateToProps,
+        mapDispatchToProps
+    )(Blocks)
+);

src/containers/costume-tab.jsx

@@ -11,6 +11,7 @@ import BackdropLibrary from './backdrop-library.jsx';
 import CameraModal from './camera-modal.jsx';
 import {connect} from 'react-redux';
 import {handleFileUpload, costumeUpload} from '../lib/file-uploader.js';
+import errorBoundaryHOC from '../lib/error-boundary-hoc.jsx';
 
 import {
     closeCameraCapture,
@@ -367,7 +368,9 @@ const mapDispatchToProps = dispatch => ({
     }
 });
 
-export default injectIntl(connect(
-    mapStateToProps,
-    mapDispatchToProps
-)(CostumeTab));
+export default errorBoundaryHOC('Costume Tab')(
+    injectIntl(connect(
+        mapStateToProps,
+        mapDispatchToProps
+    )(CostumeTab))
+);

src/containers/error-boundary.jsx

@@ -4,6 +4,7 @@ import platform from 'platform';
 import BrowserModalComponent from '../components/browser-modal/browser-modal.jsx';
 import CrashMessageComponent from '../components/crash-message/crash-message.jsx';
 import log from '../lib/log.js';
+import supportedBrowser from '../lib/supported-browser';
 import analytics from '../lib/analytics';
 
 class ErrorBoundary extends React.Component {
@@ -20,14 +21,27 @@ class ErrorBoundary extends React.Component {
             stack: 'Unknown stack',
             message: 'Unknown error'
         };
+
         // Display fallback UI
         this.setState({hasError: true});
+
+        // Log errors to analytics, separating supported browsers from unsupported.
+        if (supportedBrowser()) {
+            analytics.event({
+                category: 'error',
+                action: this.props.action,
+                label: error.message
+            });
+        } else {
+            analytics.event({
+                category: 'Unsupported Browser Error',
+                action: `(Unsupported Browser) ${this.props.action}`,
+                label: `${platform.name} ${error.message}`
+            });
+        }
+
+        // Log error locally for debugging as well.
         log.error(`Unhandled Error: ${error.stack}\nComponent stack: ${info.componentStack}`);
-        analytics.event({
-            category: 'error',
-            action: 'Fatal Error',
-            label: error.message
-        });
     }
 
     handleBack () {
@@ -40,21 +54,17 @@ class ErrorBoundary extends React.Component {
 
     render () {
         if (this.state.hasError) {
-            // don't use array.includes because that's something that causes IE to crash.
-            if (
-                platform.name === 'IE' ||
-                platform.name === 'Opera' ||
-                platform.name === 'Opera Mini' ||
-                platform.name === 'Silk') {
-                return <BrowserModalComponent onBack={this.handleBack} />;
+            if (supportedBrowser()) {
+                return <CrashMessageComponent onReload={this.handleReload} />;
             }
-            return <CrashMessageComponent onReload={this.handleReload} />;
+            return <BrowserModalComponent onBack={this.handleBack} />;
         }
         return this.props.children;
     }
 }
 
 ErrorBoundary.propTypes = {
+    action: PropTypes.string.isRequired, // Used for defining tracking action
     children: PropTypes.node
 };
 

src/containers/preview-modal.jsx

@@ -2,10 +2,10 @@ import bindAll from 'lodash.bindall';
 import PropTypes from 'prop-types';
 import React from 'react';
 import {connect} from 'react-redux';
-import platform from 'platform';
 
 import PreviewModalComponent from '../components/preview-modal/preview-modal.jsx';
 import BrowserModalComponent from '../components/browser-modal/browser-modal.jsx';
+import supportedBrowser from '../lib/supported-browser';
 
 import {
     closePreviewInfo,
@@ -35,18 +35,8 @@ class PreviewModal extends React.Component {
     handleViewProject () {
         this.props.onViewProject();
     }
-    supportedBrowser () {
-        if (
-            platform.name === 'IE' ||
-            platform.name === 'Opera' ||
-            platform.name === 'Opera Mini' ||
-            platform.name === 'Silk') {
-            return false;
-        }
-        return true;
-    }
     render () {
-        return (this.supportedBrowser() ?
+        return (supportedBrowser() ?
             <PreviewModalComponent
                 previewing={this.state.previewing}
                 onCancel={this.handleCancel}

src/containers/sound-tab.jsx

@@ -17,6 +17,7 @@ import SoundLibrary from './sound-library.jsx';
 
 import soundLibraryContent from '../lib/libraries/sounds.json';
 import {handleFileUpload, soundUpload} from '../lib/file-uploader.js';
+import errorBoundaryHOC from '../lib/error-boundary-hoc.jsx';
 
 import {connect} from 'react-redux';
 
@@ -260,7 +261,9 @@ const mapDispatchToProps = dispatch => ({
     }
 });
 
-export default injectIntl(connect(
-    mapStateToProps,
-    mapDispatchToProps
-)(SoundTab));
+export default errorBoundaryHOC('Sound Tab')(
+    injectIntl(connect(
+        mapStateToProps,
+        mapDispatchToProps
+    )(SoundTab))
+);

src/lib/app-state-hoc.jsx

@@ -71,7 +71,7 @@ const AppStateHOC = function (WrappedComponent) {
             return (
                 <Provider store={this.store}>
                     <IntlProvider>
-                        <ErrorBoundary>
+                        <ErrorBoundary action="Top Level App">
                             <WrappedComponent {...this.props} />
                         </ErrorBoundary>
                     </IntlProvider>

src/lib/error-boundary-hoc.jsx

+import React from 'react';
+import ErrorBoundary from '../containers/error-boundary.jsx';
+
+/*
+ * Higher Order Component to provide error boundary for wrapped component.
+ * A curried function, call like errorHOC(<tracking label>)(<Component>).
+ * @param {string} action - Label for GA tracking of errors.
+ * @returns {function} a function that accepts a component to wrap.
+ */
+const ErrorBoundaryHOC = function (action){
+    /**
+     * The function to be called with a React component to wrap it.
+     * @param {React.Component} WrappedComponent - Component to wrap with an error boundary.
+     * @returns {React.Component} the component wrapped with an error boundary.
+     */
+    return function (WrappedComponent) {
+        const ErrorBoundaryWrapper = props => (
+            <ErrorBoundary action={action}>
+                <WrappedComponent {...props} />
+            </ErrorBoundary>
+        );
+        return ErrorBoundaryWrapper;
+    };
+};
+
+export default ErrorBoundaryHOC;

src/lib/supported-browser.js

+import platform from 'platform';
+
+/**
+ * Helper function to determine if the browser is supported.
+ * @returns {boolean} False if the platform is definitely not supported.
+ */
+export default function () {
+    if (platform.name === 'IE' ||
+        platform.name === 'Opera' ||
+        platform.name === 'Opera Mini' ||
+        platform.name === 'Silk') {
+        return false;
+    }
+    // @todo Should also test for versions of supported browsers
+    return true;
+}