From ee7ae637c90b463e13be59295958a181d315bc13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yulia=20=C4=8Cech?= <6585477+yuliacech@users.noreply.github.com> Date: Fri, 16 Oct 2020 18:02:20 +0200 Subject: [PATCH] [ILM] Add esErrorHandler for the new es js client (#80302) (#80830) * [ILM] Add esErrorHandler for the new es js client * [ILM] Fix function call params * Rename function * Rename function * Rename function * [ILM] Change import of handleEsError to lib dependency * [ILM] Add comments about legacy and new es js client * [ILM] Add type casting for ResponseError Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../errors/handle_es_error.ts | 43 +++++++++++++++++++ .../errors/index.ts | 1 + .../errors/is_es_error.ts | 4 ++ .../es_ui_shared/server/errors/index.ts | 2 +- src/plugins/es_ui_shared/server/index.ts | 2 +- .../server/plugin.ts | 4 ++ .../api/index/register_add_policy_route.ts | 17 +++----- .../routes/api/index/register_remove_route.ts | 17 +++----- .../routes/api/index/register_retry_route.ts | 13 ++---- .../api/nodes/register_details_route.ts | 17 +++----- .../routes/api/nodes/register_list_route.ts | 18 ++++---- .../api/policies/register_create_route.ts | 17 +++----- .../api/policies/register_delete_route.ts | 17 +++----- .../api/policies/register_fetch_route.ts | 13 ++---- .../snapshot_policies/register_fetch_route.ts | 13 ++---- .../templates/register_add_policy_route.ts | 17 +++----- .../api/templates/register_fetch_route.ts | 13 ++---- .../server/shared_imports.ts | 7 +++ .../server/types.ts | 4 ++ 19 files changed, 127 insertions(+), 112 deletions(-) create mode 100644 src/plugins/es_ui_shared/__packages_do_not_import__/errors/handle_es_error.ts create mode 100644 x-pack/plugins/index_lifecycle_management/server/shared_imports.ts diff --git a/src/plugins/es_ui_shared/__packages_do_not_import__/errors/handle_es_error.ts b/src/plugins/es_ui_shared/__packages_do_not_import__/errors/handle_es_error.ts new file mode 100644 index 00000000000000..1c3643898dd1d9 --- /dev/null +++ b/src/plugins/es_ui_shared/__packages_do_not_import__/errors/handle_es_error.ts @@ -0,0 +1,43 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { ApiError } from '@elastic/elasticsearch'; +import { ResponseError } from '@elastic/elasticsearch/lib/errors'; +import { IKibanaResponse, KibanaResponseFactory } from 'kibana/server'; + +interface EsErrorHandlerParams { + error: ApiError; + response: KibanaResponseFactory; +} + +/* + * For errors returned by the new elasticsearch js client. + */ +export const handleEsError = ({ error, response }: EsErrorHandlerParams): IKibanaResponse => { + // error.name is slightly better in terms of performance, since all errors now have name property + if (error.name === 'ResponseError') { + const { statusCode, body } = error as ResponseError; + return response.customError({ + statusCode, + body: { message: body.error?.reason }, + }); + } + // Case: default + return response.internalError({ body: error }); +}; diff --git a/src/plugins/es_ui_shared/__packages_do_not_import__/errors/index.ts b/src/plugins/es_ui_shared/__packages_do_not_import__/errors/index.ts index 0d025442f4a926..484dc17868ab0d 100644 --- a/src/plugins/es_ui_shared/__packages_do_not_import__/errors/index.ts +++ b/src/plugins/es_ui_shared/__packages_do_not_import__/errors/index.ts @@ -18,3 +18,4 @@ */ export { isEsError } from './is_es_error'; +export { handleEsError } from './handle_es_error'; diff --git a/src/plugins/es_ui_shared/__packages_do_not_import__/errors/is_es_error.ts b/src/plugins/es_ui_shared/__packages_do_not_import__/errors/is_es_error.ts index 1e212307ca1ccf..80a53aac328a46 100644 --- a/src/plugins/es_ui_shared/__packages_do_not_import__/errors/is_es_error.ts +++ b/src/plugins/es_ui_shared/__packages_do_not_import__/errors/is_es_error.ts @@ -25,6 +25,10 @@ interface RequestError extends Error { statusCode?: number; } +/* + * @deprecated + * Only works with legacy elasticsearch js client errors and will be removed after 7.x last + */ export function isEsError(err: RequestError) { const isInstanceOfEsError = err instanceof esErrorsParent; const hasStatusCode = Boolean(err.statusCode); diff --git a/src/plugins/es_ui_shared/server/errors/index.ts b/src/plugins/es_ui_shared/server/errors/index.ts index c18374cd9ec315..532e02774ff502 100644 --- a/src/plugins/es_ui_shared/server/errors/index.ts +++ b/src/plugins/es_ui_shared/server/errors/index.ts @@ -17,4 +17,4 @@ * under the License. */ -export { isEsError } from '../../__packages_do_not_import__/errors'; +export { isEsError, handleEsError } from '../../__packages_do_not_import__/errors'; diff --git a/src/plugins/es_ui_shared/server/index.ts b/src/plugins/es_ui_shared/server/index.ts index 0118bbda53262b..b2c9c85d956bac 100644 --- a/src/plugins/es_ui_shared/server/index.ts +++ b/src/plugins/es_ui_shared/server/index.ts @@ -17,7 +17,7 @@ * under the License. */ -export { isEsError } from './errors'; +export { isEsError, handleEsError } from './errors'; /** dummy plugin*/ export function plugin() { diff --git a/x-pack/plugins/index_lifecycle_management/server/plugin.ts b/x-pack/plugins/index_lifecycle_management/server/plugin.ts index 40037d0c1e7775..e87f4aa17c0ef2 100644 --- a/x-pack/plugins/index_lifecycle_management/server/plugin.ts +++ b/x-pack/plugins/index_lifecycle_management/server/plugin.ts @@ -14,6 +14,7 @@ import { PluginInitializerContext, LegacyAPICaller, } from 'src/core/server'; +import { handleEsError } from './shared_imports'; import { Index as IndexWithoutIlm } from '../../index_management/common/types'; import { PLUGIN } from '../common/constants'; @@ -99,6 +100,9 @@ export class IndexLifecycleManagementServerPlugin implements Plugin { @@ -47,15 +51,8 @@ export function registerAddPolicyRoute({ router, license }: RouteDependencies) { alias ); return response.ok(); - } catch (e) { - if (e.name === 'ResponseError') { - return response.customError({ - statusCode: e.statusCode, - body: { message: e.body.error?.reason }, - }); - } - // Case: default - return response.internalError({ body: e }); + } catch (error) { + return handleEsError({ error, response }); } }) ); diff --git a/x-pack/plugins/index_lifecycle_management/server/routes/api/index/register_remove_route.ts b/x-pack/plugins/index_lifecycle_management/server/routes/api/index/register_remove_route.ts index a83a3fa1378c89..15c3e7b866c790 100644 --- a/x-pack/plugins/index_lifecycle_management/server/routes/api/index/register_remove_route.ts +++ b/x-pack/plugins/index_lifecycle_management/server/routes/api/index/register_remove_route.ts @@ -26,7 +26,11 @@ const bodySchema = schema.object({ indexNames: schema.arrayOf(schema.string()), }); -export function registerRemoveRoute({ router, license }: RouteDependencies) { +export function registerRemoveRoute({ + router, + license, + lib: { handleEsError }, +}: RouteDependencies) { router.post( { path: addBasePath('/index/remove'), validate: { body: bodySchema } }, license.guardApiRoute(async (context, request, response) => { @@ -36,15 +40,8 @@ export function registerRemoveRoute({ router, license }: RouteDependencies) { try { await removeLifecycle(context.core.elasticsearch.client.asCurrentUser, indexNames); return response.ok(); - } catch (e) { - if (e.name === 'ResponseError') { - return response.customError({ - statusCode: e.statusCode, - body: { message: e.body.error?.reason }, - }); - } - // Case: default - return response.internalError({ body: e }); + } catch (error) { + return handleEsError({ error, response }); } }) ); diff --git a/x-pack/plugins/index_lifecycle_management/server/routes/api/index/register_retry_route.ts b/x-pack/plugins/index_lifecycle_management/server/routes/api/index/register_retry_route.ts index cdcf5ed4b7ac4f..28bced0fb5a8f1 100644 --- a/x-pack/plugins/index_lifecycle_management/server/routes/api/index/register_retry_route.ts +++ b/x-pack/plugins/index_lifecycle_management/server/routes/api/index/register_retry_route.ts @@ -27,7 +27,7 @@ const bodySchema = schema.object({ indexNames: schema.arrayOf(schema.string()), }); -export function registerRetryRoute({ router, license }: RouteDependencies) { +export function registerRetryRoute({ router, license, lib: { handleEsError } }: RouteDependencies) { router.post( { path: addBasePath('/index/retry'), validate: { body: bodySchema } }, license.guardApiRoute(async (context, request, response) => { @@ -37,15 +37,8 @@ export function registerRetryRoute({ router, license }: RouteDependencies) { try { await retryLifecycle(context.core.elasticsearch.client.asCurrentUser, indexNames); return response.ok(); - } catch (e) { - if (e.name === 'ResponseError') { - return response.customError({ - statusCode: e.statusCode, - body: { message: e.body.error?.reason }, - }); - } - // Case: default - return response.internalError({ body: e }); + } catch (error) { + return handleEsError({ error, response }); } }) ); diff --git a/x-pack/plugins/index_lifecycle_management/server/routes/api/nodes/register_details_route.ts b/x-pack/plugins/index_lifecycle_management/server/routes/api/nodes/register_details_route.ts index 57034af324ed52..41b93ba59e3f53 100644 --- a/x-pack/plugins/index_lifecycle_management/server/routes/api/nodes/register_details_route.ts +++ b/x-pack/plugins/index_lifecycle_management/server/routes/api/nodes/register_details_route.ts @@ -29,7 +29,11 @@ const paramsSchema = schema.object({ nodeAttrs: schema.string(), }); -export function registerDetailsRoute({ router, license }: RouteDependencies) { +export function registerDetailsRoute({ + router, + license, + lib: { handleEsError }, +}: RouteDependencies) { router.get( { path: addBasePath('/nodes/{nodeAttrs}/details'), validate: { params: paramsSchema } }, license.guardApiRoute(async (context, request, response) => { @@ -40,15 +44,8 @@ export function registerDetailsRoute({ router, license }: RouteDependencies) { const statsResponse = await context.core.elasticsearch.client.asCurrentUser.nodes.stats(); const okResponse = { body: findMatchingNodes(statsResponse.body, nodeAttrs) }; return response.ok(okResponse); - } catch (e) { - if (e.name === 'ResponseError') { - return response.customError({ - statusCode: e.statusCode, - body: { message: e.body.error?.reason }, - }); - } - // Case: default - return response.internalError({ body: e }); + } catch (error) { + return handleEsError({ error, response }); } }) ); diff --git a/x-pack/plugins/index_lifecycle_management/server/routes/api/nodes/register_list_route.ts b/x-pack/plugins/index_lifecycle_management/server/routes/api/nodes/register_list_route.ts index bb1679e695e14c..53955d93c1e09a 100644 --- a/x-pack/plugins/index_lifecycle_management/server/routes/api/nodes/register_list_route.ts +++ b/x-pack/plugins/index_lifecycle_management/server/routes/api/nodes/register_list_route.ts @@ -62,7 +62,12 @@ export function convertSettingsIntoLists( ); } -export function registerListRoute({ router, config, license }: RouteDependencies) { +export function registerListRoute({ + router, + config, + license, + lib: { handleEsError }, +}: RouteDependencies) { const { filteredNodeAttributes } = config; const NODE_ATTRS_KEYS_TO_IGNORE: string[] = [ @@ -95,15 +100,8 @@ export function registerListRoute({ router, config, license }: RouteDependencies disallowedNodeAttributes ); return response.ok({ body }); - } catch (e) { - if (e.name === 'ResponseError') { - return response.customError({ - statusCode: e.statusCode, - body: { message: e.body.error?.reason }, - }); - } - // Case: default - return response.internalError({ body: e }); + } catch (error) { + return handleEsError({ error, response }); } }) ); diff --git a/x-pack/plugins/index_lifecycle_management/server/routes/api/policies/register_create_route.ts b/x-pack/plugins/index_lifecycle_management/server/routes/api/policies/register_create_route.ts index 359b275622f0c8..d8e40e3b30410c 100644 --- a/x-pack/plugins/index_lifecycle_management/server/routes/api/policies/register_create_route.ts +++ b/x-pack/plugins/index_lifecycle_management/server/routes/api/policies/register_create_route.ts @@ -134,7 +134,11 @@ const bodySchema = schema.object({ }), }); -export function registerCreateRoute({ router, license }: RouteDependencies) { +export function registerCreateRoute({ + router, + license, + lib: { handleEsError }, +}: RouteDependencies) { router.post( { path: addBasePath('/policies'), validate: { body: bodySchema } }, license.guardApiRoute(async (context, request, response) => { @@ -144,15 +148,8 @@ export function registerCreateRoute({ router, license }: RouteDependencies) { try { await createPolicy(context.core.elasticsearch.client.asCurrentUser, name, phases); return response.ok(); - } catch (e) { - if (e.name === 'ResponseError') { - return response.customError({ - statusCode: e.statusCode, - body: { message: e.body.error?.reason }, - }); - } - // Case: default - return response.internalError({ body: e }); + } catch (error) { + return handleEsError({ error, response }); } }) ); diff --git a/x-pack/plugins/index_lifecycle_management/server/routes/api/policies/register_delete_route.ts b/x-pack/plugins/index_lifecycle_management/server/routes/api/policies/register_delete_route.ts index cb394c12c46fa1..b0363cb7c3bc6b 100644 --- a/x-pack/plugins/index_lifecycle_management/server/routes/api/policies/register_delete_route.ts +++ b/x-pack/plugins/index_lifecycle_management/server/routes/api/policies/register_delete_route.ts @@ -23,7 +23,11 @@ const paramsSchema = schema.object({ policyNames: schema.string(), }); -export function registerDeleteRoute({ router, license }: RouteDependencies) { +export function registerDeleteRoute({ + router, + license, + lib: { handleEsError }, +}: RouteDependencies) { router.delete( { path: addBasePath('/policies/{policyNames}'), validate: { params: paramsSchema } }, license.guardApiRoute(async (context, request, response) => { @@ -33,15 +37,8 @@ export function registerDeleteRoute({ router, license }: RouteDependencies) { try { await deletePolicies(context.core.elasticsearch.client.asCurrentUser, policyNames); return response.ok(); - } catch (e) { - if (e.name === 'ResponseError') { - return response.customError({ - statusCode: e.statusCode, - body: { message: e.body.error?.reason }, - }); - } - // Case: default - return response.internalError({ body: e }); + } catch (error) { + return handleEsError({ error, response }); } }) ); diff --git a/x-pack/plugins/index_lifecycle_management/server/routes/api/policies/register_fetch_route.ts b/x-pack/plugins/index_lifecycle_management/server/routes/api/policies/register_fetch_route.ts index 8cbea256663784..fc5f369e588f1c 100644 --- a/x-pack/plugins/index_lifecycle_management/server/routes/api/policies/register_fetch_route.ts +++ b/x-pack/plugins/index_lifecycle_management/server/routes/api/policies/register_fetch_route.ts @@ -57,7 +57,7 @@ const querySchema = schema.object({ withIndices: schema.boolean({ defaultValue: false }), }); -export function registerFetchRoute({ router, license }: RouteDependencies) { +export function registerFetchRoute({ router, license, lib: { handleEsError } }: RouteDependencies) { router.get( { path: addBasePath('/policies'), validate: { query: querySchema } }, license.guardApiRoute(async (context, request, response) => { @@ -75,15 +75,8 @@ export function registerFetchRoute({ router, license }: RouteDependencies) { await addLinkedIndices(asCurrentUser, policiesMap); } return response.ok({ body: formatPolicies(policiesMap) }); - } catch (e) { - if (e.name === 'ResponseError') { - return response.customError({ - statusCode: e.statusCode, - body: { message: e.body.error?.reason }, - }); - } - // Case: default - return response.internalError({ body: e }); + } catch (error) { + return handleEsError({ error, response }); } }) ); diff --git a/x-pack/plugins/index_lifecycle_management/server/routes/api/snapshot_policies/register_fetch_route.ts b/x-pack/plugins/index_lifecycle_management/server/routes/api/snapshot_policies/register_fetch_route.ts index 869be3d557040b..00afc31c03badf 100644 --- a/x-pack/plugins/index_lifecycle_management/server/routes/api/snapshot_policies/register_fetch_route.ts +++ b/x-pack/plugins/index_lifecycle_management/server/routes/api/snapshot_policies/register_fetch_route.ts @@ -14,7 +14,7 @@ async function fetchSnapshotPolicies(client: ElasticsearchClient): Promise return response.body; } -export function registerFetchRoute({ router, license }: RouteDependencies) { +export function registerFetchRoute({ router, license, lib: { handleEsError } }: RouteDependencies) { router.get( { path: addBasePath('/snapshot_policies'), validate: false }, license.guardApiRoute(async (context, request, response) => { @@ -23,15 +23,8 @@ export function registerFetchRoute({ router, license }: RouteDependencies) { context.core.elasticsearch.client.asCurrentUser ); return response.ok({ body: Object.keys(policiesByName) }); - } catch (e) { - if (e.name === 'ResponseError') { - return response.customError({ - statusCode: e.statusCode, - body: { message: e.body.error?.reason }, - }); - } - // Case: default - return response.internalError({ body: e }); + } catch (error) { + return handleEsError({ error, response }); } }) ); diff --git a/x-pack/plugins/index_lifecycle_management/server/routes/api/templates/register_add_policy_route.ts b/x-pack/plugins/index_lifecycle_management/server/routes/api/templates/register_add_policy_route.ts index 7e7f3f1f725f83..667491ef4af65a 100644 --- a/x-pack/plugins/index_lifecycle_management/server/routes/api/templates/register_add_policy_route.ts +++ b/x-pack/plugins/index_lifecycle_management/server/routes/api/templates/register_add_policy_route.ts @@ -92,7 +92,11 @@ const querySchema = schema.object({ legacy: schema.maybe(schema.oneOf([schema.literal('true'), schema.literal('false')])), }); -export function registerAddPolicyRoute({ router, license }: RouteDependencies) { +export function registerAddPolicyRoute({ + router, + license, + lib: { handleEsError }, +}: RouteDependencies) { router.post( { path: addBasePath('/template'), validate: { body: bodySchema, query: querySchema } }, license.guardApiRoute(async (context, request, response) => { @@ -118,15 +122,8 @@ export function registerAddPolicyRoute({ router, license }: RouteDependencies) { }); } return response.ok(); - } catch (e) { - if (e.name === 'ResponseError') { - return response.customError({ - statusCode: e.statusCode, - body: { message: e.body.error?.reason }, - }); - } - // Case: default - return response.internalError({ body: e }); + } catch (error) { + return handleEsError({ error, response }); } }) ); diff --git a/x-pack/plugins/index_lifecycle_management/server/routes/api/templates/register_fetch_route.ts b/x-pack/plugins/index_lifecycle_management/server/routes/api/templates/register_fetch_route.ts index fbd102d3be1eb0..35860d21773294 100644 --- a/x-pack/plugins/index_lifecycle_management/server/routes/api/templates/register_fetch_route.ts +++ b/x-pack/plugins/index_lifecycle_management/server/routes/api/templates/register_fetch_route.ts @@ -80,7 +80,7 @@ const querySchema = schema.object({ legacy: schema.maybe(schema.oneOf([schema.literal('true'), schema.literal('false')])), }); -export function registerFetchRoute({ router, license }: RouteDependencies) { +export function registerFetchRoute({ router, license, lib: { handleEsError } }: RouteDependencies) { router.get( { path: addBasePath('/templates'), validate: { query: querySchema } }, license.guardApiRoute(async (context, request, response) => { @@ -92,15 +92,8 @@ export function registerFetchRoute({ router, license }: RouteDependencies) { ); const okResponse = { body: filterTemplates(templates, isLegacy) }; return response.ok(okResponse); - } catch (e) { - if (e.name === 'ResponseError') { - return response.customError({ - statusCode: e.statusCode, - body: { message: e.body.error?.reason }, - }); - } - // Case: default - return response.internalError({ body: e }); + } catch (error) { + return handleEsError({ error, response }); } }) ); diff --git a/x-pack/plugins/index_lifecycle_management/server/shared_imports.ts b/x-pack/plugins/index_lifecycle_management/server/shared_imports.ts new file mode 100644 index 00000000000000..068cddcee4c86a --- /dev/null +++ b/x-pack/plugins/index_lifecycle_management/server/shared_imports.ts @@ -0,0 +1,7 @@ +/* + * 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. + */ + +export { handleEsError } from '../../../../src/plugins/es_ui_shared/server'; diff --git a/x-pack/plugins/index_lifecycle_management/server/types.ts b/x-pack/plugins/index_lifecycle_management/server/types.ts index e34dc8e4b1a526..8de7a01f1febc6 100644 --- a/x-pack/plugins/index_lifecycle_management/server/types.ts +++ b/x-pack/plugins/index_lifecycle_management/server/types.ts @@ -11,6 +11,7 @@ import { LicensingPluginSetup } from '../../licensing/server'; import { IndexManagementPluginSetup } from '../../index_management/server'; import { License } from './services'; import { IndexLifecycleManagementConfig } from './config'; +import { handleEsError } from './shared_imports'; export interface Dependencies { licensing: LicensingPluginSetup; @@ -22,4 +23,7 @@ export interface RouteDependencies { router: IRouter; config: IndexLifecycleManagementConfig; license: License; + lib: { + handleEsError: typeof handleEsError; + }; }