diff --git a/packages/event-handler/src/rest/Router.ts b/packages/event-handler/src/rest/Router.ts index a8cd9b37a4..d54cec4a02 100644 --- a/packages/event-handler/src/rest/Router.ts +++ b/packages/event-handler/src/rest/Router.ts @@ -226,33 +226,40 @@ class Router { const route = this.routeRegistry.resolve(method, path); - if (route === null) { - throw new NotFoundError(`Route ${path} for method ${method} not found`); - } - - const handler = - options?.scope != null - ? route.handler.bind(options.scope) - : route.handler; - const handlerMiddleware: Middleware = async (params, reqCtx, next) => { - const handlerResult = await handler(params, reqCtx); - reqCtx.res = handlerResultToWebResponse( - handlerResult, - reqCtx.res.headers - ); + if (route === null) { + const notFoundRes = await this.handleError( + new NotFoundError(`Route ${path} for method ${method} not found`), + { ...reqCtx, scope: options?.scope } + ); + reqCtx.res = handlerResultToWebResponse( + notFoundRes, + reqCtx.res.headers + ); + } else { + const handler = + options?.scope != null + ? route.handler.bind(options.scope) + : route.handler; + + const handlerResult = await handler(params, reqCtx); + reqCtx.res = handlerResultToWebResponse( + handlerResult, + reqCtx.res.headers + ); + } await next(); }; const middleware = composeMiddleware([ ...this.middleware, - ...route.middleware, + ...(route?.middleware ?? []), handlerMiddleware, ]); const middlewareResult = await middleware( - route.params, + route?.params ?? {}, requestContext, () => Promise.resolve() ); diff --git a/packages/event-handler/src/rest/converters.ts b/packages/event-handler/src/rest/converters.ts index 0eed7f92e6..d82f2ada5c 100644 --- a/packages/event-handler/src/rest/converters.ts +++ b/packages/event-handler/src/rest/converters.ts @@ -140,7 +140,14 @@ export const handlerResultToWebResponse = ( resHeaders?: Headers ): Response => { if (response instanceof Response) { - return response; + const headers = new Headers(resHeaders); + for (const [key, value] of response.headers.entries()) { + headers.set(key, value); + } + return new Response(response.body, { + status: response.status, + headers, + }); } const headers = new Headers(resHeaders); diff --git a/packages/event-handler/tests/unit/rest/Router/decorators.test.ts b/packages/event-handler/tests/unit/rest/Router/decorators.test.ts index 246093cc05..07cae4b0f2 100644 --- a/packages/event-handler/tests/unit/rest/Router/decorators.test.ts +++ b/packages/event-handler/tests/unit/rest/Router/decorators.test.ts @@ -256,29 +256,32 @@ describe('Class: Router - Decorators', () => { }); }); - it('works with notFound decorator', async () => { + it('works with notFound decorator and preserves scope', async () => { // Prepare const app = new Router(); class Lambda { + public scope = 'scoped'; + @app.notFound() public async handleNotFound(error: NotFoundError) { return { statusCode: HttpErrorCodes.NOT_FOUND, error: 'Not Found', - message: `Decorated: ${error.message}`, + message: `${this.scope}: ${error.message}`, }; } public async handler(event: unknown, _context: Context) { - return app.resolve(event, _context); + return app.resolve(event, _context, { scope: this }); } } const lambda = new Lambda(); + const handler = lambda.handler.bind(lambda); // Act - const result = await lambda.handler( + const result = await handler( createTestEvent('/nonexistent', 'GET'), context ); @@ -289,7 +292,7 @@ describe('Class: Router - Decorators', () => { body: JSON.stringify({ statusCode: HttpErrorCodes.NOT_FOUND, error: 'Not Found', - message: 'Decorated: Route /nonexistent for method GET not found', + message: 'scoped: Route /nonexistent for method GET not found', }), headers: { 'content-type': 'application/json' }, isBase64Encoded: false, diff --git a/packages/event-handler/tests/unit/rest/Router/middleware.test.ts b/packages/event-handler/tests/unit/rest/Router/middleware.test.ts index cf1a169be3..6c239c80d6 100644 --- a/packages/event-handler/tests/unit/rest/Router/middleware.test.ts +++ b/packages/event-handler/tests/unit/rest/Router/middleware.test.ts @@ -421,6 +421,33 @@ describe('Class: Router - Middleware', () => { }); }); + it('headers set by handler before next() take precedence', async () => { + // Prepare + const app = new Router(); + + app.use(async (_params, reqCtx, next) => { + reqCtx.res.headers.set('x-before-handler', 'middleware-value'); + await next(); + }); + + app.get('/handler-precedence', async () => { + const response = Response.json({ success: true }); + response.headers.set('x-before-handler', 'handler-value'); + return response; + }); + + // Act + const result = await app.resolve( + createTestEvent('/handler-precedence', 'GET'), + context + ); + + // Assess + expect(result.statusCode).toBe(200); + expect(result.headers?.['content-type']).toBe('application/json'); + expect(result.headers?.['x-before-handler']).toBe('handler-value'); + }); + it('overwrites headers when set later in the middleware stack', async () => { // Prepare const app = new Router(); @@ -455,6 +482,24 @@ describe('Class: Router - Middleware', () => { }); }); + it('executes global middleware even when route is not found', async () => { + // Prepare + const app = new Router(); + const executionOrder: string[] = []; + + const globalMiddleware = createTrackingMiddleware( + 'global', + executionOrder + ); + app.use(globalMiddleware); + + // Act + await app.resolve(createTestEvent('/nonexistent', 'GET'), context); + + // Assess + expect(executionOrder).toEqual(['global-start', 'global-end']); + }); + it('works with class decorators and preserves scope access', async () => { // Prepare const app = new Router(); diff --git a/packages/event-handler/tests/unit/rest/converters.test.ts b/packages/event-handler/tests/unit/rest/converters.test.ts index 5bac1f13b9..e33c6136e6 100644 --- a/packages/event-handler/tests/unit/rest/converters.test.ts +++ b/packages/event-handler/tests/unit/rest/converters.test.ts @@ -549,17 +549,6 @@ describe('Converters', () => { }); describe('handlerResultToResponse', () => { - it('returns Response object as-is', () => { - // Prepare - const response = new Response('Hello', { status: 201 }); - - // Act - const result = handlerResultToWebResponse(response); - - // Assess - expect(result).toBe(response); - }); - it('converts APIGatewayProxyResult to Response', async () => { // Prepare const proxyResult = { @@ -678,5 +667,25 @@ describe('Converters', () => { // Assess expect(result.headers.get('content-type')).toBe('text/plain'); }); + + it('merges headers from resHeaders with Response object, headers from Response take precedence', () => { + // Prepare + const response = new Response('Hello', { + headers: { 'content-type': 'text/plain' }, + }); + const resHeaders = new Headers({ + 'x-custom': 'value', + 'content-type': 'application/json', // should not override existing + }); + + // Act + const result = handlerResultToWebResponse(response, resHeaders); + + // Assess + expect(result.headers.get('content-type')).toBe('text/plain'); + expect(result.headers.get('x-custom')).toBe('value'); + expect(result.status).toBe(200); + expect(result.text()).resolves.toBe('Hello'); + }); }); });