From 49346050439d66179e1de19c0bffa4ced3a8dbe6 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Thu, 8 Nov 2018 09:38:10 -0800 Subject: [PATCH] Make stacktraces available in many more cases (#6299) * Wrap with It appears that since the introduction of , 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 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 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 * Getting more stack traces * Adressing comments --- superset/assets/src/chart/Chart.jsx | 10 ++++--- superset/assets/src/chart/chartAction.js | 4 +-- superset/assets/src/chart/chartReducer.js | 4 +++ .../explore/components/ExploreChartPanel.jsx | 1 + .../TimeTable/transformProps.js | 2 +- superset/views/api.py | 4 +-- superset/views/base.py | 26 ++++++++++++------- superset/views/core.py | 10 +++---- superset/viz.py | 1 - 9 files changed, 37 insertions(+), 25 deletions(-) diff --git a/superset/assets/src/chart/Chart.jsx b/superset/assets/src/chart/Chart.jsx index d0d3461847e9d..d2abb6ba2aaa3 100644 --- a/superset/assets/src/chart/Chart.jsx +++ b/superset/assets/src/chart/Chart.jsx @@ -27,6 +27,7 @@ const propTypes = { // state chartAlert: PropTypes.string, chartStatus: PropTypes.string, + chartStackTrace: PropTypes.string, queryResponse: PropTypes.object, triggerQuery: PropTypes.bool, refreshOverlayVisible: PropTypes.bool, @@ -90,10 +91,10 @@ class Chart extends React.PureComponent { }); } - handleRenderFailure(e) { + handleRenderFailure(error, info) { const { actions, chartId } = this.props; - console.warn(e); // eslint-disable-line - actions.chartRenderingFailed(e.toString(), chartId); + console.warn(error); // eslint-disable-line + actions.chartRenderingFailed(error.toString(), chartId, info ? info.componentStack : null); } prepareChartProps() { @@ -153,6 +154,7 @@ class Chart extends React.PureComponent { width, height, chartAlert, + chartStackTrace, chartStatus, errorMessage, onDismissRefreshOverlay, @@ -183,7 +185,7 @@ class Chart extends React.PureComponent { )} diff --git a/superset/assets/src/chart/chartAction.js b/superset/assets/src/chart/chartAction.js index fb4ffd665641f..b816d87017d4d 100644 --- a/superset/assets/src/chart/chartAction.js +++ b/superset/assets/src/chart/chartAction.js @@ -35,8 +35,8 @@ export function chartUpdateFailed(queryResponse, key) { } export const CHART_RENDERING_FAILED = 'CHART_RENDERING_FAILED'; -export function chartRenderingFailed(error, key) { - return { type: CHART_RENDERING_FAILED, error, key }; +export function chartRenderingFailed(error, key, stackTrace) { + return { type: CHART_RENDERING_FAILED, error, key, stackTrace }; } export const CHART_RENDERING_SUCCEEDED = 'CHART_RENDERING_SUCCEEDED'; diff --git a/superset/assets/src/chart/chartReducer.js b/superset/assets/src/chart/chartReducer.js index f6b608cca613f..3936f9c83be1b 100644 --- a/superset/assets/src/chart/chartReducer.js +++ b/superset/assets/src/chart/chartReducer.js @@ -7,6 +7,7 @@ export const chart = { id: 0, chartAlert: null, chartStatus: 'loading', + chartStackTrace: null, chartUpdateEndTime: null, chartUpdateStartTime: 0, latestQueryFormData: {}, @@ -35,6 +36,7 @@ export default function chartReducer(charts = {}, action) { return { ...state, chartStatus: 'loading', + chartStackTrace: null, chartAlert: null, chartUpdateEndTime: null, chartUpdateStartTime: now(), @@ -56,6 +58,7 @@ export default function chartReducer(charts = {}, action) { [actions.CHART_RENDERING_FAILED](state) { return { ...state, chartStatus: 'failed', + chartStackTrace: action.stackTrace, chartAlert: t('An error occurred while rendering the visualization: %s', action.error), }; }, @@ -78,6 +81,7 @@ export default function chartReducer(charts = {}, action) { chartAlert: action.queryResponse ? action.queryResponse.error : t('Network error.'), chartUpdateEndTime: now(), queryResponse: action.queryResponse, + chartStackTrace: action.queryResponse ? action.queryResponse.stacktrace : null, }; }, [actions.TRIGGER_QUERY](state) { diff --git a/superset/assets/src/explore/components/ExploreChartPanel.jsx b/superset/assets/src/explore/components/ExploreChartPanel.jsx index efff4a0311be7..1cdc94cc9d889 100644 --- a/superset/assets/src/explore/components/ExploreChartPanel.jsx +++ b/superset/assets/src/explore/components/ExploreChartPanel.jsx @@ -43,6 +43,7 @@ class ExploreChartPanel extends React.PureComponent { height={parseInt(this.props.height, 10) - headerHeight} annotationData={chart.annotationData} chartAlert={chart.chartAlert} + chartStackTrace={chart.chartStackTrace} chartId={chart.id} chartStatus={chart.chartStatus} datasource={this.props.datasource} diff --git a/superset/assets/src/visualizations/TimeTable/transformProps.js b/superset/assets/src/visualizations/TimeTable/transformProps.js index d52088b99d03e..8492ce2ac9860 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 = 0, + columnCollection = [], groupby, metrics, url, diff --git a/superset/views/api.py b/superset/views/api.py index c1d27144bf34c..5f684f3cf9141 100644 --- a/superset/views/api.py +++ b/superset/views/api.py @@ -8,13 +8,13 @@ from superset import appbuilder, security_manager from superset.common.query_context import QueryContext from superset.models.core import Log -from .base import api, BaseSupersetView, data_payload_response, handle_superset_exception +from .base import api, BaseSupersetView, data_payload_response, handle_api_exception class Api(BaseSupersetView): @Log.log_this @api - @handle_superset_exception + @handle_api_exception @has_access_api @expose('/v1/query/', methods=['POST']) def query(self): diff --git a/superset/views/base.py b/superset/views/base.py index 657242f372174..31ab1cefc3a66 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -86,7 +86,7 @@ def wraps(self, *args, **kwargs): return functools.update_wrapper(wraps, f) -def handle_superset_exception(f): +def handle_api_exception(f): """ A decorator to catch superset exceptions. Use it after the @api decorator above so superset exception handler is triggered before the handler for generic exceptions. @@ -94,15 +94,21 @@ def handle_superset_exception(f): def wraps(self, *args, **kwargs): try: return f(self, *args, **kwargs) - except SupersetSecurityException as sse: - logging.exception(sse) - return json_error_response(utils.error_msg_from_exception(sse), - status=sse.status, - link=sse.link) - except SupersetException as se: - logging.exception(se) - return json_error_response(utils.error_msg_from_exception(se), - status=se.status) + except SupersetSecurityException as e: + logging.exception(e) + return json_error_response(utils.error_msg_from_exception(e), + status=e.status, + stacktrace=traceback.format_exc(), + link=e.link) + except SupersetException as e: + logging.exception(e) + return json_error_response(utils.error_msg_from_exception(e), + stacktrace=traceback.format_exc(), + status=e.status) + except Exception as e: + logging.exception(e) + return json_error_response(utils.error_msg_from_exception(e), + stacktrace=traceback.format_exc()) return functools.update_wrapper(wraps, f) diff --git a/superset/views/core.py b/superset/views/core.py index ef4c4ec2f524e..9b79b7574c720 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -46,7 +46,7 @@ api, BaseSupersetView, check_ownership, CsvResponse, data_payload_response, DeleteMixin, generate_download_headers, - get_error_msg, handle_superset_exception, json_error_response, json_success, + get_error_msg, handle_api_exception, json_error_response, json_success, SupersetFilter, SupersetModelView, YamlExportMixin, ) from .utils import bootstrap_user_data @@ -1108,7 +1108,6 @@ def get_samples(self, viz_obj): 'data': viz_obj.get_samples(), }) - @handle_superset_exception def generate_json( self, datasource_type, datasource_id, form_data, csv=False, query=False, force=False, results=False, @@ -1176,6 +1175,7 @@ def annotation_json(self, layer_id): @log_this @api @has_access_api + @handle_api_exception @expose('/explore_json///', methods=['GET', 'POST']) @expose('/explore_json/', methods=['GET', 'POST']) def explore_json(self, datasource_type=None, datasource_id=None): @@ -1353,7 +1353,7 @@ def explore(self, datasource_type=None, datasource_id=None): standalone_mode=standalone) @api - @handle_superset_exception + @handle_api_exception @has_access_api @expose('/filter////') def filter(self, datasource_type, datasource_id, column): @@ -2609,7 +2609,7 @@ def csv(self, client_id): return response @api - @handle_superset_exception + @handle_api_exception @has_access @expose('/fetch_datasource_metadata') @log_this @@ -2800,7 +2800,7 @@ def sqllab(self): ) @api - @handle_superset_exception + @handle_api_exception @has_access_api @expose('/slice_query//') def slice_query(self, slice_id): diff --git a/superset/viz.py b/superset/viz.py index fba68aef1d02a..91b48f2037dc3 100644 --- a/superset/viz.py +++ b/superset/viz.py @@ -445,7 +445,6 @@ def get_df_payload(self, query_obj=None, **kwargs): logging.warning('Could not cache key {}'.format(cache_key)) logging.exception(e) cache.delete(cache_key) - return { 'cache_key': self._any_cache_key, 'cached_dttm': self._any_cached_dttm,