Skip to content

Commit

Permalink
Show clear and actionable query timeout error message (#2763)
Browse files Browse the repository at this point in the history
* Show clear and actionable query timeout error message

1. Instead of waiting for a long time or server-side response 504 Gateway timeout, explore view now add a query timeout threshold. After timeout it will show specific querytimeout message.
2. fix alert box close button position.

* Show clear and actionable query timeout error message

1. Instead of waiting for a long time or server-side response 504 Gateway timeout, explore view now add a query timeout threshold. After timeout it will show specific querytimeout message.
2. fix alert box close button position.
3. fix a linting error.
  • Loading branch information
Grace Guo authored May 16, 2017
1 parent 28ac350 commit 960b26c
Show file tree
Hide file tree
Showing 9 changed files with 62 additions and 13 deletions.
4 changes: 2 additions & 2 deletions superset/assets/javascripts/SqlLab/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import thunkMiddleware from 'redux-thunk';

import { getInitialState, sqlLabReducer } from './reducers';
import { initEnhancer } from '../reduxUtils';
import { initJQueryAjaxCSRF } from '../modules/utils';
import { initJQueryAjax } from '../modules/utils';
import App from './components/App';
import { appSetup } from '../common';

Expand All @@ -15,7 +15,7 @@ import './reactable-pagination.css';
import '../components/FilterableTable/FilterableTableStyles.css';

appSetup();
initJQueryAjaxCSRF();
initJQueryAjax();

const appContainer = document.getElementById('app');
const bootstrapData = JSON.parse(appContainer.getAttribute('data-bootstrap'));
Expand Down
1 change: 1 addition & 0 deletions superset/assets/javascripts/constants.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const QUERY_TIMEOUT_THRESHOLD = 60000;
2 changes: 1 addition & 1 deletion superset/assets/javascripts/dashboard/Dashboard.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ export function dashboardContainer(dashboard, datasources, userid) {

$(document).ready(() => {
// Getting bootstrapped data from the DOM
utils.initJQueryAjaxCSRF();
utils.initJQueryAjax();
const dashboardData = $('.dashboard').data('bootstrap');

const state = getInitialState(dashboardData);
Expand Down
26 changes: 20 additions & 6 deletions superset/assets/javascripts/explorev2/actions/exploreActions.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* eslint camelcase: 0 */
import { getExploreUrl } from '../exploreUtils';
import { QUERY_TIMEOUT_THRESHOLD } from '../../constants';

const $ = window.$ = require('jquery');

Expand Down Expand Up @@ -150,6 +151,11 @@ export function chartUpdateStopped(queryRequest) {
return { type: CHART_UPDATE_STOPPED };
}

export const CHART_UPDATE_TIMEOUT = 'CHART_UPDATE_TIMEOUT';
export function chartUpdateTimeout(statusText) {
return { type: CHART_UPDATE_TIMEOUT, statusText };
}

export const CHART_UPDATE_FAILED = 'CHART_UPDATE_FAILED';
export function chartUpdateFailed(queryResponse) {
return { type: CHART_UPDATE_FAILED, queryResponse };
Expand Down Expand Up @@ -234,12 +240,20 @@ export const RUN_QUERY = 'RUN_QUERY';
export function runQuery(formData, force = false) {
return function (dispatch) {
const url = getExploreUrl(formData, 'json', force);
const queryRequest = $.getJSON(url, function (queryResponse) {
dispatch(chartUpdateSucceeded(queryResponse));
}).fail(function (err) {
if (err.statusText !== 'abort') {
dispatch(chartUpdateFailed(err.responseJSON));
}
const queryRequest = $.ajax({
url,
dataType: 'json',
success(queryResponse) {
dispatch(chartUpdateSucceeded(queryResponse));
},
error(err) {
if (err.statusText === 'timeout') {
dispatch(chartUpdateTimeout(err.statusText));
} else if (err.statusText !== 'abort') {
dispatch(chartUpdateFailed(err.responseJSON));
}
},
timeout: QUERY_TIMEOUT_THRESHOLD,
});
dispatch(chartUpdateStarted(queryRequest));
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,12 +169,14 @@ class ChartContainer extends React.PureComponent {
renderAlert() {
const msg = (
<div>
{this.props.alert}
<i
className="fa fa-close pull-right"
onClick={this.removeAlert.bind(this)}
style={{ cursor: 'pointer' }}
/>
<p
dangerouslySetInnerHTML={{ __html: this.props.alert }}
/>
</div>);
return (
<div>
Expand Down
4 changes: 2 additions & 2 deletions superset/assets/javascripts/explorev2/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ import { now } from '../modules/dates';
import { initEnhancer } from '../reduxUtils';
import AlertsWrapper from '../components/AlertsWrapper';
import { getControlsState, getFormDataFromControls } from './stores/store';
import { initJQueryAjaxCSRF } from '../modules/utils';
import { initJQueryAjax } from '../modules/utils';
import ExploreViewContainer from './components/ExploreViewContainer';
import { exploreReducer } from './reducers/exploreReducer';
import { appSetup } from '../common';
import './main.css';

appSetup();
initJQueryAjaxCSRF();
initJQueryAjax();

const exploreViewContainer = document.getElementById('js-explore-view-container');
const bootstrapData = JSON.parse(exploreViewContainer.getAttribute('data-bootstrap'));
Expand Down
11 changes: 11 additions & 0 deletions superset/assets/javascripts/explorev2/reducers/exploreReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import { getControlsState, getFormDataFromControls } from '../stores/store';
import * as actions from '../actions/exploreActions';
import { now } from '../../modules/dates';
import { QUERY_TIMEOUT_THRESHOLD } from '../../constants';

export const exploreReducer = function (state, action) {
const actionHandlers = {
Expand Down Expand Up @@ -109,6 +110,16 @@ export const exploreReducer = function (state, action) {
triggerQuery: true,
});
},
[actions.CHART_UPDATE_TIMEOUT]() {
return Object.assign({}, state, {
chartStatus: 'failed',
chartAlert: '<strong>Query timeout</strong> - visualization query are set to timeout at ' +
`${QUERY_TIMEOUT_THRESHOLD / 1000} seconds. ` +
'Perhaps your data has grown, your database is under unusual load, ' +
'or you are simply querying a data source that is to large to be processed within the timeout range. ' +
'If that is the case, we recommend that you summarize your data further.',
});
},
[actions.CHART_UPDATE_FAILED]() {
return Object.assign({}, state, {
chartStatus: 'failed',
Expand Down
2 changes: 1 addition & 1 deletion superset/assets/javascripts/modules/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ export function customizeToolTip(chart, xAxisFormatter, yAxisFormatters) {
});
}

export function initJQueryAjaxCSRF() {
export function initJQueryAjax() {
// Works in conjunction with a Flask-WTF token as described here:
// http://flask-wtf.readthedocs.io/en/stable/csrf.html#javascript-requests
const token = $('input#csrf_token').val();
Expand Down
21 changes: 21 additions & 0 deletions superset/assets/spec/javascripts/explorev2/actions_spec.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { it, describe } from 'mocha';
import { expect } from 'chai';
import sinon from 'sinon';
import $ from 'jquery';
import * as actions from '../../../javascripts/explorev2/actions/exploreActions';
import * as exploreUtils from '../../../javascripts/explorev2/exploreUtils';
import { defaultState } from '../../../javascripts/explorev2/stores/store';
import { exploreReducer } from '../../../javascripts/explorev2/reducers/exploreReducer';

Expand All @@ -16,3 +19,21 @@ describe('reducers', () => {
expect(newState.controls.show_legend.value).to.equal(true);
});
});

describe('runQuery', () => {
it('should handle query timeout', () => {
const dispatch = sinon.spy();
const urlStub = sinon.stub(exploreUtils, 'getExploreUrl', () => ('mockURL'));
const ajaxStub = sinon.stub($, 'ajax');
ajaxStub.yieldsTo('error', { statusText: 'timeout' });

const request = actions.runQuery({});
request(dispatch);

expect(dispatch.callCount).to.equal(2);
expect(dispatch.args[0][0].type).to.equal(actions.CHART_UPDATE_TIMEOUT);

urlStub.restore();
ajaxStub.restore();
});
});

0 comments on commit 960b26c

Please sign in to comment.