From db0feaf7b5dca9f1353bf3a61036d9492ce16846 Mon Sep 17 00:00:00 2001 From: svozza Date: Wed, 13 Aug 2025 03:35:17 +0100 Subject: [PATCH 1/2] feat(event-handler): add error handling functionality to BaseRouter --- packages/event-handler/src/rest/BaseRouter.ts | 91 ++++ packages/event-handler/src/rest/errors.ts | 4 +- .../tests/unit/rest/BaseRouter.test.ts | 397 +++++++++++++++++- 3 files changed, 468 insertions(+), 24 deletions(-) diff --git a/packages/event-handler/src/rest/BaseRouter.ts b/packages/event-handler/src/rest/BaseRouter.ts index 209bdba9e2..51ddd412d4 100644 --- a/packages/event-handler/src/rest/BaseRouter.ts +++ b/packages/event-handler/src/rest/BaseRouter.ts @@ -16,6 +16,11 @@ import type { } from '../types/rest.js'; import { HttpVerbs } from './constants.js'; import { ErrorHandlerRegistry } from './ErrorHandlerRegistry.js'; +import { + MethodNotAllowedError, + NotFoundError, + ServiceError, +} from './errors.js'; import { Route } from './Route.js'; import { RouteHandlerRegistry } from './RouteHandlerRegistry.js'; @@ -54,6 +59,12 @@ abstract class BaseRouter { this.isDev = isDevMode(); } + /** + * Registers a custom error handler for specific error types. + * + * @param errorType - The error constructor(s) to handle + * @param handler - The error handler function that returns an ErrorResponse + */ public errorHandler( errorType: ErrorConstructor | ErrorConstructor[], handler: ErrorHandler @@ -61,6 +72,24 @@ abstract class BaseRouter { this.errorHandlerRegistry.register(errorType, handler); } + /** + * Registers a custom handler for 404 Not Found errors. + * + * @param handler - The error handler function for NotFoundError + */ + public notFound(handler: ErrorHandler): void { + this.errorHandlerRegistry.register(NotFoundError, handler); + } + + /** + * Registers a custom handler for 405 Method Not Allowed errors. + * + * @param handler - The error handler function for MethodNotAllowedError + */ + public methodNotAllowed(handler: ErrorHandler): void { + this.errorHandlerRegistry.register(MethodNotAllowedError, handler); + } + public abstract resolve( event: unknown, context: Context, @@ -76,6 +105,68 @@ abstract class BaseRouter { } } + /** + * Handles errors by finding a registered error handler or falling + * back to a default handler. + * + * @param error - The error to handle + * @returns A Response object with appropriate status code and error details + */ + protected handleError(error: Error): Response { + const handler = this.errorHandlerRegistry.resolve(error); + if (handler != null) { + try { + const body = handler(error); + return new Response(JSON.stringify(body), { + status: body.statusCode, + headers: { 'Content-Type': 'application/json' }, + }); + } catch (handlerError) { + return this.#defaultErrorHandler(handlerError as Error); + } + } + + if (error instanceof ServiceError) { + return new Response(JSON.stringify(error.toJSON()), { + status: error.statusCode, + headers: { 'Content-Type': 'application/json' }, + }); + } + + return this.#defaultErrorHandler(error); + } + + /** + * Default error handler that returns a 500 Internal Server Error response. + * In development mode, includes stack trace and error details. + * + * @param error - The error to handle + * @returns A Response object with 500 status and error details + */ + #defaultErrorHandler(error: Error): Response { + const isProduction = + getStringFromEnv({ + key: 'NODE_ENV', + defaultValue: '', + }) === 'production'; + + return new Response( + JSON.stringify({ + statusCode: 500, + error: 'Internal Server Error', + message: isProduction ? 'Internal Server Error' : error.message, + ...(isDevMode() && { + stack: error.stack, + details: { errorName: error.name }, + }), + }), + { + status: 500, + headers: { 'Content-Type': 'application/json' }, + } + ); + } + #handleHttpMethod( method: HttpMethod, path: Path, diff --git a/packages/event-handler/src/rest/errors.ts b/packages/event-handler/src/rest/errors.ts index 426566d088..f03421bc9c 100644 --- a/packages/event-handler/src/rest/errors.ts +++ b/packages/event-handler/src/rest/errors.ts @@ -19,12 +19,12 @@ export class ParameterValidationError extends RouteMatchingError { } } -abstract class ServiceError extends Error { +export abstract class ServiceError extends Error { abstract readonly statusCode: HttpStatusCode; abstract readonly errorType: string; public readonly details?: Record; - constructor( + protected constructor( message?: string, options?: ErrorOptions, details?: Record diff --git a/packages/event-handler/tests/unit/rest/BaseRouter.test.ts b/packages/event-handler/tests/unit/rest/BaseRouter.test.ts index ed63c0249b..b1b3b7903d 100644 --- a/packages/event-handler/tests/unit/rest/BaseRouter.test.ts +++ b/packages/event-handler/tests/unit/rest/BaseRouter.test.ts @@ -3,7 +3,12 @@ import type { Context } from 'aws-lambda'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { BaseRouter } from '../../../src/rest/BaseRouter.js'; import { HttpErrorCodes, HttpVerbs } from '../../../src/rest/constants.js'; -import { BadRequestError } from '../../../src/rest/errors.js'; +import { + BadRequestError, + InternalServerError, + MethodNotAllowedError, + NotFoundError, +} from '../../../src/rest/errors.js'; import type { HttpMethod, Path, @@ -35,12 +40,17 @@ describe('Class: BaseRouter', () => { } } - public resolve(event: unknown, context: Context): Promise { + public async resolve(event: unknown, context: Context): Promise { this.#isEvent(event); const { method, path } = event; const route = this.routeRegistry.resolve(method, path); - if (route == null) throw new Error('404'); - return route.handler(event, context); + try { + if (route == null) + throw new NotFoundError(`Route ${method} ${path} not found`); + return route.handler(event, context); + } catch (error) { + return this.handleError(error as Error); + } } } @@ -215,27 +225,370 @@ describe('Class: BaseRouter', () => { }); }); - it('handles errors through registered error handlers', async () => { - // Prepare - class TestRouterWithErrorAccess extends TestResolver { - get testErrorHandlerRegistry() { - return this.errorHandlerRegistry; - } - } + describe('error handling', () => { + it('calls registered error handler when BadRequestError is thrown', async () => { + // Prepare + const app = new TestResolver(); + + app.errorHandler(BadRequestError, (error) => ({ + statusCode: HttpErrorCodes.BAD_REQUEST, + error: 'Bad Request', + message: `Handled: ${error.message}`, + })); + + app.get('/test', () => { + throw new BadRequestError('test error'); + }); - const app = new TestRouterWithErrorAccess(); - const errorHandler = (error: BadRequestError) => ({ - statusCode: HttpErrorCodes.BAD_REQUEST, - error: error.name, - message: `Handled: ${error.message}`, + // Act + const result = (await app.resolve( + { path: '/test', method: 'GET' }, + context + )) as Response; + + // Assess + expect(result).toBeInstanceOf(Response); + expect(result.status).toBe(HttpErrorCodes.BAD_REQUEST); + expect(await result.text()).toBe( + JSON.stringify({ + statusCode: HttpErrorCodes.BAD_REQUEST, + error: 'Bad Request', + message: 'Handled: test error', + }) + ); }); - app.errorHandler(BadRequestError, errorHandler); + it('calls notFound handler when route is not found', async () => { + // Prepare + const app = new TestResolver(); - // Act & Assess - const registeredHandler = app.testErrorHandlerRegistry.resolve( - new BadRequestError('test') - ); - expect(registeredHandler).toBe(errorHandler); + app.notFound((error) => ({ + statusCode: HttpErrorCodes.NOT_FOUND, + error: 'Not Found', + message: `Custom: ${error.message}`, + })); + + // Act + const result = (await app.resolve( + { path: '/nonexistent', method: 'GET' }, + context + )) as Response; + + // Assess + expect(result).toBeInstanceOf(Response); + expect(result.status).toBe(HttpErrorCodes.NOT_FOUND); + expect(await result.text()).toBe( + JSON.stringify({ + statusCode: HttpErrorCodes.NOT_FOUND, + error: 'Not Found', + message: 'Custom: Route GET /nonexistent not found', + }) + ); + }); + + it('calls methodNotAllowed handler when MethodNotAllowedError is thrown', async () => { + // Prepare + const app = new TestResolver(); + + app.methodNotAllowed((error) => ({ + statusCode: HttpErrorCodes.METHOD_NOT_ALLOWED, + error: 'Method Not Allowed', + message: `Custom: ${error.message}`, + })); + + app.get('/test', () => { + throw new MethodNotAllowedError('POST not allowed'); + }); + + // Act + const result = (await app.resolve( + { path: '/test', method: 'GET' }, + context + )) as Response; + + // Assess + expect(result).toBeInstanceOf(Response); + expect(result.status).toBe(HttpErrorCodes.METHOD_NOT_ALLOWED); + expect(await result.text()).toBe( + JSON.stringify({ + statusCode: HttpErrorCodes.METHOD_NOT_ALLOWED, + error: 'Method Not Allowed', + message: 'Custom: POST not allowed', + }) + ); + }); + + it('falls back to default error handler when registered handler throws', async () => { + // Prepare + const app = new TestResolver(); + + app.errorHandler(BadRequestError, () => { + throw new Error('Handler failed'); + }); + + app.get('/test', () => { + throw new BadRequestError('original error'); + }); + + // Act + const result = (await app.resolve( + { path: '/test', method: 'GET' }, + context + )) as Response; + + // Assess + expect(result).toBeInstanceOf(Response); + expect(result.status).toBe(HttpErrorCodes.INTERNAL_SERVER_ERROR); + const body = await result.json(); + expect(body.statusCode).toBe(HttpErrorCodes.INTERNAL_SERVER_ERROR); + expect(body.error).toBe('Internal Server Error'); + expect(body.message).toBe('Handler failed'); + }); + + it('uses default handling when no error handler is registered', async () => { + // Prepare + const app = new TestResolver(); + + app.get('/test', () => { + throw new Error('unhandled error'); + }); + + // Act + const result = (await app.resolve( + { path: '/test', method: 'GET' }, + context + )) as Response; + + // Assess + expect(result).toBeInstanceOf(Response); + expect(result.status).toBe(HttpErrorCodes.INTERNAL_SERVER_ERROR); + const body = await result.json(); + expect(body.statusCode).toBe(HttpErrorCodes.INTERNAL_SERVER_ERROR); + expect(body.error).toBe('Internal Server Error'); + expect(body.message).toBe('unhandled error'); + }); + + it('calls most specific error handler when multiple handlers match', async () => { + // Prepare + const app = new TestResolver(); + + app.errorHandler(Error, () => ({ + statusCode: HttpErrorCodes.INTERNAL_SERVER_ERROR, + error: 'Generic Error', + message: 'Generic handler', + })); + + app.errorHandler(BadRequestError, () => ({ + statusCode: HttpErrorCodes.BAD_REQUEST, + error: 'Bad Request', + message: 'Specific handler', + })); + + app.get('/test', () => { + throw new BadRequestError('test error'); + }); + + // Act + const result = (await app.resolve( + { path: '/test', method: 'GET' }, + context + )) as Response; + + // Assess + expect(result).toBeInstanceOf(Response); + expect(result.status).toBe(HttpErrorCodes.BAD_REQUEST); + expect(await result.text()).toBe( + JSON.stringify({ + statusCode: HttpErrorCodes.BAD_REQUEST, + error: 'Bad Request', + message: 'Specific handler', + }) + ); + }); + + it('uses ServiceError toJSON method when no custom handler is registered', async () => { + // Prepare + const app = new TestResolver(); + + app.get('/test', () => { + throw new InternalServerError('service error'); + }); + + // Act + const result = (await app.resolve( + { path: '/test', method: 'GET' }, + context + )) as Response; + + // Assess + expect(result).toBeInstanceOf(Response); + expect(result.status).toBe(HttpErrorCodes.INTERNAL_SERVER_ERROR); + expect(await result.text()).toBe( + JSON.stringify({ + statusCode: HttpErrorCodes.INTERNAL_SERVER_ERROR, + error: 'InternalServerError', + message: 'service error', + }) + ); + }); + + it('hides error details in production mode', async () => { + // Prepare + vi.stubEnv('NODE_ENV', 'production'); + const app = new TestResolver(); + + app.get('/test', () => { + throw new Error('sensitive error details'); + }); + + // Act + const result = (await app.resolve( + { path: '/test', method: 'GET' }, + context + )) as Response; + + // Assess + expect(result).toBeInstanceOf(Response); + expect(result.status).toBe(HttpErrorCodes.INTERNAL_SERVER_ERROR); + const body = await result.json(); + expect(body.statusCode).toBe(HttpErrorCodes.INTERNAL_SERVER_ERROR); + expect(body.error).toBe('Internal Server Error'); + expect(body.message).toBe('Internal Server Error'); + expect(body.stack).toBeUndefined(); + expect(body.details).toBeUndefined(); + }); + + it('shows error details in development mode', async () => { + // Prepare + vi.stubEnv('POWERTOOLS_DEV', 'true'); + const app = new TestResolver(); + + app.get('/test', () => { + throw new Error('debug error details'); + }); + + // Act + const result = (await app.resolve( + { path: '/test', method: 'GET' }, + context + )) as Response; + + // Assess + expect(result).toBeInstanceOf(Response); + expect(result.status).toBe(HttpErrorCodes.INTERNAL_SERVER_ERROR); + const body = await result.json(); + expect(body.statusCode).toBe(HttpErrorCodes.INTERNAL_SERVER_ERROR); + expect(body.error).toBe('Internal Server Error'); + expect(body.message).toBe('debug error details'); + expect(body.stack).toBeDefined(); + expect(body.details).toBeDefined(); + expect(body.details.errorName).toBe('Error'); + }); + + it('accepts array of error types for single handler', async () => { + // Prepare + const app = new TestResolver(); + + app.errorHandler( + [BadRequestError, MethodNotAllowedError], + (error: Error) => ({ + statusCode: HttpErrorCodes.UNPROCESSABLE_ENTITY, + error: 'Validation Error', + message: `Array handler: ${error.message}`, + }) + ); + + app.get('/bad', () => { + throw new BadRequestError('bad request'); + }); + + app.get('/method', () => { + throw new MethodNotAllowedError('method not allowed'); + }); + + // Act + const badResult = (await app.resolve( + { path: '/bad', method: 'GET' }, + context + )) as Response; + const methodResult = (await app.resolve( + { path: '/method', method: 'GET' }, + context + )) as Response; + + // Assess + expect(badResult.status).toBe(HttpErrorCodes.UNPROCESSABLE_ENTITY); + expect(await badResult.json()).toEqual({ + statusCode: HttpErrorCodes.UNPROCESSABLE_ENTITY, + error: 'Validation Error', + message: 'Array handler: bad request', + }); + + expect(methodResult.status).toBe(HttpErrorCodes.UNPROCESSABLE_ENTITY); + expect(await methodResult.json()).toEqual({ + statusCode: HttpErrorCodes.UNPROCESSABLE_ENTITY, + error: 'Validation Error', + message: 'Array handler: method not allowed', + }); + }); + + it('replaces previous handler when registering new handler for same error type', async () => { + // Prepare + const app = new TestResolver(); + + app.errorHandler(BadRequestError, () => ({ + statusCode: HttpErrorCodes.BAD_REQUEST, + error: 'First Handler', + message: 'first', + })); + + app.errorHandler(BadRequestError, (error) => ({ + statusCode: HttpErrorCodes.UNPROCESSABLE_ENTITY, + error: 'Second Handler', + message: `second: ${error.message}`, + })); + + app.get('/test', () => { + throw new BadRequestError('test error'); + }); + + // Act + const result = (await app.resolve( + { path: '/test', method: 'GET' }, + context + )) as Response; + + // Assess + expect(result.status).toBe(HttpErrorCodes.UNPROCESSABLE_ENTITY); + expect(await result.json()).toEqual({ + statusCode: HttpErrorCodes.UNPROCESSABLE_ENTITY, + error: 'Second Handler', + message: 'second: test error', + }); + }); + + it('returns response with correct Content-Type header', async () => { + // Prepare + const app = new TestResolver(); + + app.errorHandler(BadRequestError, (error) => ({ + statusCode: HttpErrorCodes.BAD_REQUEST, + error: 'Bad Request', + message: error.message, + })); + + app.get('/test', () => { + throw new BadRequestError('test error'); + }); + + // Act + const result = (await app.resolve( + { path: '/test', method: 'GET' }, + context + )) as Response; + + // Assess + expect(result.headers.get('Content-Type')).toBe('application/json'); + }); }); }); From bde82db445f4a71c2fd42854e0bb65809a763dc2 Mon Sep 17 00:00:00 2001 From: svozza Date: Wed, 13 Aug 2025 09:58:03 +0100 Subject: [PATCH 2/2] add PR comments --- packages/event-handler/src/rest/BaseRouter.ts | 14 +++------ packages/event-handler/src/types/rest.ts | 2 +- .../tests/unit/rest/BaseRouter.test.ts | 29 +++++++++---------- 3 files changed, 19 insertions(+), 26 deletions(-) diff --git a/packages/event-handler/src/rest/BaseRouter.ts b/packages/event-handler/src/rest/BaseRouter.ts index 51ddd412d4..85b6aa44ec 100644 --- a/packages/event-handler/src/rest/BaseRouter.ts +++ b/packages/event-handler/src/rest/BaseRouter.ts @@ -112,11 +112,11 @@ abstract class BaseRouter { * @param error - The error to handle * @returns A Response object with appropriate status code and error details */ - protected handleError(error: Error): Response { + protected async handleError(error: Error): Promise { const handler = this.errorHandlerRegistry.resolve(error); - if (handler != null) { + if (handler !== null) { try { - const body = handler(error); + const body = await handler(error); return new Response(JSON.stringify(body), { status: body.statusCode, headers: { 'Content-Type': 'application/json' }, @@ -144,17 +144,11 @@ abstract class BaseRouter { * @returns A Response object with 500 status and error details */ #defaultErrorHandler(error: Error): Response { - const isProduction = - getStringFromEnv({ - key: 'NODE_ENV', - defaultValue: '', - }) === 'production'; - return new Response( JSON.stringify({ statusCode: 500, error: 'Internal Server Error', - message: isProduction ? 'Internal Server Error' : error.message, + message: isDevMode() ? error.message : 'Internal Server Error', ...(isDevMode() && { stack: error.stack, details: { errorName: error.name }, diff --git a/packages/event-handler/src/types/rest.ts b/packages/event-handler/src/types/rest.ts index ffe71f3c89..d2da7239b3 100644 --- a/packages/event-handler/src/types/rest.ts +++ b/packages/event-handler/src/types/rest.ts @@ -20,7 +20,7 @@ interface ErrorContext { type ErrorHandler = ( error: T, context?: ErrorContext -) => ErrorResponse; +) => Promise; interface ErrorConstructor { new (...args: any[]): T; diff --git a/packages/event-handler/tests/unit/rest/BaseRouter.test.ts b/packages/event-handler/tests/unit/rest/BaseRouter.test.ts index b1b3b7903d..4326b7da6f 100644 --- a/packages/event-handler/tests/unit/rest/BaseRouter.test.ts +++ b/packages/event-handler/tests/unit/rest/BaseRouter.test.ts @@ -49,7 +49,7 @@ describe('Class: BaseRouter', () => { throw new NotFoundError(`Route ${method} ${path} not found`); return route.handler(event, context); } catch (error) { - return this.handleError(error as Error); + return await this.handleError(error as Error); } } } @@ -230,7 +230,7 @@ describe('Class: BaseRouter', () => { // Prepare const app = new TestResolver(); - app.errorHandler(BadRequestError, (error) => ({ + app.errorHandler(BadRequestError, async (error) => ({ statusCode: HttpErrorCodes.BAD_REQUEST, error: 'Bad Request', message: `Handled: ${error.message}`, @@ -262,7 +262,7 @@ describe('Class: BaseRouter', () => { // Prepare const app = new TestResolver(); - app.notFound((error) => ({ + app.notFound(async (error) => ({ statusCode: HttpErrorCodes.NOT_FOUND, error: 'Not Found', message: `Custom: ${error.message}`, @@ -290,7 +290,7 @@ describe('Class: BaseRouter', () => { // Prepare const app = new TestResolver(); - app.methodNotAllowed((error) => ({ + app.methodNotAllowed(async (error) => ({ statusCode: HttpErrorCodes.METHOD_NOT_ALLOWED, error: 'Method Not Allowed', message: `Custom: ${error.message}`, @@ -322,7 +322,7 @@ describe('Class: BaseRouter', () => { // Prepare const app = new TestResolver(); - app.errorHandler(BadRequestError, () => { + app.errorHandler(BadRequestError, async () => { throw new Error('Handler failed'); }); @@ -342,7 +342,7 @@ describe('Class: BaseRouter', () => { const body = await result.json(); expect(body.statusCode).toBe(HttpErrorCodes.INTERNAL_SERVER_ERROR); expect(body.error).toBe('Internal Server Error'); - expect(body.message).toBe('Handler failed'); + expect(body.message).toBe('Internal Server Error'); }); it('uses default handling when no error handler is registered', async () => { @@ -365,20 +365,20 @@ describe('Class: BaseRouter', () => { const body = await result.json(); expect(body.statusCode).toBe(HttpErrorCodes.INTERNAL_SERVER_ERROR); expect(body.error).toBe('Internal Server Error'); - expect(body.message).toBe('unhandled error'); + expect(body.message).toBe('Internal Server Error'); }); it('calls most specific error handler when multiple handlers match', async () => { // Prepare const app = new TestResolver(); - app.errorHandler(Error, () => ({ + app.errorHandler(Error, async () => ({ statusCode: HttpErrorCodes.INTERNAL_SERVER_ERROR, error: 'Generic Error', message: 'Generic handler', })); - app.errorHandler(BadRequestError, () => ({ + app.errorHandler(BadRequestError, async () => ({ statusCode: HttpErrorCodes.BAD_REQUEST, error: 'Bad Request', message: 'Specific handler', @@ -432,9 +432,8 @@ describe('Class: BaseRouter', () => { ); }); - it('hides error details in production mode', async () => { + it('hides error details when POWERTOOLS_DEV env var is not set', async () => { // Prepare - vi.stubEnv('NODE_ENV', 'production'); const app = new TestResolver(); app.get('/test', () => { @@ -491,7 +490,7 @@ describe('Class: BaseRouter', () => { app.errorHandler( [BadRequestError, MethodNotAllowedError], - (error: Error) => ({ + async (error: Error) => ({ statusCode: HttpErrorCodes.UNPROCESSABLE_ENTITY, error: 'Validation Error', message: `Array handler: ${error.message}`, @@ -536,13 +535,13 @@ describe('Class: BaseRouter', () => { // Prepare const app = new TestResolver(); - app.errorHandler(BadRequestError, () => ({ + app.errorHandler(BadRequestError, async () => ({ statusCode: HttpErrorCodes.BAD_REQUEST, error: 'First Handler', message: 'first', })); - app.errorHandler(BadRequestError, (error) => ({ + app.errorHandler(BadRequestError, async (error) => ({ statusCode: HttpErrorCodes.UNPROCESSABLE_ENTITY, error: 'Second Handler', message: `second: ${error.message}`, @@ -571,7 +570,7 @@ describe('Class: BaseRouter', () => { // Prepare const app = new TestResolver(); - app.errorHandler(BadRequestError, (error) => ({ + app.errorHandler(BadRequestError, async (error) => ({ statusCode: HttpErrorCodes.BAD_REQUEST, error: 'Bad Request', message: error.message,