From 0b93a956db6b1ac9e2db5a57e77c690372f76bb7 Mon Sep 17 00:00:00 2001 From: Saarika Bhasi <55930906+saarikabhasi@users.noreply.github.com> Date: Thu, 25 May 2023 08:57:45 -0400 Subject: [PATCH] [Enterprise Search] [Search Application] Fix error message when creating duplicate search application (#158333) ## Summar * Instead of showing Status Code **502: Internal Server Error**, Shows **409 : Conflict** * Shows `"Search application name already taken. Choose another name"` when duplicate search application is created. ## Screen Recording https://github.com/elastic/kibana/assets/55930906/96615f59-6fa2-4e4c-9758-f128ec56e455 Response --- .../common/types/error_codes.ts | 1 + .../engines/create_engine_logic.test.ts | 15 ++++++- .../routes/enterprise_search/engines.test.ts | 26 ++++++++++++ .../routes/enterprise_search/engines.ts | 40 ++++++++++++++----- .../server/utils/identify_exceptions.ts | 3 ++ 5 files changed, 75 insertions(+), 10 deletions(-) diff --git a/x-pack/plugins/enterprise_search/common/types/error_codes.ts b/x-pack/plugins/enterprise_search/common/types/error_codes.ts index 0ed9a2e1fad99..fe1a2a266dba5 100644 --- a/x-pack/plugins/enterprise_search/common/types/error_codes.ts +++ b/x-pack/plugins/enterprise_search/common/types/error_codes.ts @@ -20,6 +20,7 @@ export enum ErrorCode { PIPELINE_IS_IN_USE = 'pipeline_is_in_use', PIPELINE_NOT_FOUND = 'pipeline_not_found', RESOURCE_NOT_FOUND = 'resource_not_found', + SEARCH_APPLICATION_ALREADY_EXISTS = 'search_application_already_exists', UNAUTHORIZED = 'unauthorized', UNCAUGHT_EXCEPTION = 'uncaught_exception', } diff --git a/x-pack/plugins/enterprise_search/public/applications/applications/components/engines/create_engine_logic.test.ts b/x-pack/plugins/enterprise_search/public/applications/applications/components/engines/create_engine_logic.test.ts index 23955708fff8a..7147d061836de 100644 --- a/x-pack/plugins/enterprise_search/public/applications/applications/components/engines/create_engine_logic.test.ts +++ b/x-pack/plugins/enterprise_search/public/applications/applications/components/engines/create_engine_logic.test.ts @@ -7,7 +7,7 @@ import { LogicMounter } from '../../../__mocks__/kea_logic'; -import { Status } from '../../../../../common/types/api'; +import { HttpError, Status } from '../../../../../common/types/api'; import { KibanaLogic } from '../../../shared/kibana'; import { CreateEngineApiLogic } from '../../api/engines/create_engine_api_logic'; @@ -76,6 +76,19 @@ describe('CreateEngineLogic', () => { indices: ['search-index-01'], }); }); + it('createEngine returns error when duplicate search application is created', () => { + const httpError: HttpError = { + body: { + error: 'search_application_already_exists', + message: 'Search application name already taken. Choose another name.', + statusCode: 409, + }, + fetchOptions: {}, + request: {}, + } as HttpError; + CreateEngineApiLogic.actions.apiError(httpError); + expect(CreateEngineLogic.values.createEngineError).toEqual(httpError); + }); it('engineCreated is handled and is navigated to Search application list page', () => { jest.spyOn(CreateEngineLogic.actions, 'fetchEngines'); diff --git a/x-pack/plugins/enterprise_search/server/routes/enterprise_search/engines.test.ts b/x-pack/plugins/enterprise_search/server/routes/enterprise_search/engines.test.ts index 985a6322489d7..f637e5ac9901f 100644 --- a/x-pack/plugins/enterprise_search/server/routes/enterprise_search/engines.test.ts +++ b/x-pack/plugins/enterprise_search/server/routes/enterprise_search/engines.test.ts @@ -266,6 +266,32 @@ describe('engines routes', () => { mockRouter.shouldThrow(request); }); + + it('returns 409 when upsert create search application throws error', async () => { + (mockClient.asCurrentUser.transport.request as jest.Mock).mockRejectedValueOnce({ + meta: { + body: { + error: { + type: 'version_conflict_engine_exception', + }, + }, + statusCode: 409, + }, + name: 'elasticsearch-js', + }); + await mockRouter.callRoute({ + params: { engine_name: 'engine-name' }, + }); + expect(mockRouter.response.customError).toHaveBeenCalledWith({ + body: { + attributes: { + error_code: 'search_application_already_exists', + }, + message: 'Search application name already taken. Choose another name.', + }, + statusCode: 409, + }); + }); }); describe('DELETE /internal/enterprise_search/engines/{engine_name}', () => { diff --git a/x-pack/plugins/enterprise_search/server/routes/enterprise_search/engines.ts b/x-pack/plugins/enterprise_search/server/routes/enterprise_search/engines.ts index c8544e7343815..7d49f5b879a26 100644 --- a/x-pack/plugins/enterprise_search/server/routes/enterprise_search/engines.ts +++ b/x-pack/plugins/enterprise_search/server/routes/enterprise_search/engines.ts @@ -6,6 +6,7 @@ */ import { SearchResponse, AcknowledgedResponseBase } from '@elastic/elasticsearch/lib/api/types'; import { schema } from '@kbn/config-schema'; +import { i18n } from '@kbn/i18n'; import { EnterpriseSearchEngine, @@ -21,7 +22,10 @@ import { RouteDependencies } from '../../plugin'; import { createError } from '../../utils/create_error'; import { elasticsearchErrorHandler } from '../../utils/elasticsearch_error_handler'; -import { isNotFoundException } from '../../utils/identify_exceptions'; +import { + isNotFoundException, + isVersionConflictEngineException, +} from '../../utils/identify_exceptions'; export function registerEnginesRoutes({ log, router }: RouteDependencies) { router.get( @@ -88,14 +92,32 @@ export function registerEnginesRoutes({ log, router }: RouteDependencies) { }, elasticsearchErrorHandler(log, async (context, request, response) => { const { client } = (await context.core).elasticsearch; - const engine = - await client.asCurrentUser.transport.request({ - body: { indices: request.body.indices }, - method: 'PUT', - path: `/_application/search_application/${request.params.engine_name}`, - querystring: request.query, - }); - return response.ok({ body: engine }); + try { + const engine = + await client.asCurrentUser.transport.request({ + body: { indices: request.body.indices }, + method: 'PUT', + path: `/_application/search_application/${request.params.engine_name}`, + querystring: request.query, + }); + return response.ok({ body: engine }); + } catch (error) { + if (isVersionConflictEngineException(error)) { + return createError({ + errorCode: ErrorCode.SEARCH_APPLICATION_ALREADY_EXISTS, + message: i18n.translate( + 'xpack.enterpriseSearch.server.routes.createSearchApplication.searchApplciationExistsError', + { + defaultMessage: 'Search application name already taken. Choose another name.', + } + ), + response, + statusCode: 409, + }); + } + + throw error; + } }) ); diff --git a/x-pack/plugins/enterprise_search/server/utils/identify_exceptions.ts b/x-pack/plugins/enterprise_search/server/utils/identify_exceptions.ts index 04b8a089fa009..915d80312ee79 100644 --- a/x-pack/plugins/enterprise_search/server/utils/identify_exceptions.ts +++ b/x-pack/plugins/enterprise_search/server/utils/identify_exceptions.ts @@ -39,3 +39,6 @@ export const isNotFoundException = (error: ElasticsearchResponseError) => export const isIllegalArgumentException = (error: ElasticsearchResponseError) => error.meta?.body?.error?.type === 'illegal_argument_exception'; + +export const isVersionConflictEngineException = (error: ElasticsearchResponseError) => + error.meta?.body?.error?.type === 'version_conflict_engine_exception';