fix(react): Fix React Router v6 paramaterization

packages/react/src/reactrouterv6.tsx

@@ -2,7 +2,7 @@
 // https://gist.github.com/wontondon/e8c4bdf2888875e4c755712e99279536
 
 import { Transaction, TransactionContext, TransactionSource } from '@sentry/types';
-import { getGlobalObject, logger } from '@sentry/utils';
+import { getGlobalObject, getNumberOfUrlSegments, logger } from '@sentry/utils';
 import hoistNonReactStatics from 'hoist-non-react-statics';
 import React from 'react';
 
@@ -20,9 +20,23 @@ type Params<Key extends string = string> = {
   readonly [key in Key]: string | undefined;
 };
 
+// https://github.com/remix-run/react-router/blob/9fa54d643134cd75a0335581a75db8100ed42828/packages/react-router/lib/router.ts#L114-L134
 interface RouteMatch<ParamKey extends string = string> {
+  /**
+   * The names and values of dynamic parameters in the URL.
+   */
   params: Params<ParamKey>;
+  /**
+   * The portion of the URL pathname that was matched.
+   */
   pathname: string;
+  /**
+   * The portion of the URL pathname that was matched before child routes.
+   */
+  pathnameBase: string;
+  /**
+   * The route object that was used to match.
+   */
   route: RouteObject;
 }
 
@@ -94,13 +108,31 @@ function getNormalizedName(
 
   const branches = matchRoutes(routes, location);
 
+  let pathBuilder = '';
   if (branches) {
     // eslint-disable-next-line @typescript-eslint/prefer-for-of
     for (let x = 0; x < branches.length; x++) {
-      if (branches[x].route && branches[x].route.path && branches[x].pathname === location.pathname) {
-        const path = branches[x].route.path;
+      const branch = branches[x];
+      const route = branch.route;
+      if (route) {
+        // Early return if index route
+        if (route.index) {
+          return [branch.pathname, 'route'];
+        }
+
+        const path = route.path;
         if (path) {
-          return [path, 'route'];
+          const newPath = path[0] === '/' ? path : `/${path}`;
+          pathBuilder += newPath;
+          if (branch.pathname === location.pathname) {
+            // If the route defined on the element is something like
+            // <Route path="/stores/:storeId/products/:productId" element={<div>Product</div>} />
+            // We should check against the branch.pathname for the number of / seperators
+            if (getNumberOfUrlSegments(pathBuilder) !== getNumberOfUrlSegments(branch.pathname)) {
+              return [newPath, 'route'];
+            }
+            return [pathBuilder, 'route'];
+          }
         }
       }
     }

packages/react/test/reactrouterv6.test.tsx

@@ -196,4 +196,38 @@ describe('React Router v6', () => {
       metadata: { source: 'route' },
     });
   });
+
+  it('works with nested paths with parameters', () => {
+    const [mockStartTransaction] = createInstrumentation();
+    const SentryRoutes = withSentryReactRouterV6Routing(Routes);
+
+    render(
+      <MemoryRouter initialEntries={['/']}>
+        <SentryRoutes>
+          <Route index element={<Navigate to="/projects/123/views/234" />} />
+          <Route path="account" element={<div>Account Page</div>} />
+          <Route path="projects">
+            <Route index element={<div>Project Index</div>} />
+            <Route path=":projectId" element={<div>Project Page</div>}>
+              <Route index element={<div>Project Page Root</div>} />
+              <Route element={<div>Editor</div>}>
+                <Route path="views/:viewId" element={<div>View Canvas</div>} />
+                <Route path="spaces/:spaceId" element={<div>Space Canvas</div>} />
+              </Route>
+            </Route>
+          </Route>
+
+          <Route path="*" element={<div>No Match Page</div>} />
+        </SentryRoutes>
+      </MemoryRouter>,
+    );
+
+    expect(mockStartTransaction).toHaveBeenCalledTimes(2);
+    expect(mockStartTransaction).toHaveBeenLastCalledWith({
+      name: '/projects/:projectId/views/:viewId',
+      op: 'navigation',
+      tags: { 'routing.instrumentation': 'react-router-v6' },
+      metadata: { source: 'route' },
+    });
+  });
 });

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

