This is an automated email from the ASF dual-hosted git repository.
beto pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/master by this push:
new 4ce475f Wrap <LoadableRenderer /> with <ErrorBoundary /> (#6294)
4ce475f is described below
commit 4ce475f2872fc4fd3a459cd1ec5d32bfad05dac6
Author: Maxime Beauchemin <maximebeauche...@gmail.com>
AuthorDate: Wed Nov 7 17:11:52 2018 -0800
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 | 24 +++++++-----
superset/assets/src/components/CachedLabel.jsx | 2 +-
superset/assets/src/components/ErrorBoundary.jsx | 45 ++++++++++++++++++++++
.../assets/src/components/StackTraceMessage.jsx | 25 +++++-------
.../src/visualizations/TimeTable/TimeTable.jsx | 2 +-
.../src/visualizations/TimeTable/transformProps.js | 2 +-
6 files changed, 72 insertions(+), 28 deletions(-)
diff --git a/superset/assets/src/chart/Chart.jsx
b/superset/assets/src/chart/Chart.jsx
index a4be8dc..d0d3461 100644
--- a/superset/assets/src/chart/Chart.jsx
+++ b/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>
);
}
diff --git a/superset/assets/src/components/CachedLabel.jsx
b/superset/assets/src/components/CachedLabel.jsx
index 845eac5..f529cde 100644
--- a/superset/assets/src/components/CachedLabel.jsx
+++ b/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>);
}
diff --git a/superset/assets/src/components/ErrorBoundary.jsx
b/superset/assets/src/components/ErrorBoundary.jsx
new file mode 100644
index 0000000..1be2d2e
--- /dev/null
+++ b/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;
diff --git a/superset/assets/src/components/StackTraceMessage.jsx
b/superset/assets/src/components/StackTraceMessage.jsx
index 9253108..d3a0f68 100644
--- a/superset/assets/src/components/StackTraceMessage.jsx
+++ b/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>
}
diff --git a/superset/assets/src/visualizations/TimeTable/TimeTable.jsx
b/superset/assets/src/visualizations/TimeTable/TimeTable.jsx
index c414a7c..0eaafc7 100644
--- a/superset/assets/src/visualizations/TimeTable/TimeTable.jsx
+++ b/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;
diff --git a/superset/assets/src/visualizations/TimeTable/transformProps.js
b/superset/assets/src/visualizations/TimeTable/transformProps.js
index d880fae..d52088b 100644
--- a/superset/assets/src/visualizations/TimeTable/transformProps.js
+++ b/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,
