Wrap <LoadableRenderer /> with <ErrorBoundary /> (#6294)

* Wrap <LoadableRenderer /> with <ErrorBoundary />

It appears that since the introduction of <SuperChart />, errors in the
visualization javascript (which are somewhat common and expected,
especially as we'll support plugins) were not handled and the whole
page would throw and go missing.

Here I'm introducing a new <ErrorBoundary /> component that elegantly
wraps other
components and handles errors. It's inspired by:
https://reactjs.org/docs/error-boundaries.html

The default behavior of the component is to simply surface the error
as an <Alert bsStyle="danger" /> and exposes the React stacktrace
when clicking on the error.

It's also possible to use component and pass an onError handler and not
show the default message.

This also fixes some minor bugs in TimeTable.

* Addressing comments

superset/assets/src/chart/Chart.jsx

@@ -9,6 +9,7 @@ import RefreshChartOverlay from '../components/RefreshChartOverlay';
 import StackTraceMessage from '../components/StackTraceMessage';
 import ChartProps from '../visualizations/core/models/ChartProps';
 import SuperChart from '../visualizations/core/components/SuperChart';
+import ErrorBoundary from '../components/ErrorBoundary';
 import './chart.css';
 
 const propTypes = {
@@ -92,7 +93,7 @@ class Chart extends React.PureComponent {
   handleRenderFailure(e) {
     const { actions, chartId } = this.props;
     console.warn(e); // eslint-disable-line
-    actions.chartRenderingFailed(e, chartId);
+    actions.chartRenderingFailed(e.toString(), chartId);
   }
 
   prepareChartProps() {
@@ -181,7 +182,8 @@ class Chart extends React.PureComponent {
         {chartAlert && (
           <StackTraceMessage
             message={chartAlert}
-            queryResponse={queryResponse}
+            link={queryResponse ? queryResponse.link : null}
+            stackTrace={queryResponse ? queryResponse.stacktrace : null}
           />
         )}
 
@@ -194,14 +196,16 @@ class Chart extends React.PureComponent {
           />
         )}
 
-        <SuperChart
-          className={`slice_container ${snakeCase(vizType)} ${isFaded ? ' faded' : ''}`}
-          chartType={vizType}
-          chartProps={skipChartRendering ? null : this.prepareChartProps()}
-          onRenderSuccess={this.handleRenderSuccess}
-          onRenderFailure={this.handleRenderFailure}
-          skipRendering={skipChartRendering}
-        />
+        <ErrorBoundary onError={this.handleRenderFailure} showMessage={false}>
+          <SuperChart
+            className={`slice_container ${snakeCase(vizType)} ${isFaded ? ' faded' : ''}`}
+            chartType={vizType}
+            chartProps={skipChartRendering ? null : this.prepareChartProps()}
+            onRenderSuccess={this.handleRenderSuccess}
+            onRenderFailure={this.handleRenderFailure}
+            skipRendering={skipChartRendering}
+          />
+        </ErrorBoundary>
       </div>
     );
   }

superset/assets/src/components/CachedLabel.jsx

@@ -60,7 +60,7 @@ class CacheLabel extends React.PureComponent {
           onMouseOver={this.mouseOver.bind(this)}
           onMouseOut={this.mouseOut.bind(this)}
         >
-          cached <i className="fa fa-refresh" />
+          {t('cached')} <i className="fa fa-refresh" />
         </Label>
       </TooltipWrapper>);
   }

superset/assets/src/components/ErrorBoundary.jsx

@@ -0,0 +1,45 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+import { t } from '@superset-ui/translation';
+import StackTraceMessage from './StackTraceMessage';
+
+const propTypes = {
+  children: PropTypes.node.isRequired,
+  onError: PropTypes.func,
+  showMessage: PropTypes.bool,
+};
+const defaultProps = {
+  onError: () => {},
+  showMessage: true,
+};
+
+export default class ErrorBoundary extends React.Component {
+  constructor(props) {
+    super(props);
+    this.state = { error: null, info: null };
+  }
+
+  componentDidCatch(error, info) {
+    this.props.onError(error, info);
+    this.setState({ error, info });
+  }
+
+  render() {
+    const { error, info } = this.state;
+    if (error) {
+      const firstLine = error ? error.toString() : null;
+      const message = (
+        <span>
+          <strong>{t('Unexpected error')}</strong>{firstLine ? `: ${firstLine}` : ''}
+        </span>);
+      if (this.props.showMessage) {
+        return (
+          <StackTraceMessage message={message} stackTrace={info ? info.componentStack : null} />);
+      }
+      return null;
+    }
+    return this.props.children;
+  }
+}
+ErrorBoundary.propTypes = propTypes;
+ErrorBoundary.defaultProps = defaultProps;

superset/assets/src/components/StackTraceMessage.jsx

@@ -4,12 +4,15 @@ import PropTypes from 'prop-types';
 import { Alert, Collapse } from 'react-bootstrap';
 
 const propTypes = {
-  message: PropTypes.string,
-  queryResponse: PropTypes.object,
+  message: PropTypes.node.isRequired,
+  link: PropTypes.string,
+  stackTrace: PropTypes.string,
   showStackTrace: PropTypes.bool,
 };
 const defaultProps = {
   showStackTrace: false,
+  link: null,
+  stackTrace: null,
 };
 
 class StackTraceMessage extends React.PureComponent {
@@ -21,25 +24,17 @@ class StackTraceMessage extends React.PureComponent {
     };
   }
 
-  hasTrace() {
-    return this.props.queryResponse && this.props.queryResponse.stacktrace;
-  }
-
-  hasLink() {
-    return this.props.queryResponse && this.props.queryResponse.link;
-  }
-
   render() {
     return (
-      <div className={`stack-trace-container${this.hasTrace() ? ' has-trace' : ''}`}>
+      <div className={`stack-trace-container${this.props.stackTrace ? ' has-trace' : ''}`}>
         <Alert
           bsStyle="warning"
           onClick={() => this.setState({ showStackTrace: !this.state.showStackTrace })}
         >
           {this.props.message}
-          {this.hasLink() &&
+          {this.props.link &&
           <a
-            href={this.props.queryResponse.link}
+            href={this.props.link}
             target="_blank"
             rel="noopener noreferrer"
           >
@@ -47,10 +42,10 @@ class StackTraceMessage extends React.PureComponent {
           </a>
        }
         </Alert>
-        {this.hasTrace() &&
+        {this.props.stackTrace &&
           <Collapse in={this.state.showStackTrace}>
             <pre>
-              {this.props.queryResponse.stacktrace}
+              {this.props.stackTrace}
             </pre>
           </Collapse>
         }

superset/assets/src/visualizations/TimeTable/TimeTable.jsx

@@ -226,7 +226,7 @@ class TimeTable extends React.PureComponent {
       .map(time => ({ ...data[time], time }));
     const reversedEntries = entries.concat().reverse();
 
-    const defaultSort = rowType === 'column' ? {
+    const defaultSort = rowType === 'column' && columnConfigs.length ? {
       column: columnConfigs[0].key,
       direction: 'desc',
     } : false;

superset/assets/src/visualizations/TimeTable/transformProps.js

@@ -1,7 +1,7 @@
 export default function transformProps(chartProps) {
   const { height, datasource, formData, payload } = chartProps;
   const {
-    columnCollection,
+    columnCollection = 0,
     groupby,
     metrics,
     url,