[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

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);

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 });
+      });
+    });
+  });
 });

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'));

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 || [];

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'));

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}}`,
+            ),
           ),
         ),
       );

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)),
+        ),
       );
   };
 }

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'));

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(/[\]]/, '\\]');