Skip to content

Commit

Permalink
Fix hapi upgrade (#25723)
Browse files Browse the repository at this point in the history
* Fixes consistent error handling in routes, adds tests for error handling

* Upgrades to failure tests per review
  • Loading branch information
jasonrhodes authored Nov 15, 2018
1 parent c21cf21 commit f926df8
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 3 deletions.
78 changes: 78 additions & 0 deletions x-pack/plugins/apm/server/routes/__test__/routeFailures.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
* 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.
*/

import { Server } from 'hapi';
// @ts-ignore
import { initErrorsApi } from '../errors';
import { initServicesApi } from '../services';
// @ts-ignore
import { initStatusApi } from '../status_check';
import { initTracesApi } from '../traces';
import { initTransactionsApi } from '../transactions';

describe('route handlers fail properly', () => {
let consoleErrorSpy: any;

async function testRouteFailures(init: (server: Server) => void) {
const mockServer = { route: jest.fn() };
init((mockServer as unknown) as Server);
expect(mockServer.route).toHaveBeenCalled();

const routes = mockServer.route.mock.calls;
const mockReq = {
params: {},
query: {},
pre: {
setup: {
config: { get: jest.fn() },
client: jest.fn(() => Promise.reject(new Error('request failed')))
}
}
};

routes.forEach(async (route, i) => {
test(`route ${i + 1} of ${
routes.length
} should fail with a Boom error`, async () => {
await expect(route[0].handler(mockReq)).rejects.toMatchObject({
message: 'request failed',
isBoom: true
});
expect(consoleErrorSpy).toHaveBeenCalledTimes(1);
});
});
}

beforeEach(() => {
consoleErrorSpy = jest
.spyOn(global.console, 'error')
.mockImplementation(undefined);
});

afterEach(() => {
consoleErrorSpy.mockRestore();
});

describe('error routes', async () => {
await testRouteFailures(initErrorsApi);
});

describe('service routes', async () => {
await testRouteFailures(initServicesApi);
});

describe('status check routes', async () => {
await testRouteFailures(initStatusApi);
});

describe('trace routes', async () => {
await testRouteFailures(initTracesApi);
});

describe('transaction routes', async () => {
await testRouteFailures(initTransactionsApi);
});
});
2 changes: 1 addition & 1 deletion x-pack/plugins/apm/server/routes/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const defaultErrorHandler = (err: Error) => {
// tslint:disable-next-line
console.error(err.stack);
// @ts-ignore
return Boom.boomify(err, { statusCode: 400 });
throw Boom.boomify(err, { statusCode: 400 });
};

export function initServicesApi(server: Server) {
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/apm/server/routes/traces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const defaultErrorHandler = (err: Error) => {
// tslint:disable-next-line
console.error(err.stack);
// @ts-ignore
return Boom.boomify(err, { statusCode: 400 });
throw Boom.boomify(err, { statusCode: 400 });
};

export function initTracesApi(server: Server) {
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/apm/server/routes/transactions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const defaultErrorHandler = (err: Error) => {
// tslint:disable-next-line
console.error(err.stack);
// @ts-ignore
Boom.boomify(err, { statusCode: err.statusCode || 400 });
throw Boom.boomify(err, { statusCode: 400 });
};

export function initTransactionsApi(server: Server) {
Expand Down

0 comments on commit f926df8

Please sign in to comment.