fix(react): Fix react router v4/v5 instrumentation (#11855)

Turns out we were not correctly setting parametrized route names for
react router v4/v5 😬 We had no proper test covering this.

So I added a new E2E test `react-router-5` that actually checks that
parametrization etc. works as expected.
While at it, I also updated the react-router-6 E2E test to actually
check these things as well - previously, we mostly only checked that
_anything_ was sent to sentry, but didn't look at the content we sent.

Fixes #11815

Author: mydea

.github/workflows/build.yml

@@ -1008,6 +1008,7 @@ jobs:
             'nextjs-14',
             'react-create-hash-router',
             'react-router-6-use-routes',
+            'react-router-5',
             'standard-frontend-react',
             'svelte-5',
             'sveltekit',

dev-packages/e2e-tests/test-applications/react-router-5/.gitignore

@@ -0,0 +1,29 @@
+# See https://help.github.com/articles/ignoring-files/ for more about ignoring files.
+
+# dependencies
+/node_modules
+/.pnp
+.pnp.js
+
+# testing
+/coverage
+
+# production
+/build
+
+# misc
+.DS_Store
+.env.local
+.env.development.local
+.env.test.local
+.env.production.local
+
+npm-debug.log*
+yarn-debug.log*
+yarn-error.log*
+
+/test-results/
+/playwright-report/
+/playwright/.cache/
+
+!*.d.ts

dev-packages/e2e-tests/test-applications/react-router-5/.npmrc

@@ -0,0 +1,2 @@
+@sentry:registry=http://127.0.0.1:4873
+@sentry-internal:registry=http://127.0.0.1:4873

dev-packages/e2e-tests/test-applications/react-router-5/package.json

@@ -0,0 +1,60 @@
+{
+  "name": "react-router-5-e2e-test-app",
+  "version": "0.1.0",
+  "private": true,
+  "dependencies": {
+    "@sentry/react": "latest || *",
+    "@testing-library/jest-dom": "5.14.1",
+    "@testing-library/react": "13.0.0",
+    "@testing-library/user-event": "13.2.1",
+    "history": "4.9.0",
+    "@types/history": "4.7.11",
+    "@types/jest": "27.0.1",
+    "@types/node": "16.7.13",
+    "@types/react": "18.0.0",
+    "@types/react-dom": "18.0.0",
+    "@types/react-router": "5.1.20",
+    "@types/react-router-dom": "5.3.3",
+    "react": "18.2.0",
+    "react-dom": "18.2.0",
+    "react-router-dom": "5.3.4",
+    "react-scripts": "5.0.1",
+    "typescript": "4.9.5",
+    "web-vitals": "2.1.0"
+  },
+  "scripts": {
+    "build": "react-scripts build",
+    "start": "serve -s build",
+    "test": "playwright test",
+    "clean": "npx rimraf node_modules,pnpm-lock.yaml",
+    "test:build": "pnpm install && npx playwright install && pnpm build",
+    "test:assert": "pnpm test"
+  },
+  "eslintConfig": {
+    "extends": [
+      "react-app",
+      "react-app/jest"
+    ]
+  },
+  "browserslist": {
+    "production": [
+      ">0.2%",
+      "not dead",
+      "not op_mini all"
+    ],
+    "development": [
+      "last 1 chrome version",
+      "last 1 firefox version",
+      "last 1 safari version"
+    ]
+  },
+  "devDependencies": {
+    "@playwright/test": "^1.43.1",
+    "@sentry-internal/event-proxy-server": "link:../../../event-proxy-server",
+    "ts-node": "^10.9.2",
+    "serve": "14.0.1"
+  },
+  "volta": {
+    "extends": "../../package.json"
+  }
+}

dev-packages/e2e-tests/test-applications/react-router-5/playwright.config.ts

@@ -0,0 +1,82 @@
+import type { PlaywrightTestConfig } from '@playwright/test';
+import { devices } from '@playwright/test';
+
+const reactPort = 3030;
+const eventProxyPort = 3031;
+
+/**
+ * See https://playwright.dev/docs/test-configuration.
+ */
+const config: PlaywrightTestConfig = {
+  testDir: './tests',
+  /* Maximum time one test can run for. */
+  timeout: 150_000,
+  expect: {
+    /**
+     * Maximum time expect() should wait for the condition to be met.
+     * For example in `await expect(locator).toHaveText();`
+     */
+    timeout: 5000,
+  },
+  /* Run tests in files in parallel */
+  fullyParallel: true,
+  /* Fail the build on CI if you accidentally left test.only in the source code. */
+  forbidOnly: !!process.env.CI,
+  /* Retry on CI only */
+  retries: 0,
+  /* Opt out of parallel tests on CI. */
+  workers: 1,
+  /* Reporter to use. See https://playwright.dev/docs/test-reporters */
+  reporter: 'list',
+  /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */
+  use: {
+    /* Maximum time each action such as `click()` can take. Defaults to 0 (no limit). */
+    actionTimeout: 0,
+
+    /* Collect trace when retrying the failed test. See https://playwright.dev/docs/trace-viewer */
+    trace: 'on-first-retry',
+
+    baseURL: `http://localhost:${reactPort}`,
+  },
+
+  /* Configure projects for major browsers */
+  projects: [
+    {
+      name: 'chromium',
+      use: {
+        ...devices['Desktop Chrome'],
+      },
+    },
+    // For now we only test Chrome!
+    // {
+    //   name: 'firefox',
+    //   use: {
+    //     ...devices['Desktop Firefox'],
+    //   },
+    // },
+    // {
+    //   name: 'webkit',
+    //   use: {
+    //     ...devices['Desktop Safari'],
+    //   },
+    // },
+  ],
+
+  /* Run your local dev server before starting the tests */
+
+  webServer: [
+    {
+      command: 'pnpm ts-node-script start-event-proxy.ts',
+      port: eventProxyPort,
+    },
+    {
+      command: 'pnpm start',
+      port: reactPort,
+      env: {
+        PORT: `${reactPort}`,
+      },
+    },
+  ],
+};
+
+export default config;

dev-packages/e2e-tests/test-applications/react-router-5/public/index.html

@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html lang="en">
+  <head>
+    <meta charset="utf-8" />
+    <meta name="viewport" content="width=device-width, initial-scale=1" />
+    <meta name="theme-color" content="#000000" />
+    <meta name="description" content="Web site created using create-react-app" />
+    <title>React App</title>
+  </head>
+  <body>
+    <noscript>You need to enable JavaScript to run this app.</noscript>
+    <div id="root"></div>
+    <!--
+      This HTML file is a template.
+      If you open it directly in the browser, you will see an empty page.
+
+      You can add webfonts, meta tags, or analytics to this file.
+      The build step will place the bundled scripts into the <body> tag.
+
+      To begin the development, run `npm start` or `yarn start`.
+      To create a production bundle, use `npm run build` or `yarn build`.
+    -->
+  </body>
+</html>

dev-packages/e2e-tests/test-applications/react-router-5/src/globals.d.ts

@@ -0,0 +1,5 @@
+interface Window {
+  recordedTransactions?: string[];
+  capturedExceptionId?: string;
+  sentryReplayId?: string;
+}

dev-packages/e2e-tests/test-applications/react-router-5/src/index.tsx

@@ -0,0 +1,42 @@
+import * as Sentry from '@sentry/react';
+import { createBrowserHistory } from 'history';
+// biome-ignore lint/nursery/noUnusedImports: <explanation>
+import React from 'react';
+import ReactDOM from 'react-dom/client';
+import { Route, Router, Switch } from 'react-router-dom';
+import Index from './pages/Index';
+import User from './pages/User';
+
+const replay = Sentry.replayIntegration();
+
+const history = createBrowserHistory();
+
+Sentry.init({
+  environment: 'qa', // dynamic sampling bias to keep transactions
+  dsn:
+    process.env.REACT_APP_E2E_TEST_DSN ||
+    'https://3b6c388182fb435097f41d181be2b2ba@o4504321058471936.ingest.sentry.io/4504321066008576',
+  integrations: [Sentry.reactRouterV5BrowserTracingIntegration({ history }), replay],
+  // We recommend adjusting this value in production, or using tracesSampler
+  // for finer control
+  tracesSampleRate: 1.0,
+  release: 'e2e-test',
+  tunnel: 'http://localhost:3031/', // proxy server
+
+  // Always capture replays, so we can test this properly
+  replaysSessionSampleRate: 1.0,
+  replaysOnErrorSampleRate: 0.0,
+});
+
+// Create Custom Sentry Route component
+export const SentryRoute = Sentry.withSentryRouting(Route);
+
+const root = ReactDOM.createRoot(document.getElementById('root') as HTMLElement);
+root.render(
+  <Router history={history}>
+    <Switch>
+      <SentryRoute path="/user/:id" component={User} />
+      <SentryRoute path="/" component={Index} />
+    </Switch>
+  </Router>,
+);

dev-packages/e2e-tests/test-applications/react-router-5/src/pages/Index.tsx

@@ -0,0 +1,25 @@
+import * as Sentry from '@sentry/react';
+// biome-ignore lint/nursery/noUnusedImports: Need React import for JSX
+import * as React from 'react';
+import { Link } from 'react-router-dom';
+
+const Index = () => {
+  return (
+    <>
+      <input
+        type="button"
+        value="Capture Exception"
+        id="exception-button"
+        onClick={() => {
+          const eventId = Sentry.captureException(new Error('I am an error!'));
+          window.capturedExceptionId = eventId;
+        }}
+      />
+      <Link to="/user/5" id="navigation">
+        navigate
+      </Link>
+    </>
+  );
+};
+
+export default Index;

dev-packages/e2e-tests/test-applications/react-router-5/src/pages/User.tsx

@@ -0,0 +1,8 @@
+// biome-ignore lint/nursery/noUnusedImports: Need React import for JSX
+import * as React from 'react';
+
+const User = (params: { id: string }) => {
+  return <p>Show user details for {params.id}</p>;
+};
+
+export default User;

dev-packages/e2e-tests/test-applications/react-router-5/src/react-app-env.d.ts

@@ -0,0 +1 @@
+/// <reference types="react-scripts" />

dev-packages/e2e-tests/test-applications/react-router-5/start-event-proxy.ts

@@ -0,0 +1,6 @@
+import { startEventProxyServer } from '@sentry-internal/event-proxy-server';
+
+startEventProxyServer({
+  port: 3031,
+  proxyServerName: 'react-router-5',
+});

dev-packages/e2e-tests/test-applications/react-router-5/tests/errors.test.ts

@@ -0,0 +1,59 @@
+import { expect, test } from '@playwright/test';
+import { waitForError, waitForTransaction } from '@sentry-internal/event-proxy-server';
+
+test('Sends correct error event', async ({ page }) => {
+  const errorEventPromise = waitForError('react-router-5', event => {
+    return !event.type && event.exception?.values?.[0]?.value === 'I am an error!';
+  });
+
+  await page.goto('/');
+
+  const exceptionButton = page.locator('id=exception-button');
+  await exceptionButton.click();
+
+  const errorEvent = await errorEventPromise;
+
+  expect(errorEvent.exception?.values).toHaveLength(1);
+  expect(errorEvent.exception?.values?.[0]?.value).toBe('I am an error!');
+
+  expect(errorEvent.request).toEqual({
+    headers: expect.any(Object),
+    url: 'http://localhost:3030/',
+  });
+
+  expect(errorEvent.transaction).toEqual('/');
+
+  expect(errorEvent.contexts?.trace).toEqual({
+    trace_id: expect.any(String),
+    span_id: expect.any(String),
+  });
+});
+
+test('Sets correct transactionName', async ({ page }) => {
+  const transactionPromise = waitForTransaction('react-router-5', async transactionEvent => {
+    return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload';
+  });
+
+  const errorEventPromise = waitForError('react-router-5', event => {
+    return !event.type && event.exception?.values?.[0]?.value === 'I am an error!';
+  });
+
+  await page.goto('/');
+  const transactionEvent = await transactionPromise;
+
+  // Only capture error once transaction was sent
+  const exceptionButton = page.locator('id=exception-button');
+  await exceptionButton.click();
+
+  const errorEvent = await errorEventPromise;
+
+  expect(errorEvent.exception?.values).toHaveLength(1);
+  expect(errorEvent.exception?.values?.[0]?.value).toBe('I am an error!');
+
+  expect(errorEvent.transaction).toEqual('/');
+
+  expect(errorEvent.contexts?.trace).toEqual({
+    trace_id: transactionEvent.contexts?.trace?.trace_id,
+    span_id: expect.not.stringContaining(transactionEvent.contexts?.trace?.span_id || ''),
+  });
+});