diff --git a/superset/assets/spec/javascripts/explore/chartActions_spec.js b/superset/assets/spec/javascripts/chart/chartActions_spec.js similarity index 97% rename from superset/assets/spec/javascripts/explore/chartActions_spec.js rename to superset/assets/spec/javascripts/chart/chartActions_spec.js index 50635f12f0..21fbf90837 100644 --- a/superset/assets/spec/javascripts/explore/chartActions_spec.js +++ b/superset/assets/spec/javascripts/chart/chartActions_spec.js @@ -102,7 +102,7 @@ describe('chart actions', () => { }); it('should dispatch CHART_UPDATE_FAILED action upon non-timeout non-abort failure', () => { - fetchMock.post(MOCK_URL, { throws: { error: 'misc error' } }, { overwriteRoutes: true }); + fetchMock.post(MOCK_URL, { throws: { statusText: 'misc error' } }, { overwriteRoutes: true }); const timeoutInSec = 1 / 1000; const actionThunk = actions.runQuery({}, false, timeoutInSec); diff --git a/superset/assets/spec/javascripts/modules/utils_spec.jsx b/superset/assets/spec/javascripts/modules/utils_spec.jsx index f03e10d862..46a73ec297 100644 --- a/superset/assets/spec/javascripts/modules/utils_spec.jsx +++ b/superset/assets/spec/javascripts/modules/utils_spec.jsx @@ -5,27 +5,34 @@ import { d3TimeFormatPreset, defaultNumberFormatter, mainMetric, + getClientErrorObject, } from '../../../src/modules/utils'; describe('utils', () => { - it('formatSelectOptionsForRange', () => { - expect(formatSelectOptionsForRange(0, 4)).toEqual([ - [0, '0'], - [1, '1'], - [2, '2'], - [3, '3'], - [4, '4'], - ]); - expect(formatSelectOptionsForRange(1, 2)).toEqual([ - [1, '1'], - [2, '2'], - ]); + describe('formatSelectOptionsForRange', () => { + it('returns an array of arrays for the range specified (inclusive)', () => { + expect(formatSelectOptionsForRange(0, 4)).toEqual([ + [0, '0'], + [1, '1'], + [2, '2'], + [3, '3'], + [4, '4'], + ]); + expect(formatSelectOptionsForRange(1, 2)).toEqual([ + [1, '1'], + [2, '2'], + ]); + }); }); - it('d3format', () => { - expect(d3format('.3s', 1234)).toBe('1.23k'); - expect(d3format('.3s', 1237)).toBe('1.24k'); - expect(d3format('', 1237)).toBe('1.24k'); + + describe('d3format', () => { + it('returns a string formatted number as specified', () => { + expect(d3format('.3s', 1234)).toBe('1.23k'); + expect(d3format('.3s', 1237)).toBe('1.24k'); + expect(d3format('', 1237)).toBe('1.24k'); + }); }); + describe('d3FormatPreset', () => { it('is a function', () => { expect(typeof d3FormatPreset).toBe('function'); @@ -34,6 +41,7 @@ describe('utils', () => { expect(d3FormatPreset('.3s')(3000000)).toBe('3.00M'); }); }); + describe('d3TimeFormatPreset', () => { it('is a function', () => { expect(typeof d3TimeFormatPreset).toBe('function'); @@ -42,6 +50,7 @@ describe('utils', () => { expect(d3FormatPreset('smart_date')(0)).toBe('1970'); }); }); + describe('defaultNumberFormatter', () => { expect(defaultNumberFormatter(10)).toBe('10'); expect(defaultNumberFormatter(1)).toBe('1'); @@ -61,6 +70,7 @@ describe('utils', () => { expect(defaultNumberFormatter(-111000000)).toBe('-111M'); expect(defaultNumberFormatter(-0.23)).toBe('-230m'); }); + describe('mainMetric', () => { it('is null when no options', () => { expect(mainMetric([])).toBeUndefined(); @@ -88,4 +98,44 @@ describe('utils', () => { expect(mainMetric(metrics)).toBe('foo'); }); }); + + describe('getClientErrorObject', () => { + it('Returns a Promise', () => { + const response = getClientErrorObject('error'); + expect(response.constructor === Promise).toBe(true); + }); + + it('Returns a Promise that resolves to an object with an error key', () => { + const error = 'error'; + + return getClientErrorObject(error).then((errorObj) => { + expect(errorObj).toMatchObject({ error }); + }); + }); + + it('Handles Response that can be parsed as json', () => { + const jsonError = { something: 'something', error: 'Error message' }; + const jsonErrorString = JSON.stringify(jsonError); + + return getClientErrorObject(new Response(jsonErrorString)).then((errorObj) => { + expect(errorObj).toMatchObject(jsonError); + }); + }); + + it('Handles Response that can be parsed as text', () => { + const textError = 'Hello I am a text error'; + + return getClientErrorObject(new Response(textError)).then((errorObj) => { + expect(errorObj).toMatchObject({ error: textError }); + }); + }); + + it('Handles plain text as input', () => { + const error = 'error'; + + return getClientErrorObject(error).then((errorObj) => { + expect(errorObj).toMatchObject({ error }); + }); + }); + }); }); diff --git a/superset/assets/src/SqlLab/App.jsx b/superset/assets/src/SqlLab/App.jsx index 36d8ddd1f9..746a0e91c0 100644 --- a/superset/assets/src/SqlLab/App.jsx +++ b/superset/assets/src/SqlLab/App.jsx @@ -7,7 +7,6 @@ import { hot } from 'react-hot-loader'; import getInitialState from './getInitialState'; import rootReducer from './reducers'; import { initEnhancer } from '../reduxUtils'; -import { initJQueryAjax } from '../modules/utils'; import App from './components/App'; import { appSetup } from '../common'; @@ -16,7 +15,6 @@ import '../../stylesheets/reactable-pagination.css'; import '../components/FilterableTable/FilterableTableStyles.css'; appSetup(); -initJQueryAjax(); const appContainer = document.getElementById('app'); const bootstrapData = JSON.parse(appContainer.getAttribute('data-bootstrap')); diff --git a/superset/assets/src/chart/chartAction.js b/superset/assets/src/chart/chartAction.js index 12df6a2bc4..fa6821582a 100644 --- a/superset/assets/src/chart/chartAction.js +++ b/superset/assets/src/chart/chartAction.js @@ -5,7 +5,7 @@ import { getExploreUrlAndPayload, getAnnotationJsonUrl } from '../explore/explor import { requiresQuery, ANNOTATION_SOURCE_TYPES } from '../modules/AnnotationTypes'; import { addDangerToast } from '../messageToasts/actions'; import { Logger, LOG_ACTIONS_LOAD_CHART } from '../logger'; -import { COMMON_ERR_MESSAGES } from '../utils/common'; +import { getClientErrorObject } from '../modules/utils'; import { t } from '../locales'; export const CHART_UPDATE_STARTED = 'CHART_UPDATE_STARTED'; @@ -163,7 +163,7 @@ export function runQuery(formData, force = false, timeout = 60, key) { }); return dispatch(chartUpdateSucceeded(json, key)); }) - .catch((err) => { + .catch((response) => { Logger.append(LOG_ACTIONS_LOAD_CHART, { slice_id: key, has_err: true, @@ -171,28 +171,15 @@ export function runQuery(formData, force = false, timeout = 60, key) { start_offset: logStart, duration: Logger.getTimestamp() - logStart, }); - if (err.statusText === 'timeout') { - dispatch(chartUpdateTimeout(err.statusText, timeout, key)); - } else if (err.statusText === 'AbortError') { - dispatch(chartUpdateStopped(key)); - } else { - let errObject = err; - if (err.responseJSON) { - errObject = err.responseJSON; - } else if (err.stack) { - errObject = { - error: - t('Unexpected error: ') + - (err.description || t('(no description, click to see stack trace)')), - stacktrace: err.stack, - }; - } else if (err.responseText && err.responseText.indexOf('CSRF') >= 0) { - errObject = { - error: COMMON_ERR_MESSAGES.SESSION_TIMED_OUT, - }; - } - dispatch(chartUpdateFailed(errObject, key)); + + if (response.statusText === 'timeout') { + return dispatch(chartUpdateTimeout(response.statusText, timeout, key)); + } else if (response.statusText === 'AbortError') { + return dispatch(chartUpdateStopped(key)); } + return getClientErrorObject(response).then(parsedResponse => + dispatch(chartUpdateFailed(parsedResponse, key)), + ); }); const annotationLayers = formData.annotation_layers || []; diff --git a/superset/assets/src/dashboard/App.jsx b/superset/assets/src/dashboard/App.jsx index 1167e9bfa6..404677bdfa 100644 --- a/superset/assets/src/dashboard/App.jsx +++ b/superset/assets/src/dashboard/App.jsx @@ -6,13 +6,11 @@ import { hot } from 'react-hot-loader'; import { initEnhancer } from '../reduxUtils'; import { appSetup } from '../common'; -import { initJQueryAjax } from '../modules/utils'; import DashboardContainer from './containers/Dashboard'; import getInitialState from './reducers/getInitialState'; import rootReducer from './reducers/index'; appSetup(); -initJQueryAjax(); const appContainer = document.getElementById('app'); const bootstrapData = JSON.parse(appContainer.getAttribute('data-bootstrap')); diff --git a/superset/assets/src/dashboard/actions/dashboardState.js b/superset/assets/src/dashboard/actions/dashboardState.js index 89c0a9aba7..883a508c0f 100644 --- a/superset/assets/src/dashboard/actions/dashboardState.js +++ b/superset/assets/src/dashboard/actions/dashboardState.js @@ -6,7 +6,7 @@ import { addChart, removeChart, refreshChart } from '../../chart/chartAction'; import { chart as initChart } from '../../chart/chartReducer'; import { fetchDatasourceMetadata } from '../../dashboard/actions/datasources'; import { applyDefaultFormData } from '../../explore/store'; -import { getAjaxErrorMsg } from '../../modules/utils'; +import { getClientErrorObject } from '../../modules/utils'; import { Logger, LOG_ACTIONS_CHANGE_DASHBOARD_FILTER, @@ -143,11 +143,14 @@ export function saveDashboardRequest(data, id, saveType) { ), ]), ) - .catch(error => - dispatch( - addDangerToast( - `${t('Sorry, there was an error saving this dashboard: ')} - ${getAjaxErrorMsg(error) || error}`, + .catch(response => + getClientErrorObject(response).then(({ error }) => + dispatch( + addDangerToast( + `${t( + 'Sorry, there was an error saving this dashboard: ', + )} ${error}}`, + ), ), ), ); diff --git a/superset/assets/src/dashboard/actions/datasources.js b/superset/assets/src/dashboard/actions/datasources.js index eab9e99f22..85d91bdafd 100644 --- a/superset/assets/src/dashboard/actions/datasources.js +++ b/superset/assets/src/dashboard/actions/datasources.js @@ -1,5 +1,5 @@ import { SupersetClient } from '@superset-ui/core'; -import { getAjaxErrorMsg } from '../../modules/utils'; +import { getClientErrorObject } from '../../modules/utils'; export const SET_DATASOURCE = 'SET_DATASOURCE'; export function setDatasource(datasource, key) { @@ -29,8 +29,10 @@ export function fetchDatasourceMetadata(key) { endpoint: `/superset/fetch_datasource_metadata?datasourceKey=${key}`, }) .then(data => dispatch(data, key)) - .catch(error => - dispatch(fetchDatasourceFailed(getAjaxErrorMsg(error), key)), + .catch(response => + getClientErrorObject(response).then(({ error }) => + dispatch(fetchDatasourceFailed(error, key)), + ), ); }; } diff --git a/superset/assets/src/explore/App.jsx b/superset/assets/src/explore/App.jsx index 002eb261a6..886620ecd1 100644 --- a/superset/assets/src/explore/App.jsx +++ b/superset/assets/src/explore/App.jsx @@ -6,7 +6,6 @@ import thunk from 'redux-thunk'; import { initEnhancer } from '../reduxUtils'; import ToastPresenter from '../messageToasts/containers/ToastPresenter'; -import { initJQueryAjax } from '../modules/utils'; import ExploreViewContainer from './components/ExploreViewContainer'; import getInitialState from './reducers/getInitialState'; import rootReducer from './reducers/index'; @@ -16,7 +15,6 @@ import './main.css'; import '../../stylesheets/reactable-pagination.css'; appSetup(); -initJQueryAjax(); const exploreViewContainer = document.getElementById('app'); const bootstrapData = JSON.parse(exploreViewContainer.getAttribute('data-bootstrap')); diff --git a/superset/assets/src/modules/utils.js b/superset/assets/src/modules/utils.js index 879f7e8f8a..e2133f179e 100644 --- a/superset/assets/src/modules/utils.js +++ b/superset/assets/src/modules/utils.js @@ -3,6 +3,8 @@ import d3 from 'd3'; import $ from 'jquery'; import { SupersetClient } from '@superset-ui/core'; import { formatDate, UTC } from './dates'; +import { COMMON_ERR_MESSAGES } from '../utils/common'; +import { t } from '../locales'; const siFormatter = d3.format('.3s'); @@ -119,16 +121,56 @@ function showApiMessage(resp) { .appendTo($('#alert-container')); } +export function getClientErrorObject(response) { + // takes a Response object as input, attempts to read response as Json if possible, + // and returns a Promise that resolves to a plain object with error key and text value. + return new Promise((resolve) => { + if (typeof response === 'string') { + resolve({ error: response }); + } else if (response && response.constructor === Response && !response.bodyUsed) { + // attempt to read the body as json, and fallback to text. we must clone the + // response in order to fallback to .text() because Response is single-read + response.clone().json().then((errorJson) => { + let error = { ...response, ...errorJson }; + if (error.stack) { + error = { + ...error, + error: t('Unexpected error: ') + + (error.description || t('(no description, click to see stack trace)')), + stacktrace: error.stack, + }; + } else if (error.responseText && error.responseText.indexOf('CSRF') >= 0) { + error = { + ...error, + error: COMMON_ERR_MESSAGES.SESSION_TIMED_OUT, + }; + } + resolve(error); + }).catch(() => { + // fall back to reading as text + response.text().then((errorText) => { + resolve({ ...response, error: errorText }); + }); + }); + } else { + // fall back to Response.statusText or generic error of we cannot read the response + resolve({ ...response, error: response.statusText || t('An error occurred') }); + } + }); + +} + export function toggleCheckbox(apiUrlPrefix, selector) { SupersetClient.get({ endpoint: apiUrlPrefix + $(selector)[0].checked }) .then(() => {}) - .catch((response) => { - // @TODO utility function to read this - const resp = response.responseJSON; - if (resp && resp.message) { - showApiMessage(resp); - } - }); + .catch(response => + getClientErrorObject(response) + .then((parsedResp) => { + if (parsedResp && parsedResp.message) { + showApiMessage(parsedResp); + } + }), + ); } /** @@ -188,31 +230,10 @@ export function formatSelectOptions(options) { ); } -export function getAjaxErrorMsg(error) { - const respJSON = error.responseJSON; - return (respJSON && respJSON.error) ? respJSON.error : - error.responseText; -} - export function getDatasourceParameter(datasourceId, datasourceType) { return `${datasourceId}__${datasourceType}`; } -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(); - if (token) { - $.ajaxSetup({ - beforeSend(xhr, settings) { - if (!/^(GET|HEAD|OPTIONS|TRACE)$/i.test(settings.type) && !this.crossDomain) { - xhr.setRequestHeader('X-CSRFToken', token); - } - }, - }); - } -} - export function getParam(name) { /* eslint no-useless-escape: 0 */ const formattedName = name.replace(/[\[]/, '\\[').replace(/[\]]/, '\\]');