diff --git a/packages/agent/src/routes/system/error-handling.ts b/packages/agent/src/routes/system/error-handling.ts index 7f074467f9..89a0507a1b 100644 --- a/packages/agent/src/routes/system/error-handling.ts +++ b/packages/agent/src/routes/system/error-handling.ts @@ -42,6 +42,9 @@ export default class ErrorHandling extends BaseRoute { if (!this.options.isProduction) { process.nextTick(() => this.debugLogError(context, e)); } + + // The error will be used by Logger + throw e; } } diff --git a/packages/agent/src/routes/system/logger.ts b/packages/agent/src/routes/system/logger.ts index 742c6eea6e..58c57b0a10 100644 --- a/packages/agent/src/routes/system/logger.ts +++ b/packages/agent/src/routes/system/logger.ts @@ -20,7 +20,6 @@ export default class Logger extends BaseRoute { await next(); } catch (e) { error = e; - throw e; } finally { let logLevel: LoggerLevel = 'Info'; if (context.response.status >= HttpCode.BadRequest) logLevel = 'Warn'; diff --git a/packages/agent/src/types.ts b/packages/agent/src/types.ts index 49520d7e83..1298d1b5dd 100644 --- a/packages/agent/src/types.ts +++ b/packages/agent/src/types.ts @@ -47,8 +47,8 @@ export enum HttpCode { export enum RouteType { // Changing the values of this enum changes the order in which routes are loaded into koa-router. - ErrorHandler = 0, - LoggerHandler = 1, + LoggerHandler = 0, + ErrorHandler = 1, PublicRoute = 2, Authentication = 3, PrivateRoute = 4, diff --git a/packages/agent/test/routes/index.test.ts b/packages/agent/test/routes/index.test.ts index ab00d746c9..99ab4d7b8f 100644 --- a/packages/agent/test/routes/index.test.ts +++ b/packages/agent/test/routes/index.test.ts @@ -27,6 +27,7 @@ import ScopeInvalidation from '../../src/routes/security/scope-invalidation'; import ErrorHandling from '../../src/routes/system/error-handling'; import HealthCheck from '../../src/routes/system/healthcheck'; import Logger from '../../src/routes/system/logger'; +import { RouteType } from '../../src/types'; import * as factories from '../__factories__'; describe('Route index', () => { @@ -81,6 +82,22 @@ describe('Route index', () => { const countCollectionRoutes = COLLECTION_ROUTES_CTOR.length * dataSource.collections.length; expect(routes.length).toEqual(ROOT_ROUTES_CTOR.length + countCollectionRoutes); }); + + test('should order routes correctly', async () => { + const dataSource = setupWithoutRelation(); + + const routes = makeRoutes( + dataSource, + factories.forestAdminHttpDriverOptions.build(), + factories.forestAdminHttpDriverServices.build(), + ); + + // with LoggerHandler in first + expect(routes[0].type).toEqual(RouteType.LoggerHandler); + + // followed by ErrorHandler + expect(routes[1].type).toEqual(RouteType.ErrorHandler); + }); }); describe('when a data source with an one to many relation', () => { diff --git a/packages/agent/test/routes/system/error-handling.test.ts b/packages/agent/test/routes/system/error-handling.test.ts index 3e0697d127..fa1c19bce8 100644 --- a/packages/agent/test/routes/system/error-handling.test.ts +++ b/packages/agent/test/routes/system/error-handling.test.ts @@ -71,7 +71,7 @@ describe('ErrorHandling', () => { const context = createMockContext(); const next = jest.fn().mockRejectedValue(new ValidationError('hello')); - await handleError.call(route, context, next); + await expect(handleError.call(route, context, next)).rejects.toThrow(); expect(context.response.status).toStrictEqual(HttpCode.BadRequest); expect(context.response.body).toStrictEqual({ @@ -84,7 +84,7 @@ describe('ErrorHandling', () => { const context = createMockContext(); const next = jest.fn().mockRejectedValue(new BadRequestError('message')); - await handleError.call(route, context, next); + await expect(handleError.call(route, context, next)).rejects.toThrow(); expect(context.response.status).toStrictEqual(HttpCode.BadRequest); expect(context.response.body).toStrictEqual({ @@ -97,7 +97,7 @@ describe('ErrorHandling', () => { const context = createMockContext(); const next = jest.fn().mockRejectedValue(new UnprocessableError('message')); - await handleError.call(route, context, next); + await expect(handleError.call(route, context, next)).rejects.toThrow(); expect(context.response.status).toStrictEqual(HttpCode.Unprocessable); expect(context.response.body).toStrictEqual({ @@ -110,7 +110,7 @@ describe('ErrorHandling', () => { const context = createMockContext(); const next = jest.fn().mockRejectedValue(new ForbiddenError('forbidden')); - await handleError.call(route, context, next); + await expect(handleError.call(route, context, next)).rejects.toThrow(); expect(context.response.status).toStrictEqual(HttpCode.Forbidden); expect(context.response.body).toStrictEqual({ @@ -123,7 +123,7 @@ describe('ErrorHandling', () => { const context = createMockContext(); const next = jest.fn().mockRejectedValue(new NotFoundError('message')); - await handleError.call(route, context, next); + await expect(handleError.call(route, context, next)).rejects.toThrow(); expect(context.response.status).toStrictEqual(HttpCode.NotFound); expect(context.response.body).toStrictEqual({ @@ -138,7 +138,7 @@ describe('ErrorHandling', () => { .fn() .mockRejectedValue(new Error('Internal error with important data that should not leak')); - await handleError.call(route, context, next); + await expect(handleError.call(route, context, next)).rejects.toThrow(); expect(context.response.status).toStrictEqual(HttpCode.InternalServerError); expect(context.response.body).toStrictEqual({ @@ -151,7 +151,7 @@ describe('ErrorHandling', () => { const context = createMockContext(); const next = jest.fn().mockRejectedValue(new Error('My Error')); - await handleError.call(route, context, next); + await expect(handleError.call(route, context, next)).rejects.toThrow(); expect(context.response.status).toStrictEqual(HttpCode.InternalServerError); expect(context.response.body).toStrictEqual({ @@ -165,7 +165,7 @@ describe('ErrorHandling', () => { const context = createMockContext(); const next = jest.fn().mockRejectedValue(new FakePayloadError('message')); - await handleError.call(route, context, next); + await expect(handleError.call(route, context, next)).rejects.toThrow(); expect(context.response.status).toStrictEqual(HttpCode.Forbidden); expect(context.response.body).toStrictEqual({ @@ -196,7 +196,7 @@ describe('ErrorHandling', () => { const next = jest.fn().mockRejectedValue(fakeErrorFromDependency); - await handleError.call(route, context, next); + await expect(handleError.call(route, context, next)).rejects.toThrow(); expect(context.response.status).toStrictEqual(HttpCode.InternalServerError); expect(context.response.body).toStrictEqual({ @@ -233,7 +233,8 @@ describe('ErrorHandling', () => { const context = createMockContext({ method: 'POST' }); const next = jest.fn().mockRejectedValue(new Error('Internal error')); - await handleError.call(route, context, next); + await expect(handleError.call(route, context, next)).rejects.toThrow(); + await new Promise(setImmediate); expect(console.error).toHaveBeenCalled(); diff --git a/packages/agent/test/routes/system/logger.test.ts b/packages/agent/test/routes/system/logger.test.ts index 6e370a7352..4da502f59a 100644 --- a/packages/agent/test/routes/system/logger.test.ts +++ b/packages/agent/test/routes/system/logger.test.ts @@ -71,7 +71,8 @@ describe('Logger', () => { }; const next = jest.fn().mockRejectedValue(error); - await expect(handleLog.call(route, context, next)).rejects.toThrow('RouteError'); + await expect(handleLog.call(route, context, next)).toResolve(); + expect(options.logger).toHaveBeenCalledWith('Error', '[500] GET someUrl - 0ms', error); }); });