Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Improve error handling when retrieving all detectors #267

Merged
merged 2 commits into from
Aug 4, 2020
Merged

Improve error handling when retrieving all detectors #267

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
69326f9
Improve error handling on home pages
ohltyler Jul 31, 2020
c51c5b0
Add more error logging
ohltyler Aug 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 21 additions & 13 deletions public/pages/Dashboard/Container/DashboardOverview.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ import {
} from '@elastic/eui';
//@ts-ignore
import chrome from 'ui/chrome';
// @ts-ignore
import { toastNotifications } from 'ui/notify';
import { AnomalousDetectorsList } from '../Components/AnomalousDetectorsList';
import {
GET_ALL_DETECTORS_QUERY_PARAMS,
Expand All @@ -54,6 +56,8 @@ export function DashboardOverview() {

const allDetectorList = adState.detectorList;

const errorGettingDetectors = adState.errorMessage;

const [isLoadingDetectors, setIsLoadingDetectors] = useState(true);

const [currentDetectors, setCurrentDetectors] = useState(
Expand All @@ -69,7 +73,7 @@ export function DashboardOverview() {
[key: string]: DetectorListItem;
}) => {
const detectorNames = Object.values(detectorsIdMap).map(
detectorListItem => {
(detectorListItem) => {
return detectorListItem.name;
}
);
Expand All @@ -85,7 +89,7 @@ export function DashboardOverview() {
const handleDetectorsFilterChange = (
options: EuiComboBoxOptionProps[]
): void => {
const selectedNames = options.map(option => option.label);
const selectedNames = options.map((option) => option.label);

setSelectedDetectorsName(selectedNames);
setAllDetectorsSelected(isEmpty(selectedNames));
Expand All @@ -103,7 +107,7 @@ export function DashboardOverview() {
options: EuiComboBoxOptionProps[]
): void => {
const selectedStates = options.map(
option => option.label as DETECTOR_STATE
(option) => option.label as DETECTOR_STATE
);
setSelectedDetectorStates(selectedStates);
setAllDetectorStatesSelected(isEmpty(selectedStates));
Expand All @@ -122,7 +126,7 @@ export function DashboardOverview() {
const handleIndicesFilterChange = (
options: EuiComboBoxOptionProps[]
): void => {
const selectedIndices = options.map(option => option.label);
const selectedIndices = options.map((option) => option.label);
setSelectedIndices(selectedIndices);
setAllIndicesSelected(isEmpty(selectedIndices));
};
Expand All @@ -138,21 +142,21 @@ export function DashboardOverview() {
} else {
detectorsToFilter = cloneDeep(
Object.values(allDetectorList)
).filter(detectorItem => selectedNameList.includes(detectorItem.name));
).filter((detectorItem) => selectedNameList.includes(detectorItem.name));
}

let filteredDetectorItemsByNamesAndIndex = detectorsToFilter;
if (!allIndicesSelected) {
filteredDetectorItemsByNamesAndIndex = detectorsToFilter.filter(
detectorItem =>
(detectorItem) =>
selectedIndexList.includes(detectorItem.indices.toString())
);
}

let finalFilteredDetectors = filteredDetectorItemsByNamesAndIndex;
if (!allDetectorStatesSelected) {
finalFilteredDetectors = filteredDetectorItemsByNamesAndIndex.filter(
detectorItem => selectedStateList.includes(detectorItem.curState)
(detectorItem) => selectedStateList.includes(detectorItem.curState)
);
}

Expand All @@ -161,12 +165,7 @@ export function DashboardOverview() {

const intializeDetectors = async () => {
setIsLoadingDetectors(true);
try {
await dispatch(getDetectorList(GET_ALL_DETECTORS_QUERY_PARAMS));
} catch (error) {
console.log('Error is found during getting detector list', error);
}
setIsLoadingDetectors(false);
dispatch(getDetectorList(GET_ALL_DETECTORS_QUERY_PARAMS));
dispatch(getIndices(''));
dispatch(getAliases(''));
};
Expand All @@ -175,6 +174,14 @@ export function DashboardOverview() {
intializeDetectors();
}, []);

useEffect(() => {
if (errorGettingDetectors) {
console.error(errorGettingDetectors);
toastNotifications.addDanger('Unable to get all detectors');
ohltyler marked this conversation as resolved.
Show resolved Hide resolved
setIsLoadingDetectors(false);
}
}, [errorGettingDetectors]);

useEffect(() => {
chrome.breadcrumbs.set([
BREADCRUMBS.ANOMALY_DETECTOR,
Expand All @@ -184,6 +191,7 @@ export function DashboardOverview() {

useEffect(() => {
setCurrentDetectors(Object.values(allDetectorList));
setIsLoadingDetectors(false);
}, [allDetectorList]);

useEffect(() => {
Expand Down
50 changes: 30 additions & 20 deletions public/pages/DetectorsList/containers/List/List.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,10 @@ import {
getDetectorsToDisplay,
} from '../../../utils/helpers';
import { staticColumn } from '../../utils/tableUtils';
import { DETECTOR_ACTION } from '../../utils/constants';
import {
DETECTOR_ACTION,
SINGLE_DETECTOR_ERROR_MSG,
} from '../../utils/constants';
import { getTitleWithCount, Listener } from '../../../../utils/utils';
import { ListActions } from '../../components/ListActions/ListActions';
import { searchMonitors } from '../../../../redux/reducers/alerting';
Expand Down Expand Up @@ -117,6 +120,9 @@ export const DetectorList = (props: ListProps) => {
const dispatch = useDispatch();
const allDetectors = useSelector((state: AppState) => state.ad.detectorList);
const allMonitors = useSelector((state: AppState) => state.alerting.monitors);
const errorGettingDetectors = useSelector(
(state: AppState) => state.ad.errorMessage
);
const elasticsearchState = useSelector(
(state: AppState) => state.elasticsearch
);
Expand Down Expand Up @@ -171,6 +177,17 @@ export const DetectorList = (props: ListProps) => {
getInitialMonitors();
}, []);

useEffect(() => {
if (
errorGettingDetectors &&
errorGettingDetectors !== SINGLE_DETECTOR_ERROR_MSG
) {
console.error(errorGettingDetectors);
toastNotifications.addDanger('Unable to get all detectors');
setIsLoadingFinalDetectors(false);
}
}, [errorGettingDetectors]);

// Updating displayed indices (initializing to first 20 for now)
const visibleIndices = get(elasticsearchState, 'indices', []) as CatIndex[];
const visibleAliases = get(elasticsearchState, 'aliases', []) as IndexAlias[];
Expand Down Expand Up @@ -256,14 +273,7 @@ export const DetectorList = (props: ListProps) => {
}, [confirmModalState.isRequestingToClose, isLoading]);

const getUpdatedDetectors = async () => {
try {
dispatch(getDetectorList(GET_ALL_DETECTORS_QUERY_PARAMS));
} catch (error) {
toastNotifications.addDanger(
`Error is found while getting detector list: ${error}`
);
setIsLoadingFinalDetectors(false);
}
dispatch(getDetectorList(GET_ALL_DETECTORS_QUERY_PARAMS));
};

const handlePageChange = (pageNumber: number) => {
Expand Down Expand Up @@ -306,7 +316,7 @@ export const DetectorList = (props: ListProps) => {
const sanitizedQuery = sanitizeSearchText(searchValue);
setIndexQuery(sanitizedQuery);
await dispatch(getPrioritizedIndices(sanitizedQuery));
setState(state => ({
setState((state) => ({
...state,
page: 0,
}));
Expand All @@ -321,8 +331,8 @@ export const DetectorList = (props: ListProps) => {
states =
options.length == 0
? ALL_DETECTOR_STATES
: options.map(option => option.label as DETECTOR_STATE);
setState(state => ({
: options.map((option) => option.label as DETECTOR_STATE);
setState((state) => ({
...state,
page: 0,
selectedDetectorStates: states,
Expand All @@ -335,7 +345,7 @@ export const DetectorList = (props: ListProps) => {
indices =
options.length == 0
? ALL_INDICES
: options.map(option => option.label).slice(0, MAX_SELECTED_INDICES);
: options.map((option) => option.label).slice(0, MAX_SELECTED_INDICES);

setState({
...state,
Expand All @@ -345,7 +355,7 @@ export const DetectorList = (props: ListProps) => {
};

const handleResetFilter = () => {
setState(state => ({
setState((state) => ({
...state,
queryParams: {
...state.queryParams,
Expand Down Expand Up @@ -443,7 +453,7 @@ export const DetectorList = (props: ListProps) => {
const validIds = getDetectorsForAction(
selectedDetectorsForAction,
DETECTOR_ACTION.START
).map(detector => detector.id);
).map((detector) => detector.id);
const promises = validIds.map(async (id: string) => {
return dispatch(startDetector(id));
});
Expand All @@ -453,7 +463,7 @@ export const DetectorList = (props: ListProps) => {
'All selected detectors have been started successfully'
);
})
.catch(error => {
.catch((error) => {
toastNotifications.addDanger(
`Error starting all selected detectors: ${error}`
);
Expand All @@ -468,7 +478,7 @@ export const DetectorList = (props: ListProps) => {
const validIds = getDetectorsForAction(
selectedDetectorsForAction,
DETECTOR_ACTION.STOP
).map(detector => detector.id);
).map((detector) => detector.id);
const promises = validIds.map(async (id: string) => {
return dispatch(stopDetector(id));
});
Expand All @@ -479,7 +489,7 @@ export const DetectorList = (props: ListProps) => {
);
if (listener) listener.onSuccess();
})
.catch(error => {
.catch((error) => {
toastNotifications.addDanger(
`Error stopping all selected detectors: ${error}`
);
Expand All @@ -498,7 +508,7 @@ export const DetectorList = (props: ListProps) => {
const validIds = getDetectorsForAction(
selectedDetectorsForAction,
DETECTOR_ACTION.DELETE
).map(detector => detector.id);
).map((detector) => detector.id);
const promises = validIds.map(async (id: string) => {
return dispatch(deleteDetector(id));
});
Expand All @@ -508,7 +518,7 @@ export const DetectorList = (props: ListProps) => {
'All selected detectors have been deleted successfully'
);
})
.catch(error => {
.catch((error) => {
toastNotifications.addDanger(
`Error deleting all selected detectors: ${error}`
);
Expand Down
2 changes: 2 additions & 0 deletions public/pages/DetectorsList/utils/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,5 @@ export enum DETECTOR_ACTION {
STOP,
DELETE,
}

export const SINGLE_DETECTOR_ERROR_MSG = 'Not Found';
6 changes: 4 additions & 2 deletions public/redux/reducers/ad.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,9 +192,10 @@ const reducer = handleActions<Detectors>(
),
totalDetectors: action.result.data.response.totalDetectors,
}),
FAILURE: (state: Detectors): Detectors => ({
FAILURE: (state: Detectors, action: APIErrorAction): Detectors => ({
...state,
requesting: false,
errorMessage: action.error,
}),
},
[UPDATE_DETECTOR]: {
Expand All @@ -214,9 +215,10 @@ const reducer = handleActions<Detectors>(
},
},
}),
FAILURE: (state: Detectors): Detectors => ({
FAILURE: (state: Detectors, action: APIErrorAction): Detectors => ({
...state,
requesting: false,
errorMessage: action.error,
}),
},

Expand Down
19 changes: 13 additions & 6 deletions server/routes/ad.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,13 +460,17 @@ const getDetectors = async (
);
return detectorStateResp;
} catch (err) {
console.log(
'Anomaly detector - Unable to retrieve detector state',
err
console.log('Error getting detector profile ', err);
return Promise.reject(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer you log the err. Same for below changes.

Copy link
Contributor Author

@ohltyler ohltyler Aug 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I can add in more error logs. The way it is currently implemented, there will still be an error log when the thrown errors get caught in the outermost catch() block of the function.

new Error('Error retrieving all detector states')
);
}
});
const detectorStateResponses = await Promise.all(detectorStatePromises);
const detectorStateResponses = await Promise.all(
detectorStatePromises
).catch((err) => {
throw err;
});
const finalDetectorStates = getFinalDetectorStates(
detectorStateResponses,
finalDetectors
Expand All @@ -484,12 +488,15 @@ const getDetectors = async (
});
return detectorResp;
} catch (err) {
console.log('Anomaly detector - Unable to get detector job', err);
console.log('Error getting detector ', err);
return Promise.reject(new Error('Error retrieving all detectors'));
}
});
const detectorsWithJobResponses = await Promise.all(
detectorsWithJobPromises
);
).catch((err) => {
throw err;
});
const finalDetectorsWithJob = getDetectorsWithJob(
detectorsWithJobResponses
);
Expand Down