This is an automated email from the ASF dual-hosted git repository.
ccwilliams pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/master by this push:
new d8d50a1 [superset-client] use getClientErrorObject for client error
handling (#6163)
d8d50a1 is described below
commit d8d50a168daf36aef83e02585617b10335321018
Author: Chris Williams <willias...@users.noreply.github.com>
AuthorDate: Mon Oct 22 22:42:56 2018 -0700
[superset-client] use getClientErrorObject for client error handling (#6163)
* [superset-client] use getClientErrorObject for client error handling
* fix getClientErrorObject json parsing
* fix getClientErrorObject test typos
* kick build
---
.../{explore => chart}/chartActions_spec.js | 2 +-
.../assets/spec/javascripts/modules/utils_spec.jsx | 82 +++++++++++++++++-----
superset/assets/src/SqlLab/App.jsx | 2 -
superset/assets/src/chart/chartAction.js | 33 +++------
superset/assets/src/dashboard/App.jsx | 2 -
.../assets/src/dashboard/actions/dashboardState.js | 15 ++--
.../assets/src/dashboard/actions/datasources.js | 8 ++-
superset/assets/src/explore/App.jsx | 2 -
superset/assets/src/modules/utils.js | 77 ++++++++++++--------
9 files changed, 140 insertions(+), 83 deletions(-)
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 50635f1..21fbf90 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 f03e10d..46a73ec 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 36d8ddd..746a0e9 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 12df6a2..fa68215 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 1167e9b..404677b 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 89c0a9a..883a508 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 eab9e99..85d91bd 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 002eb26..886620e 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 879f7e8..e2133f1 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(/[\]]/, '\\]');
