Skip to content

Commit

Permalink
[Enterprise Search] Show error panel on API call errors (elastic#156273)
Browse files Browse the repository at this point in the history
## Summary

This PR adds generic error handling for API calls that manage the ELSER
model: creation (download), fetching and starting the deployment. If an
error occurs, the ELSER panel is replaced with the error panel. The user
can navigate to the ML Notifications page with a link, which might
contain diagnostic information about the error.


![ELSER_error_handling](https://user-images.githubusercontent.com/14224983/235535266-93e8e822-0603-4a10-8aab-ec77cd167bf6.gif)


### Checklist

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [x] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))

---------

Co-authored-by: kibanamachine <[email protected]>
  • Loading branch information
demjened and kibanamachine authored May 2, 2023
1 parent 291cdb6 commit 5263339
Show file tree
Hide file tree
Showing 11 changed files with 220 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ export const createTextExpansionModel = async (): Promise<CreateTextExpansionMod

export const CreateTextExpansionModelApiLogic = createApiLogic(
['create_text_expansion_model_api_logic'],
createTextExpansionModel
createTextExpansionModel,
{ showErrorFlash: false }
);

export type CreateTextExpansionModelApiLogicActions = Actions<
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ export const fetchTextExpansionModelStatus = async () => {

export const FetchTextExpansionModelApiLogic = createApiLogic(
['fetch_text_expansion_model_api_logic'],
fetchTextExpansionModelStatus
fetchTextExpansionModelStatus,
{ showErrorFlash: false }
);

export type FetchTextExpansionModelApiLogicActions = Actions<
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ export const startTextExpansionModel = async (): Promise<StartTextExpansionModel

export const StartTextExpansionModelApiLogic = createApiLogic(
['start_text_expansion_model_api_logic'],
startTextExpansionModel
startTextExpansionModel,
{ showErrorFlash: false }
);

export type StartTextExpansionModelApiLogicActions = Actions<
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import { shallow } from 'enzyme';

import { EuiButton } from '@elastic/eui';

import { HttpError } from '../../../../../../../common/types/api';

import {
TextExpansionCallOut,
DeployModel,
Expand All @@ -22,6 +24,8 @@ import {
ModelStarted,
} from './text_expansion_callout';

import { TextExpansionErrors } from './text_expansion_errors';

jest.mock('./text_expansion_callout_data', () => ({
useTextExpansionCallOutData: jest.fn(() => ({
dismiss: jest.fn(),
Expand All @@ -33,6 +37,7 @@ jest.mock('./text_expansion_callout_data', () => ({
}));

const DEFAULT_VALUES = {
startTextExpansionModelError: undefined,
isCreateButtonDisabled: false,
isModelDownloadInProgress: false,
isModelDownloaded: false,
Expand All @@ -45,6 +50,21 @@ describe('TextExpansionCallOut', () => {
jest.clearAllMocks();
setMockValues(DEFAULT_VALUES);
});
it('renders error panel instead of normal panel if there are some errors', () => {
setMockValues({
...DEFAULT_VALUES,
startTextExpansionModelError: {
body: {
error: 'some-error',
message: 'some-error-message',
statusCode: 500,
},
} as HttpError,
});

const wrapper = shallow(<TextExpansionCallOut />);
expect(wrapper.find(TextExpansionErrors).length).toBe(1);
});
it('renders panel with deployment instructions if the model is not deployed', () => {
const wrapper = shallow(<TextExpansionCallOut />);
expect(wrapper.find(DeployModel).length).toBe(1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ import { docLinks } from '../../../../../shared/doc_links';
import { KibanaLogic } from '../../../../../shared/kibana';

import { useTextExpansionCallOutData } from './text_expansion_callout_data';
import { TextExpansionCalloutLogic } from './text_expansion_callout_logic';
import { getTextExpansionError, TextExpansionCalloutLogic } from './text_expansion_callout_logic';
import { TextExpansionErrors } from './text_expansion_errors';

export interface TextExpansionCallOutState {
dismiss: () => void;
Expand Down Expand Up @@ -327,13 +328,24 @@ export const ModelStarted = ({
export const TextExpansionCallOut: React.FC<TextExpansionCallOutProps> = (props) => {
const { dismiss, isDismissable, show } = useTextExpansionCallOutData(props);
const {
createTextExpansionModelError,
fetchTextExpansionModelError,
isCreateButtonDisabled,
isModelDownloadInProgress,
isModelDownloaded,
isModelStarted,
isStartButtonDisabled,
startTextExpansionModelError,
} = useValues(TextExpansionCalloutLogic);

// In case of an error, show the error callout only
const error = getTextExpansionError(
createTextExpansionModelError,
fetchTextExpansionModelError,
startTextExpansionModelError
);
if (error) return <TextExpansionErrors error={error} />;

if (!show) return null;

if (isModelDownloadInProgress) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,25 +9,29 @@ import { LogicMounter } from '../../../../../__mocks__/kea_logic';

import { HttpResponse } from '@kbn/core/public';

import { ErrorResponse, Status } from '../../../../../../../common/types/api';
import { ErrorResponse, HttpError, Status } from '../../../../../../../common/types/api';
import { MlModelDeploymentState } from '../../../../../../../common/types/ml';
import { CreateTextExpansionModelApiLogic } from '../../../../api/ml_models/text_expansion/create_text_expansion_model_api_logic';
import { FetchTextExpansionModelApiLogic } from '../../../../api/ml_models/text_expansion/fetch_text_expansion_model_api_logic';

import {
getTextExpansionError,
TextExpansionCalloutLogic,
TextExpansionCalloutValues,
} from './text_expansion_callout_logic';

const DEFAULT_VALUES: TextExpansionCalloutValues = {
createTextExpansionModelError: undefined,
createTextExpansionModelStatus: Status.IDLE,
createdTextExpansionModel: undefined,
fetchTextExpansionModelError: undefined,
isCreateButtonDisabled: false,
isModelDownloadInProgress: false,
isModelDownloaded: false,
isModelStarted: false,
isPollingTextExpansionModelActive: false,
isStartButtonDisabled: false,
startTextExpansionModelError: undefined,
startTextExpansionModelStatus: Status.IDLE,
textExpansionModel: undefined,
textExpansionModelPollTimeoutId: null,
Expand Down Expand Up @@ -55,6 +59,37 @@ describe('TextExpansionCalloutLogic', () => {
expect(TextExpansionCalloutLogic.values).toEqual(DEFAULT_VALUES);
});

describe('getTextExpansionError', () => {
const error = {
body: {
error: 'some-error',
message: 'some-error-message',
statusCode: 500,
},
} as HttpError;
it('returns null if there is no error', () => {
expect(getTextExpansionError(undefined, undefined, undefined)).toBe(null);
});
it('uses the correct title and message from a create error', () => {
expect(getTextExpansionError(error, undefined, undefined)).toEqual({
title: 'Error with ELSER deployment',
message: error.body?.message,
});
});
it('uses the correct title and message from a fetch error', () => {
expect(getTextExpansionError(undefined, error, undefined)).toEqual({
title: 'Error fetching ELSER model',
message: error.body?.message,
});
});
it('uses the correct title and message from a start error', () => {
expect(getTextExpansionError(undefined, undefined, error)).toEqual({
title: 'Error starting ELSER deployment',
message: error.body?.message,
});
});
});

describe('listeners', () => {
describe('createTextExpansionModelPollingTimeout', () => {
const duration = 5000;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@

import { kea, MakeLogicType } from 'kea';

import { Status } from '../../../../../../../common/types/api';
import { i18n } from '@kbn/i18n';

import { HttpError, Status } from '../../../../../../../common/types/api';
import { MlModelDeploymentState } from '../../../../../../../common/types/ml';
import { getErrorsFromHttpResponse } from '../../../../../shared/flash_messages/handle_api_errors';
import {
CreateTextExpansionModelApiLogic,
CreateTextExpansionModelApiLogicActions,
Expand Down Expand Up @@ -46,19 +49,67 @@ interface TextExpansionCalloutActions {
}

export interface TextExpansionCalloutValues {
createTextExpansionModelError: HttpError | undefined;
createTextExpansionModelStatus: Status;
createdTextExpansionModel: CreateTextExpansionModelResponse | undefined;
fetchTextExpansionModelError: HttpError | undefined;
isCreateButtonDisabled: boolean;
isModelDownloadInProgress: boolean;
isModelDownloaded: boolean;
isModelStarted: boolean;
isPollingTextExpansionModelActive: boolean;
isStartButtonDisabled: boolean;
startTextExpansionModelError: HttpError | undefined;
startTextExpansionModelStatus: Status;
textExpansionModel: FetchTextExpansionModelResponse | undefined;
textExpansionModelPollTimeoutId: null | ReturnType<typeof setTimeout>;
}

/**
* Extracts the topmost error in precedence order (create > start > fetch).
* @param createError
* @param fetchError
* @param startError
* @returns the extracted error or null if there is no error
*/
export const getTextExpansionError = (
createError: HttpError | undefined,
fetchError: HttpError | undefined,
startError: HttpError | undefined
) => {
return createError !== undefined
? {
title: i18n.translate(
'xpack.enterpriseSearch.content.indices.pipelines.textExpansionCreateError.title',
{
defaultMessage: 'Error with ELSER deployment',
}
),
message: getErrorsFromHttpResponse(createError)[0],
}
: startError !== undefined
? {
title: i18n.translate(
'xpack.enterpriseSearch.content.indices.pipelines.textExpansionStartError.title',
{
defaultMessage: 'Error starting ELSER deployment',
}
),
message: getErrorsFromHttpResponse(startError)[0],
}
: fetchError !== undefined
? {
title: i18n.translate(
'xpack.enterpriseSearch.content.indices.pipelines.textExpansionFetchError.title',
{
defaultMessage: 'Error fetching ELSER model',
}
),
message: getErrorsFromHttpResponse(fetchError)[0],
}
: null;
};

export const TextExpansionCalloutLogic = kea<
MakeLogicType<TextExpansionCalloutValues, TextExpansionCalloutActions>
>({
Expand All @@ -74,23 +125,35 @@ export const TextExpansionCalloutLogic = kea<
connect: {
actions: [
CreateTextExpansionModelApiLogic,
['makeRequest as createTextExpansionModel', 'apiSuccess as createTextExpansionModelSuccess'],
[
'makeRequest as createTextExpansionModel',
'apiSuccess as createTextExpansionModelSuccess',
'apiError as createTextExpansionModelError',
],
FetchTextExpansionModelApiLogic,
[
'makeRequest as fetchTextExpansionModel',
'apiSuccess as fetchTextExpansionModelSuccess',
'apiError as fetchTextExpansionModelError',
],
StartTextExpansionModelApiLogic,
['makeRequest as startTextExpansionModel', 'apiSuccess as startTextExpansionModelSuccess'],
[
'makeRequest as startTextExpansionModel',
'apiSuccess as startTextExpansionModelSuccess',
'apiError as startTextExpansionModelError',
],
],
values: [
CreateTextExpansionModelApiLogic,
['data as createdTextExpansionModel', 'status as createTextExpansionModelStatus'],
[
'data as createdTextExpansionModel',
'status as createTextExpansionModelStatus',
'error as createTextExpansionModelError',
],
FetchTextExpansionModelApiLogic,
['data as textExpansionModel'],
['data as textExpansionModel', 'error as fetchTextExpansionModelError'],
StartTextExpansionModelApiLogic,
['status as startTextExpansionModelStatus'],
['status as startTextExpansionModelStatus', 'error as startTextExpansionModelError'],
],
},
events: ({ actions, values }) => ({
Expand Down Expand Up @@ -169,7 +232,7 @@ export const TextExpansionCalloutLogic = kea<
selectors: ({ selectors }) => ({
isCreateButtonDisabled: [
() => [selectors.createTextExpansionModelStatus],
(status: Status) => status !== Status.IDLE,
(status: Status) => status !== Status.IDLE && status !== Status.ERROR,
],
isModelDownloadInProgress: [
() => [selectors.textExpansionModel],
Expand All @@ -195,7 +258,7 @@ export const TextExpansionCalloutLogic = kea<
],
isStartButtonDisabled: [
() => [selectors.startTextExpansionModelStatus],
(status: Status) => status !== Status.IDLE,
(status: Status) => status !== Status.IDLE && status !== Status.ERROR,
],
}),
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { setMockValues } from '../../../../../__mocks__/kea_logic';

import React from 'react';

import { shallow } from 'enzyme';

import { EuiCallOut } from '@elastic/eui';

import { TextExpansionErrors } from './text_expansion_errors';

describe('TextExpansionErrors', () => {
beforeEach(() => {
jest.clearAllMocks();
setMockValues({});
});
const error = {
title: 'some-error-title',
message: 'some-error-message',
};
it('extracts error panel with the given title and message', () => {
const wrapper = shallow(<TextExpansionErrors error={error} />);
expect(wrapper.find(EuiCallOut).length).toBe(1);
expect(wrapper.find(EuiCallOut).prop('title')).toEqual(error.title);
expect(wrapper.find(EuiCallOut).find('p').length).toBe(1);
expect(wrapper.find(EuiCallOut).find('p').text()).toEqual(error.message);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import React from 'react';

import { useValues } from 'kea';

import { EuiCallOut, EuiLink } from '@elastic/eui';

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

import { HttpLogic } from '../../../../../shared/http';

import { ML_NOTIFICATIONS_PATH } from '../../../../routes';

export const TextExpansionErrors = ({ error }: { error: { title: string; message: string } }) => {
const { http } = useValues(HttpLogic);

return (
<>
<EuiCallOut color="danger" iconType="error" title={error.title}>
<p>{error.message}</p>
<EuiLink href={http.basePath.prepend(ML_NOTIFICATIONS_PATH)} target="_blank">
{i18n.translate(
'xpack.enterpriseSearch.content.indices.pipelines.textExpansionCreateError.mlNotificationsLink',
{
defaultMessage: 'Machine Learning notifications',
}
)}
</EuiLink>
</EuiCallOut>
</>
);
};
Loading

0 comments on commit 5263339

Please sign in to comment.