Skip to content

Commit 8dd304c

Browse files
committed
fix(node): Adjust Express URL parameterization for array routes
1 parent 7a0dc54 commit 8dd304c

File tree

3 files changed

+81
-8
lines changed

3 files changed

+81
-8
lines changed

packages/node-integration-tests/suites/express/tracing/server.ts

+4
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ app.get(/\/test\/regex/, (_req, res) => {
2525
res.send({ response: 'response 2' });
2626
});
2727

28+
app.get(['/test/array1', /\/test\/array[2-9]/], (_req, res) => {
29+
res.send({ response: 'response 3' });
30+
});
31+
2832
app.use(Sentry.Handlers.errorHandler());
2933

3034
export default app;

packages/node-integration-tests/suites/express/tracing/test.ts

+29
Original file line numberDiff line numberDiff line change
@@ -53,3 +53,32 @@ test('should set a correct transaction name for routes specified in RegEx', asyn
5353
},
5454
});
5555
});
56+
57+
test.each([
58+
['', 'array1'],
59+
['', 'array5'],
60+
])('%s should set a correct transaction name for routes consisting of arrays of routes', async (_, segment) => {
61+
const url = await runServer(__dirname, `${__dirname}/server.ts`);
62+
const envelope = await getEnvelopeRequest(`${url}/${segment}`);
63+
64+
expect(envelope).toHaveLength(3);
65+
66+
assertSentryTransaction(envelope[2], {
67+
transaction: 'GET /test/array1,/\\/test\\/array[2-9]',
68+
transaction_info: {
69+
source: 'route',
70+
},
71+
contexts: {
72+
trace: {
73+
data: {
74+
url: `/test/${segment}`,
75+
},
76+
op: 'http.server',
77+
status: 'ok',
78+
tags: {
79+
'http.status_code': '200',
80+
},
81+
},
82+
},
83+
});
84+
});

packages/tracing/src/integrations/node/express.ts

+48-8
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,13 @@ type ExpressRouter = Router & {
5151
) => unknown;
5252
};
5353

54+
type RouteType = string | RegExp;
55+
5456
/* Type used for pathing the express router prototype */
5557
type Layer = {
5658
match: (path: string) => boolean;
5759
handle_request: (req: PatchedRequest, res: ExpressResponse, next: () => void) => void;
58-
route?: { path: string | RegExp };
60+
route?: { path: RouteType | RouteType[] };
5961
path?: string;
6062
};
6163

@@ -273,11 +275,8 @@ function instrumentRouter(appOrRouter: ExpressRouter): void {
273275
req._reconstructedRoute = '';
274276
}
275277

276-
// If the layer's partial route has params, the route is stored in layer.route.
277-
// Since a route might be defined with a RegExp, we convert it toString to make sure we end up with a string
278-
const lrp = layer.route?.path;
279-
const isRegex = isRegExp(lrp);
280-
const layerRoutePath = isRegex ? lrp?.toString() : (lrp as string);
278+
// If the layer's partial route has params, is a regex or an array, the route is stored in layer.route.
279+
const { layerRoutePath, isRegex, numExtraSegments }: LayerRoutePathInfo = getLayerRoutePathString(layer);
281280

282281
// Otherwise, the hardcoded path (i.e. a partial route without params) is stored in layer.path
283282
const partialRoute = layerRoutePath || layer.path || '';
@@ -301,7 +300,7 @@ function instrumentRouter(appOrRouter: ExpressRouter): void {
301300
// Now we check if we are in the "last" part of the route. We determine this by comparing the
302301
// number of URL segments from the original URL to that of our reconstructed parameterized URL.
303302
// If we've reached our final destination, we update the transaction name.
304-
const urlLength = getNumberOfUrlSegments(req.originalUrl || '');
303+
const urlLength = getNumberOfUrlSegments(req.originalUrl || '') + numExtraSegments;
305304
const routeLength = getNumberOfUrlSegments(req._reconstructedRoute);
306305

307306
if (urlLength === routeLength) {
@@ -319,7 +318,48 @@ function instrumentRouter(appOrRouter: ExpressRouter): void {
319318
};
320319
}
321320

321+
type LayerRoutePathInfo = {
322+
layerRoutePath?: string;
323+
isRegex: boolean;
324+
numExtraSegments: number;
325+
};
326+
327+
/**
328+
* Extracts and stringifies the layer's route which can either be a string with parameters (`users/:id`),
329+
* a RegEx (`/test/`) or an array of strings and regexes (`['/path1', /\/path[2-5]/, /path/:id]`).
330+
*
331+
* @param layer the layer to extract the stringified route from
332+
*
333+
* @returns an object containing the stringified route, a flag determining if the route was a regex
334+
* and the number of extra segments to the matched path that are additionally in the route,
335+
* if the route was an array (defaults to 0).
336+
*/
337+
function getLayerRoutePathString(layer: Layer): LayerRoutePathInfo {
338+
const lrp = layer.route?.path;
339+
340+
const isRegex = isRegExp(lrp);
341+
const isArray = Array.isArray(lrp);
342+
343+
const numExtraSegments = isArray ? getNumberOfArrayUrlSegments(lrp) - getNumberOfUrlSegments(layer.path || '') : 0;
344+
345+
const layerRoutePath = isArray ? lrp.join(',') : isRegex ? lrp?.toString() : (lrp as string | undefined);
346+
347+
return { layerRoutePath, isRegex, numExtraSegments };
348+
}
349+
350+
/**
351+
* Returns the number of URL segments in an array of routes
352+
*
353+
* Example: ['/api/test', /\/api\/post[0-9]/, '/users/:id/details`] -> 7
354+
*/
355+
function getNumberOfArrayUrlSegments(routesArray: RouteType[]): number {
356+
return routesArray.reduce((accNumSegments: number, currentRoute: RouteType) => {
357+
// array members can be a RegEx -> convert them toString
358+
return accNumSegments + getNumberOfUrlSegments(currentRoute.toString());
359+
}, 0);
360+
}
361+
322362
function getNumberOfUrlSegments(url: string): number {
323363
// split at '/' or at '\/' to split regex urls correctly
324-
return url.split(/\\?\//).filter(s => s.length > 0).length;
364+
return url.split(/\\?\//).filter(s => s.length > 0 && s !== ',').length;
325365
}

0 commit comments

Comments
 (0)