Skip to content

Commit

Permalink
[Enterprise Search] [Search Application] Fix error message when creat…
Browse files Browse the repository at this point in the history
…ing 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

<img width="1728" alt="Response"
src="https://github.com/elastic/kibana/assets/55930906/56368b62-1799-4749-a019-67154dfb1cca">
  • Loading branch information
saarikabhasi authored May 25, 2023
1 parent 59c32ce commit 0b93a95
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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',
}
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(
Expand Down Expand Up @@ -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<EnterpriseSearchEngineUpsertResponse>({
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<EnterpriseSearchEngineUpsertResponse>({
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;
}
})
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

0 comments on commit 0b93a95

Please sign in to comment.