Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Ingest Manager] Handle Legacy ES client errors #76865

Merged
merged 9 commits into from
Sep 10, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, any>;
type ITestEsErrorsFnParams = [errorCode: string, error: any, expectedMessage: string];

describe('defaultIngestErrorHandler', () => {
let mockContract: ReturnType<typeof createAppContextStartContractMock>;
Expand All @@ -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');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,33 +4,46 @@
* 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,
KibanaRequest,
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<IKibanaResponse>;

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) {
Expand All @@ -48,6 +61,22 @@ export const defaultIngestErrorHandler: IngestErrorHandler = async ({
response,
}: IngestErrorHandlerParams): Promise<IKibanaResponse> => {
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) {
Expand Down Expand Up @@ -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 {}
20 changes: 20 additions & 0 deletions x-pack/plugins/ingest_manager/server/errors/index.ts
Original file line number Diff line number Diff line change
@@ -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 {}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down