diff --git a/x-pack/plugins/ingest_manager/server/errors.test.ts b/x-pack/plugins/ingest_manager/server/errors/handlers.test.ts similarity index 73% rename from x-pack/plugins/ingest_manager/server/errors.test.ts rename to x-pack/plugins/ingest_manager/server/errors/handlers.test.ts index 70e3a3b4150ade..361386a86d5478 100644 --- a/x-pack/plugins/ingest_manager/server/errors.test.ts +++ b/x-pack/plugins/ingest_manager/server/errors/handlers.test.ts @@ -5,16 +5,19 @@ */ import Boom from 'boom'; +import { errors } from 'elasticsearch'; import { httpServerMock } from 'src/core/server/mocks'; -import { createAppContextStartContractMock } from './mocks'; - +import { createAppContextStartContractMock } from '../mocks'; +import { appContextService } from '../services'; import { IngestManagerError, RegistryError, PackageNotFoundError, defaultIngestErrorHandler, -} from './errors'; -import { appContextService } from './services'; +} from './index'; + +const LegacyESErrors = errors as Record; +type ITestEsErrorsFnParams = [errorCode: string, error: any, expectedMessage: string]; describe('defaultIngestErrorHandler', () => { let mockContract: ReturnType; @@ -29,6 +32,55 @@ describe('defaultIngestErrorHandler', () => { appContextService.stop(); }); + async function testEsErrorsFn(...args: ITestEsErrorsFnParams) { + const [, error, expectedMessage] = args; + jest.clearAllMocks(); + const response = httpServerMock.createResponseFactory(); + await defaultIngestErrorHandler({ error, response }); + + // response + expect(response.ok).toHaveBeenCalledTimes(0); + expect(response.customError).toHaveBeenCalledTimes(1); + expect(response.customError).toHaveBeenCalledWith({ + statusCode: error.status, + body: { message: expectedMessage }, + }); + + // logging + expect(mockContract.logger?.error).toHaveBeenCalledTimes(1); + expect(mockContract.logger?.error).toHaveBeenCalledWith(expectedMessage); + } + + describe('use the HTTP error status code provided by LegacyESErrors', () => { + const statusCodes = Object.keys(LegacyESErrors).filter((key) => /^\d+$/.test(key)); + const errorCodes = statusCodes.filter((key) => parseInt(key, 10) >= 400); + const casesWithPathResponse: ITestEsErrorsFnParams[] = errorCodes.map((errorCode) => [ + errorCode, + new LegacyESErrors[errorCode]('the root message', { + path: '/path/to/call', + response: 'response is here', + }), + 'the root message response from /path/to/call: response is here', + ]); + const casesWithOtherMeta: ITestEsErrorsFnParams[] = errorCodes.map((errorCode) => [ + errorCode, + new LegacyESErrors[errorCode]('the root message', { + other: '/path/to/call', + props: 'response is here', + }), + 'the root message', + ]); + const casesWithoutMeta: ITestEsErrorsFnParams[] = errorCodes.map((errorCode) => [ + errorCode, + new LegacyESErrors[errorCode]('some message'), + 'some message', + ]); + + test.each(casesWithPathResponse)('%d - with path & response', testEsErrorsFn); + test.each(casesWithOtherMeta)('%d - with other metadata', testEsErrorsFn); + test.each(casesWithoutMeta)('%d - without metadata', testEsErrorsFn); + }); + describe('IngestManagerError', () => { it('502: RegistryError', async () => { const error = new RegistryError('xyz'); diff --git a/x-pack/plugins/ingest_manager/server/errors.ts b/x-pack/plugins/ingest_manager/server/errors/handlers.ts similarity index 60% rename from x-pack/plugins/ingest_manager/server/errors.ts rename to x-pack/plugins/ingest_manager/server/errors/handlers.ts index 9829a4de23d7be..9f776565cf2626 100644 --- a/x-pack/plugins/ingest_manager/server/errors.ts +++ b/x-pack/plugins/ingest_manager/server/errors/handlers.ts @@ -4,7 +4,6 @@ * you may not use this file except in compliance with the Elastic License. */ -/* eslint-disable max-classes-per-file */ import Boom, { isBoom } from 'boom'; import { RequestHandlerContext, @@ -12,25 +11,39 @@ import { IKibanaResponse, KibanaResponseFactory, } from 'src/core/server'; -import { appContextService } from './services'; +import { errors as LegacyESErrors } from 'elasticsearch'; +import { appContextService } from '../services'; +import { IngestManagerError, RegistryError, PackageNotFoundError } from './index'; type IngestErrorHandler = ( params: IngestErrorHandlerParams ) => IKibanaResponse | Promise; - interface IngestErrorHandlerParams { error: IngestManagerError | Boom | Error; response: KibanaResponseFactory; request?: KibanaRequest; context?: RequestHandlerContext; } +// unsure if this is correct. would prefer to use something "official" +// this type is based on BadRequest values observed while debugging https://github.com/elastic/kibana/issues/75862 -export class IngestManagerError extends Error { - constructor(message?: string) { - super(message); - this.name = this.constructor.name; // for stack traces - } +interface LegacyESClientError { + message: string; + stack: string; + status: number; + displayName: string; + path?: string; + query?: string | undefined; + body?: { + error: object; + status: number; + }; + statusCode?: number; + response?: string; } +export const isLegacyESClientError = (error: any): error is LegacyESClientError => { + return error instanceof LegacyESErrors._Abstract; +}; const getHTTPResponseCode = (error: IngestManagerError): number => { if (error instanceof RegistryError) { @@ -48,6 +61,22 @@ export const defaultIngestErrorHandler: IngestErrorHandler = async ({ response, }: IngestErrorHandlerParams): Promise => { const logger = appContextService.getLogger(); + if (isLegacyESClientError(error)) { + // there was a problem communicating with ES (e.g. via `callCluster`) + // only log the message + const message = + error?.path && error?.response + ? // if possible, return the failing endpoint and its response + `${error.message} response from ${error.path}: ${error.response}` + : error.message; + + logger.error(message); + + return response.customError({ + statusCode: error?.statusCode || error.status, + body: { message }, + }); + } // our "expected" errors if (error instanceof IngestManagerError) { @@ -76,9 +105,3 @@ export const defaultIngestErrorHandler: IngestErrorHandler = async ({ body: { message: error.message }, }); }; - -export class RegistryError extends IngestManagerError {} -export class RegistryConnectionError extends RegistryError {} -export class RegistryResponseError extends RegistryError {} -export class PackageNotFoundError extends IngestManagerError {} -export class PackageOutdatedError extends IngestManagerError {} diff --git a/x-pack/plugins/ingest_manager/server/errors/index.ts b/x-pack/plugins/ingest_manager/server/errors/index.ts new file mode 100644 index 00000000000000..5e36a2ec9a884a --- /dev/null +++ b/x-pack/plugins/ingest_manager/server/errors/index.ts @@ -0,0 +1,20 @@ +/* + * 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. + */ + +/* eslint-disable max-classes-per-file */ +export { defaultIngestErrorHandler } from './handlers'; + +export class IngestManagerError extends Error { + constructor(message?: string) { + super(message); + this.name = this.constructor.name; // for stack traces + } +} +export class RegistryError extends IngestManagerError {} +export class RegistryConnectionError extends RegistryError {} +export class RegistryResponseError extends RegistryError {} +export class PackageNotFoundError extends IngestManagerError {} +export class PackageOutdatedError extends IngestManagerError {} diff --git a/x-pack/plugins/ingest_manager/server/services/epm/elasticsearch/ingest_pipeline/install.ts b/x-pack/plugins/ingest_manager/server/services/epm/elasticsearch/ingest_pipeline/install.ts index 44e4eddfbbe6a7..878c6ea8f28047 100644 --- a/x-pack/plugins/ingest_manager/server/services/epm/elasticsearch/ingest_pipeline/install.ts +++ b/x-pack/plugins/ingest_manager/server/services/epm/elasticsearch/ingest_pipeline/install.ts @@ -156,7 +156,12 @@ async function installPipeline({ body: pipeline.contentForInstallation, }; if (pipeline.extension === 'yml') { - callClusterParams.headers = { ['Content-Type']: 'application/yaml' }; + callClusterParams.headers = { + // pipeline is YAML + 'Content-Type': 'application/yaml', + // but we want JSON responses (to extract error messages, status code, or other metadata) + Accept: 'application/json', + }; } // This uses the catch-all endpoint 'transport.request' because we have to explicitly