Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BIOMAGE-1026] Improved error messages #290

Merged
merged 12 commits into from
Jun 3, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Array [
},
Object {
"payload": Object {
"error": "Error saving experiment",
"error": "We couldn't save your data.",
},
"type": "experiments/error",
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ describe('saveExperiment action', () => {

it('Dispatches a notification when fetch fails.', async () => {
fetchMock.resetMocks();
fetchMock.mockReject(new Error('some weird error that happened'));
fetchMock.mockResponse(JSON.stringify({ message: 'Error from server hidden from user' }), { status: 500 });

const store = mockStore(initialState);
await store.dispatch(saveExperiment(mockExperiment.id));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ Array [
Object {
"payload": Object {
"error": "Could not start gem2s.",
"errorType": [Error: some weird error that happened],
"errorType": "some weird error that happened",
},
"type": "experimentSettings/backendStatusError",
},
Expand Down
2 changes: 1 addition & 1 deletion src/__test__/redux/actions/pipelines/runGem2s.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ describe('runGem2s action', () => {

it('Dispatches status error if loading fails', async () => {
fetchMock.resetMocks();
fetchMock.mockReject(new Error('some weird error that happened'));
fetchMock.mockResponse(JSON.stringify({ message: 'some weird error that happened' }), { status: 400 });

const store = mockStore(initialState);
await store.dispatch(runGem2s(experimentId));
Expand Down
4 changes: 1 addition & 3 deletions src/__test__/redux/actions/projects/deleteProject.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,9 @@ describe('deleteProject action', () => {
};

beforeEach(async () => {
const response = new Response({});

fetchMock.resetMocks();
fetchMock.doMock();
fetchMock.mockResolvedValue(response);
fetchMock.mockResponse(JSON.stringify({}));
});

it('Dispatches event correctly for one sample', async () => {
Expand Down
18 changes: 9 additions & 9 deletions src/__test__/redux/actions/projects/loadProjects.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,24 @@ describe('load projects ', () => {
},
};

const response = new Response(
JSON.stringify(
[
{ name: 'I am project', samples: ['Best sample so far', 'and another one'] },
{ name: 'project am I', samples: ['tired of unit testing honestly'] },
],
),
const response = JSON.stringify(
[
{ name: 'I am project', samples: ['Best sample so far', 'and another one'] },
{ name: 'project am I', samples: ['tired of unit testing honestly'] },
],
);

fetchMock.resetMocks();
fetchMock.doMock();
fetchMock.mockResolvedValue(response);
fetchMock.mockResponse(response);

it('Dispatches load action correctly', async () => {
const store = mockStore(initialState);
await store.dispatch(loadProjects());
const action = store.getActions()[3];
expect(action.type).toEqual(PROJECTS_LOADED);
const actions = store.getActions();
const lastAction = actions[actions.length - 1];
expect(lastAction.type).toEqual(PROJECTS_LOADED);
});

it('Dispatches error correctly', async () => {
Expand Down
2 changes: 1 addition & 1 deletion src/__test__/redux/actions/projects/saveProject.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ describe('saveProject action', () => {
const errorMsg = 'some weird error that happened';

fetchMock.resetMocks();
fetchMock.mockReject(new Error(errorMsg));
fetchMock.mockResponse(JSON.stringify({ message: errorMsg }), { status: 400 });

const store = mockStore(initialState);

Expand Down
2 changes: 1 addition & 1 deletion src/__test__/redux/actions/samples/saveSamples.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ describe('saveSamples action', () => {
const errorMsg = 'some weird error that happened';

fetchMock.resetMocks();
fetchMock.mockReject(new Error(errorMsg));
fetchMock.mockResponse(JSON.stringify({ message: errorMsg }), { status: 400 });

const store = mockStore(initialState);

Expand Down
6 changes: 3 additions & 3 deletions src/components/FeedbackButton.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {
Button, Dropdown, Card, Input, Space,
} from 'antd';
import { CommentOutlined, DownOutlined } from '@ant-design/icons';
import messages from './notification/messages';
import endUserMessages from '../utils/endUserMessages';
import pushNotificationMessage from '../utils/pushNotificationMessage';

const { TextArea } = Input;
Expand Down Expand Up @@ -52,9 +52,9 @@ const FeedbackButton = () => {
throw new Error('Invalid status code returned.');
}
setFeedbackText('');
pushNotificationMessage('success', messages.feedbackSuccessful);
pushNotificationMessage('success', endUserMessages.FEEDBACK_SUCCESSFUL);
} catch {
pushNotificationMessage('error', messages.feedbackError);
pushNotificationMessage('error', endUserMessages.FEEDBACK_ERROR);
}
};

Expand Down
4 changes: 2 additions & 2 deletions src/components/UserButton.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
} from 'antd';

import { Auth, Hub } from 'aws-amplify';
import messages from './notification/messages';
import endUserMessages from '../utils/endUserMessages';
import pushNotificationMessage from '../utils/pushNotificationMessage';

const UserButton = () => {
Expand All @@ -29,7 +29,7 @@ const UserButton = () => {
break;
case 'signIn_failure':
case 'cognitoHostedUI_failure':
pushNotificationMessage('error', messages.signInError);
pushNotificationMessage('error', endUserMessages.ERROR_SIGN_IN);
break;
default:
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
} from '../../../redux/actions/cellSets';
import composeTree from '../../../utils/composeTree';
import { isBrowser } from '../../../utils/environment';
import messages from '../../notification/messages';
import endUserMessages from '../../../utils/endUserMessages';
import PlatformError from '../../PlatformError';
import CellSetOperation from './CellSetOperation';
import { union, intersection, complement } from '../../../utils/cellSetOperations';
Expand Down Expand Up @@ -57,7 +57,7 @@ const CellSetsTool = (props) => {
if (
notifications
&& notifications.message
&& notifications.message.message === messages.newClusterCreated
&& notifications.message.message === endUserMessages.NEW_CLUSTER_CREATED
) {
animateScroll.scrollTo(height, {
containerId: 'cell-set-tool-container',
Expand All @@ -82,7 +82,7 @@ const CellSetsTool = (props) => {
};

/**
* Remders the content inside the tool. Can be a skeleton during loading
* Renders the content inside the tool. Can be a skeleton during loading
* or a hierarchical tree listing all cell sets.
*/
const renderContent = () => {
Expand Down
2 changes: 1 addition & 1 deletion src/components/data-management/ProjectDetails.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React, { useState, useEffect, useRef } from 'react';
import {
Table, Typography, Space, Tooltip, PageHeader, Button, Input, Progress, Row, Col,
Table, Typography, Space, Tooltip, Button, Input, Progress, Row, Col,
} from 'antd';
import { useRouter } from 'next/router';
import { useSelector, useDispatch } from 'react-redux';
Expand Down
9 changes: 0 additions & 9 deletions src/components/notification/messages.js

This file was deleted.

11 changes: 7 additions & 4 deletions src/pages/data-management/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,12 @@ const DataManagementPage = ({ route }) => {
const {
saving: sampleSaving,
} = useSelector((state) => state.samples.meta);
const [newProjectModalVisible, setNewProjectModalVisible] = useState(false);
const {
activeProjectUuid,
loading: projectsLoading,
} = useSelector((state) => state.projects.meta);
const experiments = useSelector((state) => state.experiments);
const { activeProjectUuid, loading: projectsLoading } = useSelector((state) => state.projects.meta);
const [newProjectModalVisible, setNewProjectModalVisible] = useState(false);
const activeProject = projectsList[activeProjectUuid];

const existingExperiments = activeProject?.experiments
Expand Down Expand Up @@ -125,8 +128,8 @@ const DataManagementPage = ({ route }) => {
title='Data Management'
/>
<LoadingModal
visible={projectSaving || sampleSaving}
message={projectSaving || sampleSaving || ''}
visible={Boolean(projectSaving || sampleSaving)}
message={projectSaving ?? sampleSaving ?? ''}
/>

<NewProjectModal
Expand Down
8 changes: 4 additions & 4 deletions src/redux/actions/cellSets/createCellSet.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { v4 as uuidv4 } from 'uuid';
import messages from '../../../components/notification/messages';
import endUserMessages from '../../../utils/endUserMessages';
import pushNotificationMessage from '../../../utils/pushNotificationMessage';
import { CELL_SETS_CREATE } from '../../actionTypes/cellSets';
import saveCellSets from './saveCellSets';
import pushNotificationMessage from '../../../utils/pushNotificationMessage';

const createCellSet = (experimentId, name, color, cellIds) => async (dispatch, getState) => {
const {
Expand All @@ -21,7 +21,7 @@ const createCellSet = (experimentId, name, color, cellIds) => async (dispatch, g
};

if (data.cellIds.size === 0) {
pushNotificationMessage('info', messages.emptyClusterNotCreated);
pushNotificationMessage('info', endUserMessages.EMPTY_CLUSTER_NOT_CREATED);
return;
}

Expand All @@ -34,7 +34,7 @@ const createCellSet = (experimentId, name, color, cellIds) => async (dispatch, g
});

await dispatch(saveCellSets(experimentId));
pushNotificationMessage('info', messages.newClusterCreated);
pushNotificationMessage('info', endUserMessages.NEW_CLUSTER_CREATED);
};

export default createCellSet;
12 changes: 8 additions & 4 deletions src/redux/actions/cellSets/loadCellSets.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import fetchAPI from '../../../utils/fetchAPI';
import { isServerError, throwIfRequestFailed } from '../../../utils/fetchErrors';
import endUserMessages from '../../../utils/endUserMessages';
import {
CELL_SETS_LOADED, CELL_SETS_LOADING, CELL_SETS_ERROR,
} from '../../actionTypes/cellSets';
Expand All @@ -17,12 +19,11 @@ const loadCellSets = (experimentId) => async (dispatch, getState) => {
});
}

const url = `/v1/experiments/${experimentId}/cellSets`;
try {
const response = await fetchAPI(`/v1/experiments/${experimentId}/cellSets`);
const response = await fetchAPI(url);
const json = await response.json();
if (!response.ok) {
throw new Error('HTTP status code was not 200.');
}
throwIfRequestFailed(response, json, endUserMessages.ERROR_FETCHING_CELL_SETS);
dispatch({
type: CELL_SETS_LOADED,
payload: {
Expand All @@ -31,6 +32,9 @@ const loadCellSets = (experimentId) => async (dispatch, getState) => {
},
});
} catch (e) {
if (!isServerError(e)) {
console.error(`fetch ${url} error ${e.message}`);
}
dispatch({
type: CELL_SETS_ERROR,
payload: {
Expand Down
14 changes: 10 additions & 4 deletions src/redux/actions/cellSets/saveCellSets.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import fetchAPI from '../../../utils/fetchAPI';
import { CELL_SETS_SAVE } from '../../actionTypes/cellSets';
import { isServerError, throwIfRequestFailed } from '../../../utils/fetchErrors';
import endUserMessages from '../../../utils/endUserMessages';
import pushNotificationMessage from '../../../utils/pushNotificationMessage';
import composeTree from '../../../utils/composeTree';
import messages from '../../../components/notification/messages';
import { CELL_SETS_SAVE } from '../../actionTypes/cellSets';

const saveCellSets = (experimentId) => async (dispatch, getState) => {
const {
Expand All @@ -17,9 +18,10 @@ const saveCellSets = (experimentId) => async (dispatch, getState) => {
}

const treeData = composeTree(hierarchy, properties);
const url = `/v1/experiments/${experimentId}/cellSets`;
try {
const response = await fetchAPI(
`/v1/experiments/${experimentId}/cellSets`,
url,
{
method: 'PUT',
headers: {
Expand All @@ -33,6 +35,7 @@ const saveCellSets = (experimentId) => async (dispatch, getState) => {
);

const json = await response.json();
throwIfRequestFailed(response, json, endUserMessages.ERROR_SAVING);

dispatch({
type: CELL_SETS_SAVE,
Expand All @@ -42,7 +45,10 @@ const saveCellSets = (experimentId) => async (dispatch, getState) => {
},
});
} catch (e) {
pushNotificationMessage('error', messages.saveCellSets);
if (!isServerError(e)) {
console.error(`fetch ${url} error ${e.message}`);
}
pushNotificationMessage('error', endUserMessages.ERROR_SAVING);
}
};

Expand Down
2 changes: 1 addition & 1 deletion src/redux/actions/cellSets/updateCellSetsClustering.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ const updateCellSetsClustering = (experimentId, resolution) => async (dispatch,
data: newCellSets,
},
});
dispatch(saveCellSets(experimentId));
await dispatch(saveCellSets(experimentId));
} catch (e) {
dispatch({
type: CELL_SETS_ERROR,
Expand Down
21 changes: 13 additions & 8 deletions src/redux/actions/componentConfig/loadPlotConfig.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
import _ from 'lodash';
import fetchAPI from '../../../utils/fetchAPI';
import { LOAD_CONFIG } from '../../actionTypes/componentConfig';
import { isServerError, throwIfRequestFailed } from '../../../utils/fetchErrors';
import endUserMessages from '../../../utils/endUserMessages';
import pushNotificationMessage from '../../../utils/pushNotificationMessage';
import messages from '../../../components/notification/messages';
import { LOAD_CONFIG } from '../../actionTypes/componentConfig';
import { initialPlotConfigStates } from '../../reducers/componentConfig/initialState';

const loadPlotConfig = (experimentId, plotUuid, plotType) => async (dispatch) => {
const url = `/v1/experiments/${experimentId}/plots-tables/${plotUuid}`;
try {
const response = await fetchAPI(
`/v1/experiments/${experimentId}/plots-tables/${plotUuid}`,
);
const response = await fetchAPI(url);
const data = await response.json();
if (response.ok) {
const data = await response.json();
const config = _.merge({}, initialPlotConfigStates[plotType], data.config);
dispatch({
type: LOAD_CONFIG,
Expand All @@ -33,10 +33,15 @@ const loadPlotConfig = (experimentId, plotUuid, plotType) => async (dispatch) =>
},
});
} else {
throw new Error('Server sent back different error or json conversion failed.');
throwIfRequestFailed(response, data, endUserMessages.ERROR_FETCHING_PLOT_CONFIG);
}
} catch (e) {
pushNotificationMessage('error', messages.connectionError);
let { message } = e;
if (!isServerError(e)) {
console.error(`fetch ${url} error ${message}`);
message = endUserMessages.CONNECTION_ERROR;
}
pushNotificationMessage('error', message);
}
};

Expand Down
Loading