Skip to content

Commit

Permalink
[ML] Fix error messages. (elastic#62575) (elastic#62599)
Browse files Browse the repository at this point in the history
Fixes two regressions:
- The migration to NP and its related http service changed the format of error messages, because of this, the output in toast error messages ended up less informational. This PR fixes it by reusing the ML plugin's updated getErrorMessage() function.
- The migration to NP and move to NP's useKibanaContext() introduced a regression where the modal overlays of error toast with long error message would no longer work anymore. This PR fixes it by passing on overlays as a component prop instead of using the context hook within the component.
  • Loading branch information
walterra authored Apr 6, 2020
1 parent 31644b0 commit 5f98516
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
import React from 'react';
import { render } from '@testing-library/react';

import { CoreStart } from 'src/core/public';

import { ToastNotificationText } from './toast_notification_text';

jest.mock('../../shared_imports');
Expand All @@ -15,6 +17,7 @@ jest.mock('../../app/app_dependencies');
describe('ToastNotificationText', () => {
test('should render the text as plain text', () => {
const props = {
overlays: {} as CoreStart['overlays'],
text: 'a short text message',
};
const { container } = render(<ToastNotificationText {...props} />);
Expand All @@ -23,6 +26,7 @@ describe('ToastNotificationText', () => {

test('should render the text within a modal', () => {
const props = {
overlays: {} as CoreStart['overlays'],
text:
'a text message that is longer than 140 characters. a text message that is longer than 140 characters. a text message that is longer than 140 characters. ',
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,20 @@ import {

import { i18n } from '@kbn/i18n';

import { toMountPoint } from '../../../../../../src/plugins/kibana_react/public';
import { CoreStart } from 'src/core/public';

import { useAppDependencies } from '../app_dependencies';
import { toMountPoint } from '../../../../../../src/plugins/kibana_react/public';

const MAX_SIMPLE_MESSAGE_LENGTH = 140;

export const ToastNotificationText: FC<{ text: any }> = ({ text }) => {
const { overlays } = useAppDependencies();
// Because of the use of `toMountPoint`, `useKibanaContext` doesn't work via `useAppDependencies`.
// That's why we need to pass in `overlays` as a prop cannot get it via context.
interface ToastNotificationTextProps {
overlays: CoreStart['overlays'];
text: any;
}

export const ToastNotificationText: FC<ToastNotificationTextProps> = ({ overlays, text }) => {
if (typeof text === 'string' && text.length <= MAX_SIMPLE_MESSAGE_LENGTH) {
return text;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,16 @@ import { toMountPoint } from '../../../../../../src/plugins/kibana_react/public'

import { TransformEndpointRequest, TransformEndpointResult } from '../../../common';

import { useToastNotifications } from '../app_dependencies';
import { getErrorMessage } from '../../shared_imports';

import { useAppDependencies, useToastNotifications } from '../app_dependencies';
import { TransformListRow, refreshTransformList$, REFRESH_TRANSFORM_LIST_STATE } from '../common';
import { ToastNotificationText } from '../components';

import { useApi } from './use_api';

export const useDeleteTransforms = () => {
const { overlays } = useAppDependencies();
const toastNotifications = useToastNotifications();
const api = useApi();

Expand Down Expand Up @@ -56,9 +59,7 @@ export const useDeleteTransforms = () => {
title: i18n.translate('xpack.transform.transformList.deleteTransformGenericErrorMessage', {
defaultMessage: 'An error occurred calling the API endpoint to delete transforms.',
}),
text: toMountPoint(
<ToastNotificationText text={e?.message ?? JSON.stringify(e, null, 2)} />
),
text: toMountPoint(<ToastNotificationText overlays={overlays} text={getErrorMessage(e)} />),
});
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ import { toMountPoint } from '../../../../../../../../../src/plugins/kibana_reac

import { PROGRESS_REFRESH_INTERVAL_MS } from '../../../../../../common/constants';

import { getErrorMessage } from '../../../../../shared_imports';

import { getTransformProgress, getDiscoverUrl } from '../../../../common';
import { useApi } from '../../../../hooks/use_api';
import { useAppDependencies, useToastNotifications } from '../../../../app_dependencies';
Expand Down Expand Up @@ -116,7 +118,9 @@ export const StepCreateForm: FC<Props> = React.memo(
defaultMessage: 'An error occurred creating the transform {transformId}:',
values: { transformId },
}),
text: toMountPoint(<ToastNotificationText text={e} />),
text: toMountPoint(
<ToastNotificationText overlays={deps.overlays} text={getErrorMessage(e)} />
),
});
setCreated(false);
setLoading(false);
Expand Down Expand Up @@ -157,7 +161,9 @@ export const StepCreateForm: FC<Props> = React.memo(
defaultMessage: 'An error occurred starting the transform {transformId}:',
values: { transformId },
}),
text: toMountPoint(<ToastNotificationText text={e} />),
text: toMountPoint(
<ToastNotificationText overlays={deps.overlays} text={getErrorMessage(e)} />
),
});
setStarted(false);
setLoading(false);
Expand Down Expand Up @@ -223,7 +229,9 @@ export const StepCreateForm: FC<Props> = React.memo(
'An error occurred creating the Kibana index pattern {indexPatternName}:',
values: { indexPatternName },
}),
text: toMountPoint(<ToastNotificationText text={e} />),
text: toMountPoint(
<ToastNotificationText overlays={deps.overlays} text={getErrorMessage(e)} />
),
});
setLoading(false);
return false;
Expand Down Expand Up @@ -260,7 +268,9 @@ export const StepCreateForm: FC<Props> = React.memo(
title: i18n.translate('xpack.transform.stepCreateForm.progressErrorMessage', {
defaultMessage: 'An error occurred getting the progress percentage:',
}),
text: toMountPoint(<ToastNotificationText text={e} />),
text: toMountPoint(
<ToastNotificationText overlays={deps.overlays} text={getErrorMessage(e)} />
),
});
clearInterval(interval);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import { toMountPoint } from '../../../../../../../../../src/plugins/kibana_reac
import { TransformId } from '../../../../../../common';
import { isValidIndexName } from '../../../../../../common/utils/es_utils';

import { getErrorMessage } from '../../../../../shared_imports';

import { useAppDependencies, useToastNotifications } from '../../../../app_dependencies';
import { ToastNotificationText } from '../../../../components';
import { useDocumentationLinks } from '../../../../hooks/use_documentation_links';
Expand Down Expand Up @@ -116,7 +118,9 @@ export const StepDetailsForm: FC<Props> = React.memo(
title: i18n.translate('xpack.transform.stepDetailsForm.errorGettingTransformList', {
defaultMessage: 'An error occurred getting the existing transform IDs:',
}),
text: toMountPoint(<ToastNotificationText text={e} />),
text: toMountPoint(
<ToastNotificationText overlays={deps.overlays} text={getErrorMessage(e)} />
),
});
}

Expand All @@ -127,7 +131,9 @@ export const StepDetailsForm: FC<Props> = React.memo(
title: i18n.translate('xpack.transform.stepDetailsForm.errorGettingIndexNames', {
defaultMessage: 'An error occurred getting the existing index names:',
}),
text: toMountPoint(<ToastNotificationText text={e} />),
text: toMountPoint(
<ToastNotificationText overlays={deps.overlays} text={getErrorMessage(e)} />
),
});
}

Expand All @@ -141,7 +147,9 @@ export const StepDetailsForm: FC<Props> = React.memo(
defaultMessage: 'An error occurred getting the existing index pattern titles:',
}
),
text: toMountPoint(<ToastNotificationText text={e} />),
text: toMountPoint(
<ToastNotificationText overlays={deps.overlays} text={getErrorMessage(e)} />
),
});
}
})();
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/transform/public/shared_imports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,5 @@ export {
UseRequestConfig,
useRequest,
} from '../../../../src/plugins/es_ui_shared/public/request/np_ready_request';

export { getErrorMessage } from '../../ml/common/util/errors';

0 comments on commit 5f98516

Please sign in to comment.