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

[Security Solution][Detections] Better toast errors #72205

Merged
merged 11 commits into from
Jul 17, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ import {
} from '../../../../../public/lists_plugin_deps';
import * as i18n from './translations';
import { TimelineNonEcsData, Ecs } from '../../../../graphql/types';
import { useAppToasts } from '../../../hooks/use_app_toasts';
import { useKibana } from '../../../lib/kibana';
import { errorToToaster, displaySuccessToast, useStateToaster } from '../../toasters';
import { ExceptionBuilder } from '../builder';
import { Loader } from '../../loader';
import { useAddOrUpdateException } from '../use_add_exception';
Expand Down Expand Up @@ -115,7 +115,7 @@ export const AddExceptionModal = memo(function AddExceptionModal({
Array<ExceptionListItemSchema | CreateExceptionListItemSchema>
>([]);
const [fetchOrCreateListError, setFetchOrCreateListError] = useState(false);
const [, dispatchToaster] = useStateToaster();
const { addError, addSuccess } = useAppToasts();
const { loading: isSignalIndexLoading, signalIndexName } = useSignalIndex();

const [{ isLoading: indexPatternLoading, indexPatterns }] = useFetchIndexPatterns(
Expand All @@ -124,15 +124,15 @@ export const AddExceptionModal = memo(function AddExceptionModal({

const onError = useCallback(
(error: Error) => {
errorToToaster({ title: i18n.ADD_EXCEPTION_ERROR, error, dispatchToaster });
addError(error, { title: i18n.ADD_EXCEPTION_ERROR });
onCancel();
},
[dispatchToaster, onCancel]
[addError, onCancel]
);
const onSuccess = useCallback(() => {
displaySuccessToast(i18n.ADD_EXCEPTION_SUCCESS, dispatchToaster);
addSuccess(i18n.ADD_EXCEPTION_SUCCESS);
onConfirm(shouldCloseAlert);
}, [dispatchToaster, onConfirm, shouldCloseAlert]);
}, [addSuccess, onConfirm, shouldCloseAlert]);

const [{ isLoading: addExceptionIsLoading }, addOrUpdateExceptionItems] = useAddOrUpdateException(
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import {
} from '../../../../../public/lists_plugin_deps';
import * as i18n from './translations';
import { useKibana } from '../../../lib/kibana';
import { errorToToaster, displaySuccessToast, useStateToaster } from '../../toasters';
import { useAppToasts } from '../../../hooks/use_app_toasts';
import { ExceptionBuilder } from '../builder';
import { useAddOrUpdateException } from '../use_add_exception';
import { AddExceptionComments } from '../add_exception_comments';
Expand Down Expand Up @@ -93,7 +93,7 @@ export const EditExceptionModal = memo(function EditExceptionModal({
const [exceptionItemsToAdd, setExceptionItemsToAdd] = useState<
Array<ExceptionListItemSchema | CreateExceptionListItemSchema>
>([]);
const [, dispatchToaster] = useStateToaster();
const { addError, addSuccess } = useAppToasts();
const { loading: isSignalIndexLoading, signalIndexName } = useSignalIndex();

const [{ isLoading: indexPatternLoading, indexPatterns }] = useFetchIndexPatterns(
Expand All @@ -102,15 +102,15 @@ export const EditExceptionModal = memo(function EditExceptionModal({

const onError = useCallback(
(error) => {
errorToToaster({ title: i18n.EDIT_EXCEPTION_ERROR, error, dispatchToaster });
addError(error, { title: i18n.EDIT_EXCEPTION_ERROR });
onCancel();
},
[dispatchToaster, onCancel]
[addError, onCancel]
);
const onSuccess = useCallback(() => {
displaySuccessToast(i18n.EDIT_EXCEPTION_SUCCESS, dispatchToaster);
addSuccess(i18n.EDIT_EXCEPTION_SUCCESS);
onConfirm();
}, [dispatchToaster, onConfirm]);
}, [addSuccess, onConfirm]);

const [{ isLoading: addExceptionIsLoading }, addOrUpdateExceptionItems] = useAddOrUpdateException(
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { isError } from 'lodash/fp';

import { AppToast, ActionToaster } from './';
import { isToasterError } from './errors';
import { isApiError } from '../../utils/api';
import { isAppError } from '../../utils/api';

/**
* Displays an error toast for the provided title and message
Expand Down Expand Up @@ -114,7 +114,7 @@ export const errorToToaster = ({
iconType,
errors: error.messages,
};
} else if (isApiError(error)) {
} else if (isAppError(error)) {
toast = {
id,
title,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { renderHook } from '@testing-library/react-hooks';

import { useToasts } from '../lib/kibana';
import { useAppToasts } from './use_app_toasts';

jest.mock('../lib/kibana');

describe('useDeleteList', () => {
let addErrorMock: jest.Mock;
let addSuccessMock: jest.Mock;

beforeEach(() => {
addErrorMock = jest.fn();
addSuccessMock = jest.fn();
(useToasts as jest.Mock).mockImplementation(() => ({
addError: addErrorMock,
addSuccess: addSuccessMock,
}));
});

it('works normally with a regular error', async () => {
const error = new Error('regular error');
const { result } = renderHook(() => useAppToasts());

result.current.addError(error, { title: 'title' });

expect(addErrorMock).toHaveBeenCalledWith(error, { title: 'title' });
});

it("uses a AppError's body.message as the toastMessage", async () => {
const kibanaApiError = {
message: 'Not Found',
body: { status_code: 404, message: 'Detailed Message' },
};

const { result } = renderHook(() => useAppToasts());

result.current.addError(kibanaApiError, { title: 'title' });

expect(addErrorMock).toHaveBeenCalledWith(kibanaApiError, {
title: 'title',
toastMessage: 'Detailed Message',
});
});

it('converts an unknown error to an Error', () => {
const unknownError = undefined;

const { result } = renderHook(() => useAppToasts());

result.current.addError(unknownError, { title: 'title' });

expect(addErrorMock).toHaveBeenCalledWith(Error(`${undefined}`), {
title: 'title',
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { useCallback, useRef } from 'react';

import { ErrorToastOptions, ToastsStart, Toast } from '../../../../../../src/core/public';
import { useToasts } from '../lib/kibana';
import { isAppError, AppError } from '../utils/api';

export type UseAppToasts = Pick<ToastsStart, 'addSuccess'> & {
api: ToastsStart;
addError: (error: unknown, options: ErrorToastOptions) => Toast;
};

export const useAppToasts = (): UseAppToasts => {
const toasts = useToasts();
const addError = useRef(toasts.addError.bind(toasts)).current;
const addSuccess = useRef(toasts.addSuccess.bind(toasts)).current;

const addAppError = useCallback(
(error: AppError, options: ErrorToastOptions) =>
addError(error, {
...options,
toastMessage: error.body.message,
}),
[addError]
);

const _addError = useCallback(
(error: unknown, options: ErrorToastOptions) => {
if (isAppError(error)) {
return addAppError(error, options);
} else {
if (error instanceof Error) {
return addError(error, options);
} else {
return addError(new Error(String(error)), options);
}
}
},
[addAppError, addError]
);

return { api: toasts, addError: _addError, addSuccess };
};
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,12 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { EuiGlobalToastListToast as Toast, EuiButtonIcon } from '@elastic/eui';
import { EuiButtonIcon } from '@elastic/eui';
import copy from 'copy-to-clipboard';
import React from 'react';
import uuid from 'uuid';

import * as i18n from './translations';
import { useStateToaster } from '../../components/toasters';
import { useAppToasts } from '../../hooks/use_app_toasts';

export type OnCopy = ({
content,
Expand All @@ -20,17 +19,6 @@ export type OnCopy = ({
isSuccess: boolean;
}) => void;

interface GetSuccessToastParams {
titleSummary?: string;
}

const getSuccessToast = ({ titleSummary }: GetSuccessToastParams): Toast => ({
id: `copy-success-${uuid.v4()}`,
color: 'success',
iconType: 'copyClipboard',
title: `${i18n.COPIED} ${titleSummary} ${i18n.TO_THE_CLIPBOARD}`,
});

interface Props {
children?: JSX.Element;
content: string | number;
Expand All @@ -40,7 +28,7 @@ interface Props {
}

export const Clipboard = ({ children, content, onCopy, titleSummary, toastLifeTimeMs }: Props) => {
const dispatchToaster = useStateToaster()[1];
const { addSuccess } = useAppToasts();
const onClick = (event: React.MouseEvent<HTMLButtonElement>) => {
event.preventDefault();
event.stopPropagation();
Expand All @@ -52,10 +40,7 @@ export const Clipboard = ({ children, content, onCopy, titleSummary, toastLifeTi
}

if (isSuccess) {
dispatchToaster({
type: 'addToaster',
toast: { toastLifeTimeMs, ...getSuccessToast({ titleSummary }) },
});
addSuccess(`${i18n.COPIED} ${titleSummary} ${i18n.TO_THE_CLIPBOARD}`, { toastLifeTimeMs });
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { notificationServiceMock } from '../../../../../../../../src/core/public/mocks';
import {
createKibanaContextProviderMock,
createUseUiSettingMock,
Expand All @@ -19,6 +20,7 @@ export const useUiSetting$ = jest.fn(createUseUiSetting$Mock());
export const useTimeZone = jest.fn();
export const useDateFormat = jest.fn();
export const useBasePath = jest.fn(() => '/test/base/path');
export const useToasts = jest.fn(() => notificationServiceMock.createStartContract().toasts);
export const useCurrentUser = jest.fn();
export const withKibana = jest.fn(createWithKibanaMock());
export const KibanaContextProvider = jest.fn(createKibanaContextProviderMock());
23 changes: 21 additions & 2 deletions x-pack/plugins/security_solution/public/common/utils/api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,33 @@

import { has } from 'lodash/fp';

export interface KibanaApiError {
export interface AppError {
name: string;
message: string;
body: {
message: string;
};
}

export interface KibanaError extends AppError {
body: {
message: string;
statusCode: number;
};
}

export interface SecurityAppError extends AppError {
body: {
message: string;
status_code: number;
};
}

export const isApiError = (error: unknown): error is KibanaApiError =>
export const isKibanaError = (error: unknown): error is KibanaError =>
has('message', error) && has('body.message', error) && has('body.statusCode', error);

export const isSecurityAppError = (error: unknown): error is SecurityAppError =>
has('message', error) && has('body.message', error) && has('body.status_code', error);

export const isAppError = (error: unknown): error is AppError =>
isKibanaError(error) || isSecurityAppError(error);
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ import {
useDeleteList,
useCursor,
} from '../../../shared_imports';
import { useToasts, useKibana } from '../../../common/lib/kibana';
import { useKibana } from '../../../common/lib/kibana';
import { useAppToasts } from '../../../common/hooks/use_app_toasts';
import { GenericDownloader } from '../../../common/components/generic_downloader';
import * as i18n from './translations';
import { ValueListsTable } from './table';
Expand All @@ -45,7 +46,7 @@ export const ValueListsModalComponent: React.FC<ValueListsModalProps> = ({
const { start: findLists, ...lists } = useFindLists();
const { start: deleteList, result: deleteResult } = useDeleteList();
const [exportListId, setExportListId] = useState<string>();
const toasts = useToasts();
const { addError, addSuccess } = useAppToasts();

const fetchLists = useCallback(() => {
findLists({ cursor, http, pageIndex: pageIndex + 1, pageSize });
Expand Down Expand Up @@ -82,21 +83,21 @@ export const ValueListsModalComponent: React.FC<ValueListsModalProps> = ({
const handleUploadError = useCallback(
(error: Error) => {
if (error.name !== 'AbortError') {
toasts.addError(error, { title: i18n.UPLOAD_ERROR });
addError(error, { title: i18n.UPLOAD_ERROR });
}
},
[toasts]
[addError]
);
const handleUploadSuccess = useCallback(
(response: ListSchema) => {
toasts.addSuccess({
addSuccess({
text: i18n.uploadSuccessMessage(response.name),
title: i18n.UPLOAD_SUCCESS_TITLE,
});
fetchLists();
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[toasts]
[addSuccess]
);

useEffect(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { useEffect, useState } from 'react';
import { errorToToaster, useStateToaster } from '../../../../common/components/toasters';
import { createSignalIndex, getSignalIndex } from './api';
import * as i18n from './translations';
import { isApiError } from '../../../../common/utils/api';
import { isSecurityAppError } from '../../../../common/utils/api';

type Func = () => void;

Expand Down Expand Up @@ -59,7 +59,7 @@ export const useSignalIndex = (): ReturnSignalIndex => {
signalIndexName: null,
createDeSignalIndex: createIndex,
});
if (isApiError(error) && error.body.status_code !== 404) {
if (isSecurityAppError(error) && error.body.status_code !== 404) {
errorToToaster({ title: i18n.SIGNAL_GET_NAME_FAILURE, error, dispatchToaster });
}
}
Expand All @@ -81,7 +81,7 @@ export const useSignalIndex = (): ReturnSignalIndex => {
}
} catch (error) {
if (isSubscribed) {
if (isApiError(error) && error.body.status_code === 409) {
if (isSecurityAppError(error) && error.body.status_code === 409) {
fetchData();
} else {
setSignalIndex({
Expand Down
Loading