diff --git a/x-pack/plugins/ingest_manager/server/errors.ts b/x-pack/plugins/ingest_manager/server/errors.ts index 024475d678f5c9b..1e2ad8d850a82be 100644 --- a/x-pack/plugins/ingest_manager/server/errors.ts +++ b/x-pack/plugins/ingest_manager/server/errors.ts @@ -5,7 +5,7 @@ */ /* eslint-disable max-classes-per-file */ -import Boom from 'boom'; +import Boom, { isBoom } from 'boom'; import { RequestHandlerContext, KibanaRequest, @@ -22,32 +22,34 @@ export class IngestManagerError extends Error { } export const getHTTPResponseCode = (error: IngestManagerError): number => { - if (error instanceof RegistryError) { - return 502; // Bad Gateway - } - if (error instanceof PackageNotFoundError) { - return 404; - } - if (error instanceof PackageOutdatedError) { - return 400; - } else { - return 400; // Bad Request + switch (true) { + case error instanceof RegistryConnectionError: + return 502; // Bad Gateway + + case error instanceof RegistryResponseNotFoundError: + case error instanceof PackageNotFoundError: + return 404; + + case error instanceof PackageOutdatedError: + return 400; + + default: + return 500; } }; -interface IngestErrorHandlerParams { +export type IngestErrorHandler = ( + params: IngestErrorHandlerParams +) => IKibanaResponse | Promise; + +export interface IngestErrorHandlerParams { error: IngestManagerError | Boom | Error; response: KibanaResponseFactory; request?: KibanaRequest; context?: RequestHandlerContext; } -export const defaultIngestErrorHandler = async ({ - error, - request, - response, - context, -}: IngestErrorHandlerParams): Promise => { +export const defaultIngestErrorHandler: IngestErrorHandler = async ({ error, response }) => { const logger = appContextService.getLogger(); if (error instanceof IngestManagerError) { logger.error(error.message); @@ -56,7 +58,7 @@ export const defaultIngestErrorHandler = async ({ body: { message: error.message }, }); } - if ('isBoom' in error) { + if (isBoom(error)) { logger.error(error.output.payload.message); return response.customError({ statusCode: error.output.statusCode, @@ -70,6 +72,9 @@ export const defaultIngestErrorHandler = async ({ }); }; -export class RegistryError extends IngestManagerError {} +export class RegistryResponseError extends IngestManagerError {} +export class RegistryResponseNotFoundError extends IngestManagerError {} +export class RegistryConnectionError extends IngestManagerError {} + export class PackageNotFoundError extends IngestManagerError {} export class PackageOutdatedError extends IngestManagerError {} diff --git a/x-pack/plugins/ingest_manager/server/routes/setup/handlers.test.ts b/x-pack/plugins/ingest_manager/server/routes/setup/handlers.test.ts index ce826e78c454daf..d8d0828be50a454 100644 --- a/x-pack/plugins/ingest_manager/server/routes/setup/handlers.test.ts +++ b/x-pack/plugins/ingest_manager/server/routes/setup/handlers.test.ts @@ -7,7 +7,7 @@ import { xpackMocks } from '../../../../../../x-pack/mocks'; import { httpServerMock } from 'src/core/server/mocks'; import { PostIngestSetupResponse } from '../../../common'; -import { RegistryError } from '../../errors'; +import { RegistryConnectionError } from '../../errors'; import { createAppContextStartContractMock } from '../../mocks'; import { ingestManagerSetupHandler } from './handlers'; import { appContextService } from '../../services/app_context'; @@ -66,9 +66,9 @@ describe('ingestManagerSetupHandler', () => { }); }); - it('POST /setup fails w/502 on RegistryError', async () => { + it('POST /setup fails w/502 on RegistryConnectionError', async () => { mockSetupIngestManager.mockImplementation(() => - Promise.reject(new RegistryError('Registry method mocked to throw')) + Promise.reject(new RegistryConnectionError('Registry method mocked to throw')) ); await ingestManagerSetupHandler(context, request, response); diff --git a/x-pack/plugins/ingest_manager/server/services/epm/registry/requests.test.ts b/x-pack/plugins/ingest_manager/server/services/epm/registry/requests.test.ts index f836a133a78a01b..728046ef5fdaea6 100644 --- a/x-pack/plugins/ingest_manager/server/services/epm/registry/requests.test.ts +++ b/x-pack/plugins/ingest_manager/server/services/epm/registry/requests.test.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ import { fetchUrl } from './requests'; -import { RegistryError } from '../../../errors'; +import { RegistryConnectionError } from '../../../errors'; jest.mock('node-fetch'); const { Response, FetchError } = jest.requireActual('node-fetch'); @@ -20,22 +20,22 @@ describe('setupIngestManager', () => { }); describe('fetchUrl / getResponse errors', () => { - it('regular Errors do not retry. Becomes RegistryError', async () => { + it('regular Errors do not retry. Becomes RegistryConnectionError', async () => { fetchMock.mockImplementationOnce(() => { throw new Error('mocked'); }); const promise = fetchUrl(''); - await expect(promise).rejects.toThrow(RegistryError); + await expect(promise).rejects.toThrow(RegistryConnectionError); expect(fetchMock).toHaveBeenCalledTimes(1); }); - it('TypeErrors do not retry. Becomes RegistryError', async () => { + it('TypeErrors do not retry. Becomes RegistryConnectionError', async () => { fetchMock.mockImplementationOnce(() => { // @ts-expect-error null.f(); }); const promise = fetchUrl(''); - await expect(promise).rejects.toThrow(RegistryError); + await expect(promise).rejects.toThrow(RegistryConnectionError); expect(fetchMock).toHaveBeenCalledTimes(1); }); @@ -69,7 +69,7 @@ describe('setupIngestManager', () => { expect(actualResultsOrder).toEqual(['throw', 'throw', 'throw', 'return']); }); - it('or error after 1 failure & 5 retries with RegistryError', async () => { + it('or error after 1 failure & 5 retries with RegistryConnectionError', async () => { fetchMock .mockImplementationOnce(() => { throw new FetchError('message 1', 'system', { code: 'ESOMETHING' }); @@ -97,7 +97,7 @@ describe('setupIngestManager', () => { }); const promise = fetchUrl(''); - await expect(promise).rejects.toThrow(RegistryError); + await expect(promise).rejects.toThrow(RegistryConnectionError); // doesn't retry after 1 failure & 5 failed retries expect(fetchMock).toHaveBeenCalledTimes(6); const actualResultsOrder = fetchMock.mock.results.map(({ type }: { type: string }) => type); diff --git a/x-pack/plugins/ingest_manager/server/services/epm/registry/requests.ts b/x-pack/plugins/ingest_manager/server/services/epm/registry/requests.ts index 5939dc204aae66c..83d0a64260afb13 100644 --- a/x-pack/plugins/ingest_manager/server/services/epm/registry/requests.ts +++ b/x-pack/plugins/ingest_manager/server/services/epm/registry/requests.ts @@ -7,7 +7,11 @@ import fetch, { FetchError, Response } from 'node-fetch'; import pRetry from 'p-retry'; import { streamToString } from './streams'; -import { RegistryError } from '../../../errors'; +import { + RegistryConnectionError, + RegistryResponseError, + RegistryResponseNotFoundError, +} from '../../../errors'; type FailedAttemptErrors = pRetry.FailedAttemptError | FetchError | Error; @@ -19,10 +23,13 @@ async function registryFetch(url: string) { return response; } else { // 4xx & 5xx responses - // exit without retry & throw RegistryError - throw new pRetry.AbortError( - new RegistryError(`Error connecting to package registry at ${url}: ${response.statusText}`) - ); + const is404 = response.status === 404; + const ErrorCtor = is404 ? RegistryResponseNotFoundError : RegistryResponseError; + const message = `'${response.status} ${response.statusText}' response from package registry at ${url}`; + const error = new ErrorCtor(message); + + // exit without retry (will ultimately throw RegistryConnectionError) + throw new pRetry.AbortError(error); } } @@ -38,17 +45,16 @@ export async function getResponse(url: string): Promise { // and let the others through without retrying // // throwing in onFailedAttempt will abandon all retries & fail the request - // we only want to retry system errors, so throw a RegistryError for everything else + // we only want to retry system errors, so re-throw for everything else if (!isSystemError(error)) { - throw new RegistryError( - `Error connecting to package registry at ${url}: ${error.message}` - ); + throw error; } }, }); return response; - } catch (e) { - throw new RegistryError(`Error connecting to package registry at ${url}: ${e.message}`); + } catch (error) { + // throw same error type whether isSystemError (after max retry attempts) or something else + throw new RegistryConnectionError(error); } }