Skip to content

Commit

Permalink
Handle AJAX errors by showing a notification (elastic#717)
Browse files Browse the repository at this point in the history
* timeout property for axios instance

* remove TODOS

* notifier class

* TODO

* second notification with the context

* redirect to home if workpad could not be fetched

* less noisy error checking
  • Loading branch information
tsullivan authored Jun 29, 2018
1 parent 715ff38 commit 1e8325d
Show file tree
Hide file tree
Showing 9 changed files with 45 additions and 20 deletions.
1 change: 1 addition & 0 deletions common/lib/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ export const APP_ROUTE = '/app/canvas';
export const API_ROUTE = '/api/canvas';
export const API_ROUTE_WORKPAD = `${API_ROUTE}/workpad`;
export const LOCALSTORAGE_LASTPAGE = 'canvas:lastpage';
export const FETCH_TIMEOUT = 30000; // 30 seconds
2 changes: 2 additions & 0 deletions common/lib/fetch.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import axios from 'axios';
import { FETCH_TIMEOUT } from './constants';

export const fetch = axios.create({
headers: {
Accept: 'application/json',
'Content-Type': 'application/json',
'kbn-xsrf': 'professionally-crafted-string-of-text',
},
timeout: FETCH_TIMEOUT,
});
16 changes: 8 additions & 8 deletions public/apps/workpad/routes.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as workpadService from '../../lib/workpad_service';
import { notify } from '../../lib/notify';
import { getDefaultWorkpad } from '../../state/defaults';
import { setWorkpad } from '../../state/actions/workpad';
import { setAssets, resetAssets } from '../../state/actions/assets';
Expand All @@ -15,7 +14,6 @@ export const routes = [
name: 'createWorkpad',
path: '/create',
action: dispatch => async ({ router }) => {
// TODO: handle the failed creation state
const newWorkpad = getDefaultWorkpad();
await workpadService.create(newWorkpad);
dispatch(setWorkpad(newWorkpad));
Expand All @@ -33,13 +31,15 @@ export const routes = [
// load workpad if given a new id via url param
const currentWorkpad = getWorkpad(getState());
if (params.id !== currentWorkpad.id) {
try {
const { assets, ...workpad } = await workpadService.get(params.id);
dispatch(setWorkpad(workpad));
dispatch(setAssets(assets));
} catch (fetchErr) {
notify.error(fetchErr);
const fetchedWorkpad = await workpadService.get(params.id);

if (fetchedWorkpad == null) {
return router.redirectTo('home');
}

const { assets, ...workpad } = fetchedWorkpad;
dispatch(setWorkpad(workpad));
dispatch(setAssets(assets));
}

// fetch the workpad again, to get changes
Expand Down
2 changes: 1 addition & 1 deletion public/components/update_modal/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { withState, lifecycle, compose } from 'recompose';
import Markdown from 'markdown-it';
import { get } from 'lodash';
import fetch from 'axios';
import { fetch } from '../../../common/lib/fetch';
import pkg from '../../../package.json';
import { UpdateModal as Component } from './update_modal';

Expand Down
3 changes: 0 additions & 3 deletions public/components/workpad_loader/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,12 @@ export const WorkpadLoader = compose(

// Workpad search
findWorkpads: ({ setWorkpads }) => async text => {
// TODO: handle search failures
const workpads = await workpadService.find(text);
setWorkpads(workpads);
},

// Workpad import/export methods
downloadWorkpad: () => async workpadId => {
// TODO: handle the failed loading state
const workpad = await workpadService.get(workpadId);
const jsonBlob = new Blob([JSON.stringify(workpad)], { type: 'application/json' });
fileSaver.saveAs(jsonBlob, `canvas-workpad-${workpad.name}-${workpad.id}.json`);
Expand All @@ -48,7 +46,6 @@ export const WorkpadLoader = compose(
removeWorkpad: props => async workpadId => {
const { setWorkpads, workpads, workpadId: loadedWorkpad } = props;

// TODO: handle the failed removal condition
await workpadService.remove(workpadId);

const remainingWorkpads = workpads.workpads.filter(w => w.id !== workpadId);
Expand Down
2 changes: 1 addition & 1 deletion public/functions/geoip.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import fetch from 'axios';
import { fetch } from '../../common/lib/fetch';

// TODO: We should implement our own Elastic GeoIP service instead of using freegeoip.net
export const geoip = () => ({
Expand Down
11 changes: 10 additions & 1 deletion public/lib/notify.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
import { notify as kbnNotify } from 'ui/notify';
import { Notifier } from 'ui/notify';

/*
* TODO Make this use a toast error instead of a banner.
* Toast should be able to give context about where the error originated,
* not just show you the error data
*/
const kbnNotify = new Notifier({
location: 'Canvas',
});

export const notify = {
/*
Expand Down
26 changes: 21 additions & 5 deletions public/lib/workpad_service.js
Original file line number Diff line number Diff line change
@@ -1,33 +1,49 @@
import chrome from 'ui/chrome';
import { API_ROUTE_WORKPAD } from '../../common/lib/constants';
import { notify } from '../lib/notify';
import { fetch } from '../../common/lib/fetch';

const basePath = chrome.getBasePath();
const apiPath = `${basePath}${API_ROUTE_WORKPAD}`;

/*
* The error data will be in `err.response` if the error comes from the server (example: 404)
* The error object will be error data if it comes directly from the fetch library, (example: network error)
*/
const notifyError = source => {
return err => {
const errData = err.response || err;
notify.error(source);
notify.error(errData);
};
};

export function create(workpad) {
return fetch.post(apiPath, { ...workpad, assets: workpad.assets || {} });
return fetch
.post(apiPath, { ...workpad, assets: workpad.assets || {} })
.catch(notifyError('Could not create workpad'));
}

export function get(workpadId) {
return fetch
.get(`${apiPath}/${workpadId}`)
.then(res => res.data)
.catch(({ response }) => Promise.reject(response));
.catch(notifyError('Could not get workpad'));
}

export function update(id, workpad) {
return fetch.put(`${apiPath}/${id}`, workpad);
return fetch.put(`${apiPath}/${id}`, workpad).catch(notifyError('Could not update workpad'));
}

export function remove(id) {
return fetch.delete(`${apiPath}/${id}`);
return fetch.delete(`${apiPath}/${id}`).catch(notifyError('Could not remove workpad'));
}

export function find(searchTerm) {
const validSearchTerm = typeof searchTerm === 'string' && searchTerm.length > 0;

return fetch
.get(`${apiPath}/find?name=${validSearchTerm ? searchTerm : ''}`)
.then(resp => resp.data);
.then(resp => resp.data)
.catch(notifyError('Could not find workpads'));
}
2 changes: 1 addition & 1 deletion server/functions/timelion.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import fetch from 'axios';
import { flatten } from 'lodash';
import { fetch } from '../../common/lib/fetch';
import { buildBoolArray } from './esdocs/lib/build_bool_array';

export const timelion = () => ({
Expand Down

0 comments on commit 1e8325d

Please sign in to comment.