mirror of
https://github.com/apache/superset.git
synced 2026-04-19 08:04:53 +00:00
Make stacktraces available in many more cases (#6299)
* 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 * Getting more stack traces * Adressing comments
This commit is contained in:
committed by
GitHub
parent
1fcfda4fdc
commit
4934605043
@@ -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 {
|
||||
<StackTraceMessage
|
||||
message={chartAlert}
|
||||
link={queryResponse ? queryResponse.link : null}
|
||||
stackTrace={queryResponse ? queryResponse.stacktrace : null}
|
||||
stackTrace={chartStackTrace}
|
||||
/>
|
||||
)}
|
||||
|
||||
|
||||
@@ -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';
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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}
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
export default function transformProps(chartProps) {
|
||||
const { height, datasource, formData, payload } = chartProps;
|
||||
const {
|
||||
columnCollection = 0,
|
||||
columnCollection = [],
|
||||
groupby,
|
||||
metrics,
|
||||
url,
|
||||
|
||||
@@ -8,13 +8,13 @@ from flask_appbuilder.security.decorators import has_access_api
|
||||
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):
|
||||
|
||||
@@ -86,7 +86,7 @@ def api(f):
|
||||
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)
|
||||
|
||||
|
||||
|
||||
@@ -46,7 +46,7 @@ from .base import (
|
||||
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 @@ class Superset(BaseSupersetView):
|
||||
'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 @@ class Superset(BaseSupersetView):
|
||||
@log_this
|
||||
@api
|
||||
@has_access_api
|
||||
@handle_api_exception
|
||||
@expose('/explore_json/<datasource_type>/<datasource_id>/', methods=['GET', 'POST'])
|
||||
@expose('/explore_json/', methods=['GET', 'POST'])
|
||||
def explore_json(self, datasource_type=None, datasource_id=None):
|
||||
@@ -1353,7 +1353,7 @@ class Superset(BaseSupersetView):
|
||||
standalone_mode=standalone)
|
||||
|
||||
@api
|
||||
@handle_superset_exception
|
||||
@handle_api_exception
|
||||
@has_access_api
|
||||
@expose('/filter/<datasource_type>/<datasource_id>/<column>/')
|
||||
def filter(self, datasource_type, datasource_id, column):
|
||||
@@ -2609,7 +2609,7 @@ class Superset(BaseSupersetView):
|
||||
return response
|
||||
|
||||
@api
|
||||
@handle_superset_exception
|
||||
@handle_api_exception
|
||||
@has_access
|
||||
@expose('/fetch_datasource_metadata')
|
||||
@log_this
|
||||
@@ -2800,7 +2800,7 @@ class Superset(BaseSupersetView):
|
||||
)
|
||||
|
||||
@api
|
||||
@handle_superset_exception
|
||||
@handle_api_exception
|
||||
@has_access_api
|
||||
@expose('/slice_query/<slice_id>/')
|
||||
def slice_query(self, slice_id):
|
||||
|
||||
@@ -445,7 +445,6 @@ class BaseViz(object):
|
||||
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,
|
||||
|
||||
Reference in New Issue
Block a user