Skip to content

Commit

Permalink
Add error boundary to catch errors in visualizations (#4518)
Browse files Browse the repository at this point in the history
* Add error boundary to catch errors in visualizations

* Fix: Funnel crash when step column is date/time

* CR1

* CR2
  • Loading branch information
kravets-levko authored and arikfr committed Jan 6, 2020
1 parent 260bfca commit fc9e8fe
Show file tree
Hide file tree
Showing 5 changed files with 178 additions and 49 deletions.
71 changes: 71 additions & 0 deletions client/app/components/ErrorBoundary.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { isFunction } from "lodash";
import React from "react";
import PropTypes from "prop-types";
import debug from "debug";
import Alert from "antd/lib/alert";

const logger = debug("redash:errors");

export const ErrorBoundaryContext = React.createContext({
handleError: error => {
throw error;
},
reset: () => {},
});

export function ErrorMessage({ children }) {
return <Alert message={children} type="error" showIcon />;
}

ErrorMessage.propTypes = {
children: PropTypes.node,
};

ErrorMessage.defaultProps = {
children: "Something went wrong.",
};

export default class ErrorBoundary extends React.Component {
static propTypes = {
children: PropTypes.node,
renderError: PropTypes.func, // error => ReactNode
};

static defaultProps = {
children: null,
renderError: null,
};

state = { error: null };

handleError = error => {
this.setState(this.constructor.getDerivedStateFromError(error));
this.componentDidCatch(error, null);
};

reset = () => {
this.setState({ error: null });
};

static getDerivedStateFromError(error) {
return { error };
}

componentDidCatch(error, errorInfo) {
logger(error, errorInfo);
}

render() {
const { renderError, children } = this.props;
const { error } = this.state;

if (error) {
if (isFunction(renderError)) {
return renderError(error);
}
return <ErrorMessage />;
}

return <ErrorBoundaryContext.Provider value={this}>{children}</ErrorBoundaryContext.Provider>;
}
}
29 changes: 21 additions & 8 deletions client/app/visualizations/EditVisualizationDialog.jsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { isEqual, extend, map, sortBy, findIndex, filter, pick } from "lodash";
import React, { useState, useMemo } from "react";
import React, { useState, useMemo, useRef, useEffect } from "react";
import PropTypes from "prop-types";
import Modal from "antd/lib/modal";
import Select from "antd/lib/select";
import Input from "antd/lib/input";
import * as Grid from "antd/lib/grid";
import { wrap as wrapDialog, DialogPropType } from "@/components/DialogWrapper";
import ErrorBoundary, { ErrorMessage } from "@/components/ErrorBoundary";
import Filters, { filterData } from "@/components/Filters";
import notification from "@/services/notification";
import { Visualization } from "@/services/visualization";
Expand Down Expand Up @@ -63,6 +64,8 @@ function confirmDialogClose(isDirty) {
}

function EditVisualizationDialog({ dialog, visualization, query, queryResult }) {
const errorHandlerRef = useRef();

const isNew = !visualization;

const data = useQueryResult(queryResult);
Expand Down Expand Up @@ -94,6 +97,12 @@ function EditVisualizationDialog({ dialog, visualization, query, queryResult })

const [saveInProgress, setSaveInProgress] = useState(false);

useEffect(() => {
if (errorHandlerRef.current) {
errorHandlerRef.current.reset();
}
}, [data, options]);

function onTypeChanged(newType) {
setType(newType);

Expand Down Expand Up @@ -188,13 +197,17 @@ function EditVisualizationDialog({ dialog, visualization, query, queryResult })
</label>
<Filters filters={filters} onChange={setFilters} />
<div className="scrollbox" data-test="VisualizationPreview">
<Renderer
data={filteredData}
options={options}
visualizationName={name}
onOptionsChange={onOptionsChanged}
context="query"
/>
<ErrorBoundary
ref={errorHandlerRef}
renderError={() => <ErrorMessage>Error while rendering visualization.</ErrorMessage>}>
<Renderer
data={filteredData}
options={options}
visualizationName={name}
onOptionsChange={onOptionsChanged}
context="query"
/>
</ErrorBoundary>
</div>
</Grid.Col>
</Grid.Row>
Expand Down
32 changes: 22 additions & 10 deletions client/app/visualizations/VisualizationRenderer.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import React, { useState, useMemo, useEffect, useRef } from "react";
import PropTypes from "prop-types";
import { react2angular } from "react2angular";
import useQueryResult from "@/lib/hooks/useQueryResult";
import ErrorBoundary, { ErrorMessage } from "@/components/ErrorBoundary";
import Filters, { FiltersType, filterData } from "@/components/Filters";
import { registeredVisualizations, VisualizationType } from "./index";

Expand All @@ -28,6 +29,7 @@ export function VisualizationRenderer(props) {
const data = useQueryResult(props.queryResult);
const [filters, setFilters] = useState(data.filters);
const lastOptions = useRef();
const errorHandlerRef = useRef();

// Reset local filters when query results updated
useEffect(() => {
Expand Down Expand Up @@ -60,18 +62,28 @@ export function VisualizationRenderer(props) {
}
lastOptions.current = options;

useEffect(() => {
if (errorHandlerRef.current) {
errorHandlerRef.current.reset();
}
}, [props.visualization.options, data]);

return (
<div className="visualization-renderer">
{showFilters && <Filters filters={filters} onChange={setFilters} />}
<div className="visualization-renderer-wrapper">
<Renderer
key={`visualization${visualization.id}`}
options={options}
data={filteredData}
visualizationName={visualization.name}
context={props.context}
/>
</div>
<ErrorBoundary
ref={errorHandlerRef}
renderError={() => <ErrorMessage>Error while rendering visualization.</ErrorMessage>}>
{showFilters && <Filters filters={filters} onChange={setFilters} />}
<div className="visualization-renderer-wrapper">
<Renderer
key={`visualization${visualization.id}`}
options={options}
data={filteredData}
visualizationName={visualization.name}
context={props.context}
/>
</div>
</ErrorBoundary>
</div>
);
}
Expand Down
81 changes: 52 additions & 29 deletions client/app/visualizations/chart/Renderer/PlotlyChart.jsx
Original file line number Diff line number Diff line change
@@ -1,42 +1,65 @@
import { isArray, isObject } from "lodash";
import React, { useState, useEffect } from "react";
import React, { useState, useEffect, useContext } from "react";
import { ErrorBoundaryContext } from "@/components/ErrorBoundary";
import { RendererPropTypes } from "@/visualizations";
import resizeObserver from "@/services/resizeObserver";

import getChartData from "../getChartData";
import { Plotly, prepareData, prepareLayout, updateData, applyLayoutFixes } from "../plotly";

function catchErrors(func, errorHandler) {
return (...args) => {
try {
return func(...args);
} catch (error) {
errorHandler.handleError(error);
}
};
}

export default function PlotlyChart({ options, data }) {
const [container, setContainer] = useState(null);
const errorHandler = useContext(ErrorBoundaryContext);

useEffect(() => {
if (container) {
const plotlyOptions = { showLink: false, displaylogo: false };

const chartData = getChartData(data.rows, options);
const plotlyData = prepareData(chartData, options);
const plotlyLayout = prepareLayout(container, options, plotlyData);

// It will auto-purge previous graph
Plotly.newPlot(container, plotlyData, plotlyLayout, plotlyOptions).then(() => {
applyLayoutFixes(container, plotlyLayout, (e, u) => Plotly.relayout(e, u));
});

container.on("plotly_restyle", updates => {
// This event is triggered if some plotly data/layout has changed.
// We need to catch only changes of traces visibility to update stacking
if (isArray(updates) && isObject(updates[0]) && updates[0].visible) {
updateData(plotlyData, options);
Plotly.relayout(container, plotlyLayout);
}
});

const unwatch = resizeObserver(container, () => {
applyLayoutFixes(container, plotlyLayout, (e, u) => Plotly.relayout(e, u));
});
return unwatch;
}
}, [options, data, container]);
useEffect(
catchErrors(() => {
if (container) {
const plotlyOptions = { showLink: false, displaylogo: false };

const chartData = getChartData(data.rows, options);
const plotlyData = prepareData(chartData, options);
const plotlyLayout = prepareLayout(container, options, plotlyData);

// It will auto-purge previous graph
Plotly.newPlot(container, plotlyData, plotlyLayout, plotlyOptions).then(
catchErrors(() => {
applyLayoutFixes(container, plotlyLayout, (e, u) => Plotly.relayout(e, u));
}, errorHandler)
);

container.on(
"plotly_restyle",
catchErrors(updates => {
// This event is triggered if some plotly data/layout has changed.
// We need to catch only changes of traces visibility to update stacking
if (isArray(updates) && isObject(updates[0]) && updates[0].visible) {
updateData(plotlyData, options);
Plotly.relayout(container, plotlyLayout);
}
}, errorHandler)
);

const unwatch = resizeObserver(
container,
catchErrors(() => {
applyLayoutFixes(container, plotlyLayout, (e, u) => Plotly.relayout(e, u));
}, errorHandler)
);
return unwatch;
}
}, errorHandler),
[options, data, container]
);

// Cleanup when component destroyed
useEffect(() => {
Expand Down
14 changes: 12 additions & 2 deletions client/app/visualizations/funnel/Renderer/prepareData.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,14 @@
import { map, maxBy, sortBy } from "lodash";
import { map, maxBy, sortBy, toString } from "lodash";
import moment from "moment";
import { clientConfig } from "@/services/auth";

function stepValueToString(value) {
if (moment.isMoment(value)) {
const format = clientConfig.dateTimeFormat || "DD/MM/YYYY HH:mm";
return value.format(format);
}
return toString(value);
}

export default function prepareData(rows, options) {
if (rows.length === 0 || !options.stepCol.colName || !options.valueCol.colName) {
Expand All @@ -14,7 +24,7 @@ export default function prepareData(rows, options) {
}

const data = map(rows, row => ({
step: row[options.stepCol.colName],
step: stepValueToString(row[options.stepCol.colName]),
value: parseFloat(row[options.valueCol.colName]) || 0.0,
}));

Expand Down

0 comments on commit fc9e8fe

Please sign in to comment.