Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add error boundary to catch errors in visualizations #4518

Merged
merged 4 commits into from
Jan 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
gabrieldutra marked this conversation as resolved.
Show resolved Hide resolved
}
};
}

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