@@ -1,6 +1,12 @@
 /* eslint-disable max-lines */
 import { Integration, Transaction } from '@sentry/types';
-import { CrossPlatformRequest, extractPathForTransaction, isRegExp, logger } from '@sentry/utils';
+import {
+  CrossPlatformRequest,
+  extractPathForTransaction,
+  getNumberOfUrlSegments,
+  isRegExp,
+  logger,
+} from '@sentry/utils';
 
 type Method =
   | 'all'
@@ -384,15 +390,6 @@ function getNumberOfArrayUrlSegments(routesArray: RouteType[]): number {
   }, 0);
 }
 
-/**
- * Returns number of URL segments of a passed URL.
- * Also handles URLs of type RegExp
- */
-function getNumberOfUrlSegments(url: string): number {
-  // split at '/' or at '\/' to split regex urls correctly
-  return url.split(/\\?\//).filter(s => s.length > 0 && s !== ',').length;
-}
-
 /**
  * Extracts and returns the stringified version of the layers route path
  * Handles route arrays (by joining the paths together) as well as RegExp and normal

packages/utils/src/index.ts

@@ -25,3 +25,4 @@ export * from './envelope';
 export * from './clientreport';
 export * from './ratelimit';
 export * from './baggage';
+export * from './url';

packages/utils/src/misc.ts

@@ -41,40 +41,6 @@ export function uuid4(): string {
   );
 }
 
-/**
- * Parses string form of URL into an object
- * // borrowed from https://tools.ietf.org/html/rfc3986#appendix-B
- * // intentionally using regex and not <a/> href parsing trick because React Native and other
- * // environments where DOM might not be available
- * @returns parsed URL object
- */
-export function parseUrl(url: string): {
-  host?: string;
-  path?: string;
-  protocol?: string;
-  relative?: string;
-} {
-  if (!url) {
-    return {};
-  }
-
-  const match = url.match(/^(([^:/?#]+):)?(\/\/([^/?#]*))?([^?#]*)(\?([^#]*))?(#(.*))?$/);
-
-  if (!match) {
-    return {};
-  }
-
-  // coerce to undefined values to empty string so we don't get 'undefined'
-  const query = match[6] || '';
-  const fragment = match[8] || '';
-  return {
-    host: match[4],
-    path: match[5],
-    protocol: match[2],
-    relative: match[5] + query + fragment, // everything minus origin
-  };
-}
-
 function getFirstException(event: Event): Exception | undefined {
   return event.exception && event.exception.values ? event.exception.values[0] : undefined;
 }
@@ -197,17 +163,6 @@ export function addContextToFrame(lines: string[], frame: StackFrame, linesOfCon
     .map((line: string) => snipLine(line, 0));
 }
 
-/**
- * Strip the query string and fragment off of a given URL or path (if present)
- *
- * @param urlPath Full URL or path, including possible query string and/or fragment
- * @returns URL or path without query string or fragment
- */
-export function stripUrlQueryAndFragment(urlPath: string): string {
-  // eslint-disable-next-line no-useless-escape
-  return urlPath.split(/[\?#]/, 1)[0];
-}
-
 /**
  * Checks whether or not we've already captured the given exception (note: not an identical exception - the very object
  * in question), and marks it captured if not.

packages/utils/src/requestdata.ts

@@ -15,8 +15,8 @@
 import { Event, ExtractedNodeRequestData, Transaction, TransactionSource } from '@sentry/types';
 
 import { isPlainObject, isString } from './is';
-import { stripUrlQueryAndFragment } from './misc';
 import { normalize } from './normalize';
+import { stripUrlQueryAndFragment } from './url';
 
 const DEFAULT_INCLUDES = {
   ip: false,

packages/utils/src/url.ts

@@ -0,0 +1,52 @@
+/**
+ * Parses string form of URL into an object
+ * // borrowed from https://tools.ietf.org/html/rfc3986#appendix-B
+ * // intentionally using regex and not <a/> href parsing trick because React Native and other
+ * // environments where DOM might not be available
+ * @returns parsed URL object
+ */
+export function parseUrl(url: string): {
+  host?: string;
+  path?: string;
+  protocol?: string;
+  relative?: string;
+} {
+  if (!url) {
+    return {};
+  }
+
+  const match = url.match(/^(([^:/?#]+):)?(\/\/([^/?#]*))?([^?#]*)(\?([^#]*))?(#(.*))?$/);
+
+  if (!match) {
+    return {};
+  }
+
+  // coerce to undefined values to empty string so we don't get 'undefined'
+  const query = match[6] || '';
+  const fragment = match[8] || '';
+  return {
+    host: match[4],
+    path: match[5],
+    protocol: match[2],
+    relative: match[5] + query + fragment, // everything minus origin
+  };
+}
+
+/**
+ * Strip the query string and fragment off of a given URL or path (if present)
+ *
+ * @param urlPath Full URL or path, including possible query string and/or fragment
+ * @returns URL or path without query string or fragment
+ */
+export function stripUrlQueryAndFragment(urlPath: string): string {
+  // eslint-disable-next-line no-useless-escape
+  return urlPath.split(/[\?#]/, 1)[0];
+}
+
+/**
+ * Returns number of URL segments of a passed string URL.
+ */
+export function getNumberOfUrlSegments(url: string): number {
+  // split at '/' or at '\/' to split regex urls correctly
+  return url.split(/\\?\//).filter(s => s.length > 0 && s !== ',').length;
+}

packages/utils/test/misc.test.ts

@@ -5,7 +5,6 @@ import {
   addExceptionMechanism,
   checkOrSetAlreadyCaught,
   getEventDescription,
-  stripUrlQueryAndFragment,
   uuid4,
 } from '../src/misc';
 
@@ -187,27 +186,6 @@ describe('addContextToFrame', () => {
   });
 });
 
-describe('stripQueryStringAndFragment', () => {
-  const urlString = 'http://dogs.are.great:1231/yay/';
-  const queryString = '?furry=yes&funny=very';
-  const fragment = '#adoptnotbuy';
-
-  it('strips query string from url', () => {
-    const urlWithQueryString = `${urlString}${queryString}`;
-    expect(stripUrlQueryAndFragment(urlWithQueryString)).toBe(urlString);
-  });
-
-  it('strips fragment from url', () => {
-    const urlWithFragment = `${urlString}${fragment}`;
-    expect(stripUrlQueryAndFragment(urlWithFragment)).toBe(urlString);
-  });
-
-  it('strips query string and fragment from url', () => {
-    const urlWithQueryStringAndFragment = `${urlString}${queryString}${fragment}`;
-    expect(stripUrlQueryAndFragment(urlWithQueryStringAndFragment)).toBe(urlString);
-  });
-});
-
 describe('addExceptionMechanism', () => {
   const defaultMechanism = { type: 'generic', handled: true };
 

packages/utils/test/url.test.ts

@@ -0,0 +1,33 @@
+import { getNumberOfUrlSegments, stripUrlQueryAndFragment } from '../src/url';
+
+describe('stripQueryStringAndFragment', () => {
+  const urlString = 'http://dogs.are.great:1231/yay/';
+  const queryString = '?furry=yes&funny=very';
+  const fragment = '#adoptnotbuy';
+
+  it('strips query string from url', () => {
+    const urlWithQueryString = `${urlString}${queryString}`;
+    expect(stripUrlQueryAndFragment(urlWithQueryString)).toBe(urlString);
+  });
+
+  it('strips fragment from url', () => {
+    const urlWithFragment = `${urlString}${fragment}`;
+    expect(stripUrlQueryAndFragment(urlWithFragment)).toBe(urlString);
+  });
+
+  it('strips query string and fragment from url', () => {
+    const urlWithQueryStringAndFragment = `${urlString}${queryString}${fragment}`;
+    expect(stripUrlQueryAndFragment(urlWithQueryStringAndFragment)).toBe(urlString);
+  });
+});
+
+describe('getNumberOfUrlSegments', () => {
+  test.each([
+    ['regular path', '/projects/123/views/234', 4],
+    ['single param paramaterized path', '/users/:id/details', 3],
+    ['multi param paramaterized path', '/stores/:storeId/products/:productId', 4],
+    ['regex path', String(/\/api\/post[0-9]/), 2],
+  ])('%s', (_: string, input, output) => {
+    expect(getNumberOfUrlSegments(input)).toEqual(output);
+  });
+});