fix(node): Adjust Express URL parameterization for RegEx routes (#5483)

make our URL parameterization for Express (introduced in #5450) compatible with RegEx-defined routes.

Previously, as reported in #5481, our parameterization logic would cause a crash because instead of a string, the matched route would be of type `RegExp`. This PR adjusts our logic so that we detect if we get a matched string our regex. In the latter case, we also append a `'/'` to the reconstructed partial route name so that the regex is closed.

Author: Lms24

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

@@ -21,6 +21,10 @@ app.get('/test/express', (_req, res) => {
   res.send({ response: 'response 1' });
 });
 
+app.get(/\/test\/regex/, (_req, res) => {
+  res.send({ response: 'response 2' });
+});
+
 app.use(Sentry.Handlers.errorHandler());
 
 export default app;

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

@@ -27,3 +27,29 @@ test('should create and send transactions for Express routes and spans for middl
     ],
   });
 });
+
+test('should set a correct transaction name for routes specified in RegEx', async () => {
+  const url = await runServer(__dirname, `${__dirname}/server.ts`);
+  const envelope = await getEnvelopeRequest(`${url}/regex`);
+
+  expect(envelope).toHaveLength(3);
+
+  assertSentryTransaction(envelope[2], {
+    transaction: 'GET /\\/test\\/regex/',
+    transaction_info: {
+      source: 'route',
+    },
+    contexts: {
+      trace: {
+        data: {
+          url: '/test/regex',
+        },
+        op: 'http.server',
+        status: 'ok',
+        tags: {
+          'http.status_code': '200',
+        },
+      },
+    },
+  });
+});

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

@@ -1,6 +1,6 @@
 /* eslint-disable max-lines */
 import { Integration, Transaction } from '@sentry/types';
-import { CrossPlatformRequest, extractPathForTransaction, logger } from '@sentry/utils';
+import { CrossPlatformRequest, extractPathForTransaction, isRegExp, logger } from '@sentry/utils';
 
 type Method =
   | 'all'
@@ -55,7 +55,7 @@ type ExpressRouter = Router & {
 type Layer = {
   match: (path: string) => boolean;
   handle_request: (req: PatchedRequest, res: ExpressResponse, next: () => void) => void;
-  route?: { path: string };
+  route?: { path: string | RegExp };
   path?: string;
 };
 
@@ -273,9 +273,14 @@ function instrumentRouter(appOrRouter: ExpressRouter): void {
       req._reconstructedRoute = '';
     }
 
-    // If the layer's partial route has params, the route is stored in layer.route. Otherwise, the hardcoded path
-    // (i.e. a partial route without params) is stored in layer.path
-    const partialRoute = layer.route?.path || layer.path || '';
+    // If the layer's partial route has params, the route is stored in layer.route.
+    // Since a route might be defined with a RegExp, we convert it toString to make sure we end up with a string
+    const lrp = layer.route?.path;
+    const isRegex = isRegExp(lrp);
+    const layerRoutePath = isRegex ? lrp?.toString() : (lrp as string);
+
+    // Otherwise, the hardcoded path (i.e. a partial route without params) is stored in layer.path
+    const partialRoute = layerRoutePath || layer.path || '';
 
     // Normalize the partial route so that it doesn't contain leading or trailing slashes
     // and exclude empty or '*' wildcard routes.
@@ -288,15 +293,17 @@ function instrumentRouter(appOrRouter: ExpressRouter): void {
       .join('/');
 
     // If we found a valid partial URL, we append it to the reconstructed route
-    if (finalPartialRoute.length > 0) {
-      req._reconstructedRoute += `/${finalPartialRoute}`;
+    if (finalPartialRoute && finalPartialRoute.length > 0) {
+      // If the partial route is from a regex route, we append a '/' to close the regex
+      req._reconstructedRoute += `/${finalPartialRoute}${isRegex ? '/' : ''}`;
     }
 
     // Now we check if we are in the "last" part of the route. We determine this by comparing the
     // number of URL segments from the original URL to that of our reconstructed parameterized URL.
     // If we've reached our final destination, we update the transaction name.
-    const urlLength = req.originalUrl?.split('/').filter(s => s.length > 0).length;
-    const routeLength = req._reconstructedRoute.split('/').filter(s => s.length > 0).length;
+    const urlLength = getNumberOfUrlSegments(req.originalUrl || '');
+    const routeLength = getNumberOfUrlSegments(req._reconstructedRoute);
+
     if (urlLength === routeLength) {
       const transaction = res.__sentry_transaction;
       if (transaction && transaction.metadata.source !== 'custom') {
@@ -311,3 +318,8 @@ function instrumentRouter(appOrRouter: ExpressRouter): void {
     return originalProcessParams.call(this, layer, called, req, res, done);
   };
 }
+
+function getNumberOfUrlSegments(url: string): number {
+  // split at '/' or at '\/' to split regex urls correctly
+  return url.split(/\\?\//).filter(s => s.length > 0).length;
+}