Skip to content

Commit bf53f28

Browse files
committed
refactor(event-handler): use null for unmatched route instead of raw path
1 parent 887952a commit bf53f28

5 files changed

Lines changed: 35 additions & 13 deletions

File tree

packages/event-handler/src/http/Router.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ class Router<TEnv extends Env = Env> {
259259
context,
260260
req: options.req,
261261
res: options.res,
262-
route: '',
262+
route: null as string | null,
263263
params: {} as Record<string, string>,
264264
isHttpStreaming: options.isHttpStreaming,
265265
set: options.set,
@@ -346,7 +346,9 @@ class Router<TEnv extends Env = Env> {
346346

347347
const route = this.routeRegistry.resolve(method, path);
348348

349-
requestContext.route = route?.route ?? `${method} ${path}`;
349+
if (route !== null) {
350+
requestContext.route = route.route;
351+
}
350352

351353
const handlerMiddleware: Middleware = async ({ reqCtx, next }) => {
352354
let handlerRes: HandlerResponse;

packages/event-handler/src/http/middleware/metrics.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ const getEventMetadata = (reqCtx: RequestContext): Record<string, string> => {
5454
* A middleware for emitting per-request metrics using Powertools Metrics.
5555
*
5656
* This middleware automatically:
57-
* - Adds the matched route as a metric dimension (uses `NOT_FOUND` for 404 responses to prevent dimension explosion)
57+
* - Adds the matched route as a metric dimension (uses `NOT_FOUND` when no route matches to prevent dimension explosion)
5858
* - Emits `latency` (Milliseconds), `fault` (Count), and `error` (Count) metrics
5959
* - Adds `httpMethod` and `path` metadata for all requests
6060
* - Adds `ipAddress` and `userAgent` metadata from request headers when available
@@ -89,7 +89,6 @@ const metrics = (metrics: Metrics): Middleware => {
8989
throw error;
9090
} finally {
9191
const url = new URL(reqCtx.req.url);
92-
const is404 = status === 404;
9392
const metadata = {
9493
httpMethod: reqCtx.req.method,
9594
path: url.pathname,
@@ -101,7 +100,7 @@ const metrics = (metrics: Metrics): Middleware => {
101100
metrics.addMetadata(key, value);
102101
}
103102
metrics
104-
.addDimension('route', is404 ? 'NOT_FOUND' : reqCtx.route)
103+
.addDimension('route', reqCtx.route ?? 'NOT_FOUND')
105104
.addMetric(
106105
'latency',
107106
MetricUnit.Milliseconds,

packages/event-handler/src/types/http.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ type RequestContext<TEnv extends Env = Env> = {
170170
event: EventTypeMap[T];
171171
context: Context;
172172
res: Response;
173-
route: string;
173+
route: string | null;
174174
params: Record<string, string>;
175175
responseType: T;
176176
isBase64Encoded?: boolean;

packages/event-handler/tests/unit/http/Router/basic-routing.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,7 @@ describe.each([
567567
it('sets reqCtx.route to METHOD and path for static routes', async () => {
568568
// Prepare
569569
const app = new Router();
570-
let capturedRoute = '';
570+
let capturedRoute: string | null = null;
571571
app.use(async ({ reqCtx, next }) => {
572572
await next();
573573
capturedRoute = reqCtx.route;
@@ -584,7 +584,7 @@ describe.each([
584584
it('sets reqCtx.route to METHOD and pattern for dynamic routes', async () => {
585585
// Prepare
586586
const app = new Router();
587-
let capturedRoute = '';
587+
let capturedRoute: string | null = null;
588588
app.use(async ({ reqCtx, next }) => {
589589
await next();
590590
capturedRoute = reqCtx.route;
@@ -598,10 +598,10 @@ describe.each([
598598
expect(capturedRoute).toBe('GET /users/:id');
599599
});
600600

601-
it('sets reqCtx.route to METHOD and requested path for unmatched routes', async () => {
601+
it('sets reqCtx.route to null for unmatched routes', async () => {
602602
// Prepare
603603
const app = new Router();
604-
let capturedRoute = '';
604+
let capturedRoute: string | null = '';
605605
app.use(async ({ reqCtx, next }) => {
606606
await next();
607607
capturedRoute = reqCtx.route;
@@ -612,6 +612,6 @@ describe.each([
612612
await app.resolve(createEvent('/nonexistent', 'GET'), context);
613613

614614
// Assess
615-
expect(capturedRoute).toBe('GET /nonexistent');
615+
expect(capturedRoute).toBeNull();
616616
});
617617
});

packages/event-handler/tests/unit/http/middleware/metrics.test.ts

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { Metrics } from '@aws-lambda-powertools/metrics';
22
import context from '@aws-lambda-powertools/testing-utils/context';
33
import { beforeEach, describe, expect, it, vi } from 'vitest';
4-
import { BadRequestError } from '../../../../src/http/errors.js';
4+
import { BadRequestError, NotFoundError } from '../../../../src/http/errors.js';
55
import { metrics as metricsMiddleware } from '../../../../src/http/middleware/metrics.js';
66
import { Router } from '../../../../src/http/Router.js';
77
import {
@@ -159,7 +159,7 @@ describe('Metrics Middleware', () => {
159159
);
160160
});
161161

162-
it('uses NOT_FOUND as route dimension for 404 responses', async () => {
162+
it('uses NOT_FOUND as route dimension when no route matches', async () => {
163163
// Prepare
164164
const metrics = new Metrics({ namespace: 'test' });
165165
app.use(metricsMiddleware(metrics));
@@ -179,6 +179,27 @@ describe('Metrics Middleware', () => {
179179
);
180180
});
181181

182+
it('uses the matched route pattern when a handler returns 404', async () => {
183+
// Prepare
184+
const metrics = new Metrics({ namespace: 'test' });
185+
app.use(metricsMiddleware(metrics));
186+
app.get('/users/:id', () => {
187+
throw new NotFoundError('User not found');
188+
});
189+
190+
// Act
191+
await app.resolve(createTestEvent('/users/123', 'GET'), context);
192+
193+
// Assess
194+
expect(console.log).toHaveEmittedEMFWith(
195+
expect.objectContaining({
196+
route: 'GET /users/:id',
197+
statusCode: '404',
198+
error: 1,
199+
})
200+
);
201+
});
202+
182203
it('adds ipAddress from identity.sourceIp for API Gateway V1 events', async () => {
183204
// Prepare
184205
const metrics = new Metrics({ namespace: 'test' });

0 commit comments

Comments
 (0)