From 5fa9e79fb7c5dd1c9b3bd3866076352a9d3e7502 Mon Sep 17 00:00:00 2001
From: Paul Kaplan <pkaplan@media.mit.edu>
Date: Thu, 3 May 2018 10:09:08 -0400
Subject: [PATCH] Use different errors for unsupported browsers.

Also extract the "supportedBrowser" logic into a helper (rule of 3)
---
 src/containers/error-boundary.jsx | 36 ++++++++++++++++++++-----------
 src/containers/preview-modal.jsx  | 14 ++----------
 src/lib/app-state-hoc.jsx         |  2 +-
 src/lib/supported-browser.js      | 16 ++++++++++++++
 4 files changed, 42 insertions(+), 26 deletions(-)
 create mode 100644 src/lib/supported-browser.js

diff --git a/src/containers/error-boundary.jsx b/src/containers/error-boundary.jsx
index 0c8552857..c140d0ee7 100644
--- a/src/containers/error-boundary.jsx
+++ b/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
 };
 
diff --git a/src/containers/preview-modal.jsx b/src/containers/preview-modal.jsx
index 2ed9e75c5..a1a8a3c54 100644
--- a/src/containers/preview-modal.jsx
+++ b/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}
diff --git a/src/lib/app-state-hoc.jsx b/src/lib/app-state-hoc.jsx
index 6070c436b..91d4069f0 100644
--- a/src/lib/app-state-hoc.jsx
+++ b/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>
diff --git a/src/lib/supported-browser.js b/src/lib/supported-browser.js
new file mode 100644
index 000000000..507fb1dcf
--- /dev/null
+++ b/src/lib/supported-browser.js
@@ -0,0 +1,16 @@
+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;
+}
-- 
GitLab