diff --git a/dev-packages/e2e-tests/test-applications/hydrogen-react-router-7/tests/server-transactions.test.ts b/dev-packages/e2e-tests/test-applications/hydrogen-react-router-7/tests/server-transactions.test.ts index 0455ea2e0b79..1dca64548e83 100644 --- a/dev-packages/e2e-tests/test-applications/hydrogen-react-router-7/tests/server-transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/hydrogen-react-router-7/tests/server-transactions.test.ts @@ -13,7 +13,8 @@ test('Sends parameterized transaction name to Sentry', async ({ page }) => { const transaction = await transactionPromise; expect(transaction).toBeDefined(); - expect(transaction.transaction).toBe('GET /user/123'); + // Transaction name should be parameterized (route pattern, not actual URL) + expect(transaction.transaction).toBe('GET /user/:id'); }); test('Sends two linked transactions (server & client) to Sentry', async ({ page }) => { diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/.gitignore b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/.gitignore new file mode 100644 index 000000000000..ebb991370034 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/.gitignore @@ -0,0 +1,32 @@ +# 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 + +# react router +.react-router diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/.npmrc b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/.npmrc new file mode 100644 index 000000000000..070f80f05092 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/.npmrc @@ -0,0 +1,2 @@ +@sentry:registry=http://127.0.0.1:4873 +@sentry-internal:registry=http://127.0.0.1:4873 diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/app.css b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/app.css new file mode 100644 index 000000000000..e78d2096ad20 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/app.css @@ -0,0 +1,5 @@ +body { + font-family: system-ui, sans-serif; + margin: 0; + padding: 20px; +} diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/entry.client.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/entry.client.tsx new file mode 100644 index 000000000000..c8bd9df2ba99 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/entry.client.tsx @@ -0,0 +1,33 @@ +import * as Sentry from '@sentry/react-router'; +import { StrictMode, startTransition } from 'react'; +import { hydrateRoot } from 'react-dom/client'; +import { HydratedRouter } from 'react-router/dom'; + +// Create the tracing integration with useInstrumentationAPI enabled +// This must be set BEFORE Sentry.init() to prepare the instrumentation +const tracing = Sentry.reactRouterTracingIntegration({ useInstrumentationAPI: true }); + +Sentry.init({ + environment: 'qa', // dynamic sampling bias to keep transactions + dsn: 'https://username@domain/123', + tunnel: `http://localhost:3031/`, // proxy server + integrations: [tracing], + tracesSampleRate: 1.0, + tracePropagationTargets: [/^\//], +}); + +// Get the client instrumentation from the Sentry integration +// NOTE: As of React Router 7.x, HydratedRouter does NOT invoke these hooks in Framework Mode. +// The client-side instrumentation is prepared for when React Router adds support. +// Client-side navigation is currently handled by the legacy instrumentHydratedRouter() approach. +const sentryClientInstrumentation = [tracing.clientInstrumentation]; + +startTransition(() => { + hydrateRoot( + document, + + {/* unstable_instrumentations is React Router 7.x's prop name (will become `instrumentations` in v8) */} + + , + ); +}); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/entry.server.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/entry.server.tsx new file mode 100644 index 000000000000..1cbc6b6166fe --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/entry.server.tsx @@ -0,0 +1,22 @@ +import { createReadableStreamFromReadable } from '@react-router/node'; +import * as Sentry from '@sentry/react-router'; +import { renderToPipeableStream } from 'react-dom/server'; +import { ServerRouter } from 'react-router'; +import { type HandleErrorFunction } from 'react-router'; + +const ABORT_DELAY = 5_000; + +const handleRequest = Sentry.createSentryHandleRequest({ + streamTimeout: ABORT_DELAY, + ServerRouter, + renderToPipeableStream, + createReadableStreamFromReadable, +}); + +export default handleRequest; + +export const handleError: HandleErrorFunction = Sentry.createSentryHandleError({ logErrors: true }); + +// Use Sentry's instrumentation API for server-side tracing +// `unstable_instrumentations` is React Router 7.x's export name (will become `instrumentations` in v8) +export const unstable_instrumentations = [Sentry.createSentryServerInstrumentation()]; diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/root.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/root.tsx new file mode 100644 index 000000000000..227c08f7730c --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/root.tsx @@ -0,0 +1,69 @@ +import * as Sentry from '@sentry/react-router'; +import { Links, Meta, Outlet, Scripts, ScrollRestoration, isRouteErrorResponse } from 'react-router'; +import type { Route } from './+types/root'; +import stylesheet from './app.css?url'; + +export const links: Route.LinksFunction = () => [ + { rel: 'preconnect', href: 'https://fonts.googleapis.com' }, + { + rel: 'preconnect', + href: 'https://fonts.gstatic.com', + crossOrigin: 'anonymous', + }, + { + rel: 'stylesheet', + href: 'https://fonts.googleapis.com/css2?family=Inter:ital,opsz,wght@0,14..32,100..900;1,14..32,100..900&display=swap', + }, + { rel: 'stylesheet', href: stylesheet }, +]; + +export function Layout({ children }: { children: React.ReactNode }) { + return ( + + + + + + + + + {children} + + + + + ); +} + +export default function App() { + return ; +} + +export function ErrorBoundary({ error }: Route.ErrorBoundaryProps) { + let message = 'Oops!'; + let details = 'An unexpected error occurred.'; + let stack: string | undefined; + + if (isRouteErrorResponse(error)) { + message = error.status === 404 ? '404' : 'Error'; + details = error.status === 404 ? 'The requested page could not be found.' : error.statusText || details; + } else if (error && error instanceof Error) { + Sentry.captureException(error); + if (import.meta.env.DEV) { + details = error.message; + stack = error.stack; + } + } + + return ( +
+

{message}

+

{details}

+ {stack && ( + 
+          {stack}
+
+ )} +
+ ); +} diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes.ts new file mode 100644 index 000000000000..af261d6db1eb --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes.ts @@ -0,0 +1,16 @@ +import { type RouteConfig, index, prefix, route } from '@react-router/dev/routes'; + +export default [ + index('routes/home.tsx'), + ...prefix('performance', [ + index('routes/performance/index.tsx'), + route('ssr', 'routes/performance/ssr.tsx'), + route('with/:param', 'routes/performance/dynamic-param.tsx'), + route('static', 'routes/performance/static.tsx'), + route('server-loader', 'routes/performance/server-loader.tsx'), + route('server-action', 'routes/performance/server-action.tsx'), + route('with-middleware', 'routes/performance/with-middleware.tsx'), + route('error-loader', 'routes/performance/error-loader.tsx'), + route('lazy-route', 'routes/performance/lazy-route.tsx'), + ]), +] satisfies RouteConfig; diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/home.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/home.tsx new file mode 100644 index 000000000000..d061ad54030b --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/home.tsx @@ -0,0 +1,12 @@ +import type { Route } from './+types/home'; + +export function meta({}: Route.MetaArgs) { + return [ + { title: 'React Router Instrumentation API Test' }, + { name: 'description', content: 'Testing React Router instrumentation API' }, + ]; +} + +export default function Home() { + return
home
; +} diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/dynamic-param.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/dynamic-param.tsx new file mode 100644 index 000000000000..bff0410f849c --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/dynamic-param.tsx @@ -0,0 +1,16 @@ +import type { Route } from './+types/dynamic-param'; + +// Minimal loader to trigger Sentry's route instrumentation +// eslint-disable-next-line @typescript-eslint/no-unused-vars +export function loader() { + return null; +} + +export default function DynamicParamPage({ params }: Route.ComponentProps) { + return ( +
+

Dynamic Param Page

+
Param: {params.param}
+
+ ); +} diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/error-loader.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/error-loader.tsx new file mode 100644 index 000000000000..6dd3d3013f37 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/error-loader.tsx @@ -0,0 +1,12 @@ +export function loader(): never { + throw new Error('Loader error for testing'); +} + +export default function ErrorLoaderPage() { + return ( +
+

Error Loader Page

+

This should not render

+
+ ); +} diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/index.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/index.tsx new file mode 100644 index 000000000000..901de267cb3e --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/index.tsx @@ -0,0 +1,20 @@ +import { Link } from 'react-router'; + +// Minimal loader to trigger Sentry's route instrumentation +// eslint-disable-next-line @typescript-eslint/no-unused-vars +export function loader() { + return null; +} + +export default function PerformancePage() { + return ( +
+

Performance Page

+ +
+ ); +} diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/lazy-route.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/lazy-route.tsx new file mode 100644 index 000000000000..9ea3102f6e3f --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/lazy-route.tsx @@ -0,0 +1,14 @@ +export async function loader() { + // Simulate a slow lazy load + await new Promise(resolve => setTimeout(resolve, 100)); + return { message: 'Lazy loader data' }; +} + +export default function LazyRoute() { + return ( +
+

Lazy Route

+

This route was lazily loaded

+
+ ); +} diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/server-action.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/server-action.tsx new file mode 100644 index 000000000000..4b5ad7a4f5ac --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/server-action.tsx @@ -0,0 +1,22 @@ +import { Form } from 'react-router'; +import type { Route } from './+types/server-action'; + +export async function action({ request }: Route.ActionArgs) { + const formData = await request.formData(); + const name = formData.get('name')?.toString() || ''; + await new Promise(resolve => setTimeout(resolve, 100)); + return { success: true, name }; +} + +export default function ServerActionPage({ actionData }: Route.ComponentProps) { + return ( +
+

Server Action Page

+
+ + +
+ {actionData?.success &&
Action completed for: {actionData.name}
} +
+ ); +} diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/server-loader.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/server-loader.tsx new file mode 100644 index 000000000000..3ab65bff8ecf --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/server-loader.tsx @@ -0,0 +1,16 @@ +import type { Route } from './+types/server-loader'; + +export async function loader() { + await new Promise(resolve => setTimeout(resolve, 100)); + return { data: 'burritos' }; +} + +export default function ServerLoaderPage({ loaderData }: Route.ComponentProps) { + const { data } = loaderData; + return ( +
+

Server Loader Page

+
{data}
+
+ ); +} diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/ssr.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/ssr.tsx new file mode 100644 index 000000000000..253e964ff15d --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/ssr.tsx @@ -0,0 +1,7 @@ +export default function SsrPage() { + return ( +
+

SSR Page

+
+ ); +} diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/static.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/static.tsx new file mode 100644 index 000000000000..773f6e64ebea --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/static.tsx @@ -0,0 +1,7 @@ +export default function StaticPage() { + return ( +
+

Static Page

+
+ ); +} diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/with-middleware.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/with-middleware.tsx new file mode 100644 index 000000000000..ed4f4713d7b6 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/app/routes/performance/with-middleware.tsx @@ -0,0 +1,30 @@ +import type { Route } from './+types/with-middleware'; + +// Middleware runs before loaders/actions on matching routes +// With future.v8_middleware enabled, we export 'middleware' (not 'unstable_middleware') +export const middleware: Route.MiddlewareFunction[] = [ + async function authMiddleware({ context }, next) { + // Code runs BEFORE handlers + // Type assertion to allow setting custom properties on context + (context as any).middlewareCalled = true; + + // Must call next() and return the response + const response = await next(); + + // Code runs AFTER handlers (can modify response headers here) + return response; + }, +]; + +export function loader() { + return { message: 'Middleware route loaded' }; +} + +export default function WithMiddlewarePage() { + return ( +
+

Middleware Route

+

This route has middleware

+
+ ); +} diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/instrument.mjs b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/instrument.mjs new file mode 100644 index 000000000000..bb1dad2e5da9 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/instrument.mjs @@ -0,0 +1,10 @@ +import * as Sentry from '@sentry/react-router'; + +// Initialize Sentry early (before the server starts) +// The server instrumentations are created in entry.server.tsx +Sentry.init({ + dsn: 'https://username@domain/123', + environment: 'qa', // dynamic sampling bias to keep transactions + tracesSampleRate: 1.0, + tunnel: `http://localhost:3031/`, // proxy server +}); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/package.json b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/package.json new file mode 100644 index 000000000000..9666bf218893 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/package.json @@ -0,0 +1,61 @@ +{ + "name": "react-router-7-framework-instrumentation", + "version": "0.1.0", + "type": "module", + "private": true, + "dependencies": { + "@react-router/node": "latest", + "@react-router/serve": "latest", + "@sentry/react-router": "latest || *", + "isbot": "^5.1.17", + "react": "^18.3.1", + "react-dom": "^18.3.1", + "react-router": "latest" + }, + "devDependencies": { + "@playwright/test": "~1.56.0", + "@react-router/dev": "latest", + "@sentry-internal/test-utils": "link:../../../test-utils", + "@types/node": "^20", + "@types/react": "18.3.1", + "@types/react-dom": "18.3.1", + "typescript": "^5.6.3", + "vite": "^5.4.11" + }, + "scripts": { + "build": "react-router build", + "dev": "NODE_OPTIONS='--import ./instrument.mjs' react-router dev", + "start": "NODE_OPTIONS='--import ./instrument.mjs' react-router-serve ./build/server/index.js", + "proxy": "node start-event-proxy.mjs", + "typecheck": "react-router typegen && tsc", + "clean": "npx rimraf node_modules pnpm-lock.yaml", + "test:build": "pnpm install && pnpm build", + "test:assert": "pnpm test:ts && pnpm test:playwright", + "test:ts": "pnpm typecheck", + "test:playwright": "playwright 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" + ] + }, + "volta": { + "extends": "../../package.json" + }, + "sentryTest": { + "optional": true + } +} diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/playwright.config.mjs b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/playwright.config.mjs new file mode 100644 index 000000000000..3ed5721107a7 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/playwright.config.mjs @@ -0,0 +1,8 @@ +import { getPlaywrightConfig } from '@sentry-internal/test-utils'; + +const config = getPlaywrightConfig({ + startCommand: `PORT=3030 pnpm start`, + port: 3030, +}); + +export default config; diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/react-router.config.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/react-router.config.ts new file mode 100644 index 000000000000..72f2eef3b0f5 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/react-router.config.ts @@ -0,0 +1,9 @@ +import type { Config } from '@react-router/dev/config'; + +export default { + ssr: true, + prerender: ['/performance/static'], + future: { + v8_middleware: true, + }, +} satisfies Config; diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/start-event-proxy.mjs b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/start-event-proxy.mjs new file mode 100644 index 000000000000..f70c1d3f20f1 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/start-event-proxy.mjs @@ -0,0 +1,6 @@ +import { startEventProxyServer } from '@sentry-internal/test-utils'; + +startEventProxyServer({ + port: 3031, + proxyServerName: 'react-router-7-framework-instrumentation', +}); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/constants.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/constants.ts new file mode 100644 index 000000000000..850613659daa --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/constants.ts @@ -0,0 +1 @@ +export const APP_NAME = 'react-router-7-framework-instrumentation'; diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/errors/errors.server.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/errors/errors.server.test.ts new file mode 100644 index 000000000000..85d79563d637 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/errors/errors.server.test.ts @@ -0,0 +1,93 @@ +import { expect, test } from '@playwright/test'; +import { waitForError, waitForTransaction } from '@sentry-internal/test-utils'; +import { APP_NAME } from '../constants'; + +test.describe('server - instrumentation API error capture', () => { + test('should capture loader errors with instrumentation API mechanism', async ({ page }) => { + const errorPromise = waitForError(APP_NAME, async errorEvent => { + return errorEvent.exception?.values?.[0]?.value === 'Loader error for testing'; + }); + + const txPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === 'GET /performance/error-loader'; + }); + + await page.goto(`/performance/error-loader`).catch(() => { + // Expected to fail due to loader error + }); + + const [error, transaction] = await Promise.all([errorPromise, txPromise]); + + // Verify the error was captured with correct mechanism and transaction name + expect(error).toMatchObject({ + exception: { + values: [ + { + type: 'Error', + value: 'Loader error for testing', + mechanism: { + type: 'react_router.loader', + handled: false, + }, + }, + ], + }, + transaction: 'GET /performance/error-loader', + }); + + // Verify the transaction was also created with correct attributes + expect(transaction).toMatchObject({ + transaction: 'GET /performance/error-loader', + contexts: { + trace: { + op: 'http.server', + origin: 'auto.http.react_router.instrumentation_api', + }, + }, + }); + }); + + test('should include loader span in transaction even when loader throws', async ({ page }) => { + const txPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === 'GET /performance/error-loader'; + }); + + await page.goto(`/performance/error-loader`).catch(() => { + // Expected to fail due to loader error + }); + + const transaction = await txPromise; + + // Find the loader span + const loaderSpan = transaction?.spans?.find( + (span: { data?: { 'sentry.op'?: string } }) => span.data?.['sentry.op'] === 'function.react-router.loader', + ); + + expect(loaderSpan).toMatchObject({ + data: { + 'sentry.origin': 'auto.function.react_router.instrumentation_api', + 'sentry.op': 'function.react-router.loader', + }, + op: 'function.react-router.loader', + }); + }); + + test('error and transaction should share the same trace', async ({ page }) => { + const errorPromise = waitForError(APP_NAME, async errorEvent => { + return errorEvent.exception?.values?.[0]?.value === 'Loader error for testing'; + }); + + const txPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === 'GET /performance/error-loader'; + }); + + await page.goto(`/performance/error-loader`).catch(() => { + // Expected to fail due to loader error + }); + + const [error, transaction] = await Promise.all([errorPromise, txPromise]); + + // Error and transaction should have the same trace_id + expect(error.contexts?.trace?.trace_id).toBe(transaction.contexts?.trace?.trace_id); + }); +}); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/lazy.server.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/lazy.server.test.ts new file mode 100644 index 000000000000..33ff1021d0c8 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/lazy.server.test.ts @@ -0,0 +1,115 @@ +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/test-utils'; +import { APP_NAME } from '../constants'; + +// Known React Router limitation: route.lazy hooks only work in Data Mode (createBrowserRouter). +// Framework Mode uses bundler code-splitting which doesn't trigger the lazy hook. +// See: https://github.com/remix-run/react-router/blob/main/decisions/0002-lazy-route-modules.md +// Using test.fail() to auto-detect when React Router fixes this upstream. +test.describe('server - instrumentation API lazy loading', () => { + test.fail('should instrument lazy route loading with instrumentation API origin', async ({ page }) => { + const txPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === 'GET /performance/lazy-route'; + }); + + await page.goto(`/performance/lazy-route`); + + const transaction = await txPromise; + + // Verify the lazy route content is rendered + await expect(page.locator('#lazy-route-title')).toBeVisible(); + await expect(page.locator('#lazy-route-content')).toHaveText('This route was lazily loaded'); + + expect(transaction).toMatchObject({ + contexts: { + trace: { + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + 'sentry.op': 'http.server', + 'sentry.origin': 'auto.http.react_router.instrumentation_api', + 'sentry.source': 'route', + }, + op: 'http.server', + origin: 'auto.http.react_router.instrumentation_api', + }, + }, + spans: expect.any(Array), + transaction: 'GET /performance/lazy-route', + type: 'transaction', + transaction_info: { source: 'route' }, + }); + + // Find the lazy span + const lazySpan = transaction?.spans?.find( + (span: { data?: { 'sentry.op'?: string } }) => span.data?.['sentry.op'] === 'function.react-router.lazy', + ); + + expect(lazySpan).toMatchObject({ + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + 'sentry.origin': 'auto.function.react_router.instrumentation_api', + 'sentry.op': 'function.react-router.lazy', + }, + description: 'Lazy Route Load', + parent_span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + op: 'function.react-router.lazy', + origin: 'auto.function.react_router.instrumentation_api', + }); + }); + + test('should include loader span after lazy loading completes', async ({ page }) => { + const txPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === 'GET /performance/lazy-route'; + }); + + await page.goto(`/performance/lazy-route`); + + const transaction = await txPromise; + + // Find the loader span that runs after lazy loading + const loaderSpan = transaction?.spans?.find( + (span: { data?: { 'sentry.op'?: string } }) => span.data?.['sentry.op'] === 'function.react-router.loader', + ); + + expect(loaderSpan).toMatchObject({ + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + 'sentry.origin': 'auto.function.react_router.instrumentation_api', + 'sentry.op': 'function.react-router.loader', + }, + description: '/performance/lazy-route', + op: 'function.react-router.loader', + origin: 'auto.function.react_router.instrumentation_api', + }); + }); + + test.fail('should have correct span ordering: lazy before loader', async ({ page }) => { + const txPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === 'GET /performance/lazy-route'; + }); + + await page.goto(`/performance/lazy-route`); + + const transaction = await txPromise; + + const lazySpan = transaction?.spans?.find( + (span: { data?: { 'sentry.op'?: string } }) => span.data?.['sentry.op'] === 'function.react-router.lazy', + ); + + const loaderSpan = transaction?.spans?.find( + (span: { data?: { 'sentry.op'?: string } }) => span.data?.['sentry.op'] === 'function.react-router.loader', + ); + + expect(lazySpan).toBeDefined(); + expect(loaderSpan).toBeDefined(); + + // Lazy span should start before or at the same time as loader + // (lazy loading must complete before loader can run) + expect(lazySpan!.start_timestamp).toBeLessThanOrEqual(loaderSpan!.start_timestamp); + }); +}); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/middleware.server.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/middleware.server.test.ts new file mode 100644 index 000000000000..08ee2b9cda0c --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/middleware.server.test.ts @@ -0,0 +1,85 @@ +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/test-utils'; +import { APP_NAME } from '../constants'; + +// Note: React Router middleware instrumentation now works in Framework Mode. +// Previously this was a known limitation (see: https://github.com/remix-run/react-router/discussions/12950) +test.describe('server - instrumentation API middleware', () => { + test('should instrument server middleware with instrumentation API origin', async ({ page }) => { + const txPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === 'GET /performance/with-middleware'; + }); + + await page.goto(`/performance/with-middleware`); + + const transaction = await txPromise; + + // Verify the middleware route content is rendered + await expect(page.locator('#middleware-route-title')).toBeVisible(); + await expect(page.locator('#middleware-route-content')).toHaveText('This route has middleware'); + + expect(transaction).toMatchObject({ + contexts: { + trace: { + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + 'sentry.op': 'http.server', + 'sentry.origin': 'auto.http.react_router.instrumentation_api', + 'sentry.source': 'route', + }, + op: 'http.server', + origin: 'auto.http.react_router.instrumentation_api', + }, + }, + spans: expect.any(Array), + transaction: 'GET /performance/with-middleware', + type: 'transaction', + transaction_info: { source: 'route' }, + }); + + // Find the middleware span + const middlewareSpan = transaction?.spans?.find( + (span: { data?: { 'sentry.op'?: string } }) => span.data?.['sentry.op'] === 'function.react-router.middleware', + ); + + expect(middlewareSpan).toMatchObject({ + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + 'sentry.origin': 'auto.function.react_router.instrumentation_api', + 'sentry.op': 'function.react-router.middleware', + }, + description: '/performance/with-middleware', + parent_span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + op: 'function.react-router.middleware', + origin: 'auto.function.react_router.instrumentation_api', + }); + }); + + test('should have middleware span run before loader span', async ({ page }) => { + const txPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === 'GET /performance/with-middleware'; + }); + + await page.goto(`/performance/with-middleware`); + + const transaction = await txPromise; + + const middlewareSpan = transaction?.spans?.find( + (span: { data?: { 'sentry.op'?: string } }) => span.data?.['sentry.op'] === 'function.react-router.middleware', + ); + + const loaderSpan = transaction?.spans?.find( + (span: { data?: { 'sentry.op'?: string } }) => span.data?.['sentry.op'] === 'function.react-router.loader', + ); + + expect(middlewareSpan).toBeDefined(); + expect(loaderSpan).toBeDefined(); + + // Middleware should start before loader + expect(middlewareSpan!.start_timestamp).toBeLessThanOrEqual(loaderSpan!.start_timestamp); + }); +}); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/navigation.client.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/navigation.client.test.ts new file mode 100644 index 000000000000..b6a7fc8b20ac --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/navigation.client.test.ts @@ -0,0 +1,149 @@ +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/test-utils'; +import { APP_NAME } from '../constants'; + +// Known React Router limitation: HydratedRouter doesn't invoke instrumentation API +// hooks on the client-side in Framework Mode. Server-side instrumentation works. +// See: https://github.com/remix-run/react-router/discussions/13749 +// The legacy HydratedRouter instrumentation provides fallback navigation tracking. + +test.describe('client - navigation fallback to legacy instrumentation', () => { + test('should send navigation transaction via legacy HydratedRouter instrumentation', async ({ page }) => { + // First load the performance page + await page.goto(`/performance`); + await page.waitForTimeout(1000); + + // Wait for the navigation transaction (from legacy instrumentation) + const navigationTxPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return ( + transactionEvent.transaction === '/performance/ssr' && transactionEvent.contexts?.trace?.op === 'navigation' + ); + }); + + // Click on the SSR link to navigate + await page.getByRole('link', { name: 'SSR Page' }).click(); + + const transaction = await navigationTxPromise; + + // Navigation should work via legacy HydratedRouter instrumentation + // (not instrumentation_api since that doesn't work in Framework Mode) + expect(transaction).toMatchObject({ + contexts: { + trace: { + op: 'navigation', + origin: 'auto.navigation.react_router', // Legacy origin, not instrumentation_api + }, + }, + transaction: '/performance/ssr', + type: 'transaction', + }); + }); + + test('should parameterize navigation transaction for dynamic routes', async ({ page }) => { + await page.goto(`/performance`); + await page.waitForTimeout(1000); + + const navigationTxPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return ( + transactionEvent.transaction === '/performance/with/:param' && + transactionEvent.contexts?.trace?.op === 'navigation' + ); + }); + + await page.getByRole('link', { name: 'With Param Page' }).click(); + + const transaction = await navigationTxPromise; + + expect(transaction).toMatchObject({ + contexts: { + trace: { + op: 'navigation', + origin: 'auto.navigation.react_router', + data: { + 'sentry.source': 'route', + }, + }, + }, + transaction: '/performance/with/:param', + type: 'transaction', + transaction_info: { source: 'route' }, + }); + }); +}); + +// Tests for instrumentation API navigation - expected to fail until React Router fixes upstream +test.describe('client - instrumentation API navigation (upstream limitation)', () => { + test.fixme('should send navigation transaction with instrumentation API origin', async ({ page }) => { + // First load the performance page + await page.goto(`/performance`); + + // Wait for the navigation transaction + const navigationTxPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return ( + transactionEvent.transaction === '/performance/ssr' && + transactionEvent.contexts?.trace?.data?.['sentry.origin'] === 'auto.navigation.react_router.instrumentation_api' + ); + }); + + // Click on the SSR link to navigate + await page.getByRole('link', { name: 'SSR Page' }).click(); + + const transaction = await navigationTxPromise; + + expect(transaction).toMatchObject({ + contexts: { + trace: { + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + 'sentry.op': 'navigation', + 'sentry.origin': 'auto.navigation.react_router.instrumentation_api', + 'sentry.source': 'url', + }, + op: 'navigation', + origin: 'auto.navigation.react_router.instrumentation_api', + }, + }, + transaction: '/performance/ssr', + type: 'transaction', + transaction_info: { source: 'url' }, + }); + }); + + test.fixme('should send navigation transaction on parameterized route', async ({ page }) => { + // First load the performance page + await page.goto(`/performance`); + + // Wait for the navigation transaction + const navigationTxPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return ( + transactionEvent.transaction === '/performance/with/sentry' && + transactionEvent.contexts?.trace?.data?.['sentry.origin'] === 'auto.navigation.react_router.instrumentation_api' + ); + }); + + // Click on the With Param link to navigate + await page.getByRole('link', { name: 'With Param Page' }).click(); + + const transaction = await navigationTxPromise; + + expect(transaction).toMatchObject({ + contexts: { + trace: { + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + 'sentry.op': 'navigation', + 'sentry.origin': 'auto.navigation.react_router.instrumentation_api', + 'sentry.source': 'url', + }, + op: 'navigation', + origin: 'auto.navigation.react_router.instrumentation_api', + }, + }, + transaction: '/performance/with/sentry', + type: 'transaction', + transaction_info: { source: 'url' }, + }); + }); +}); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/pageload.client.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/pageload.client.test.ts new file mode 100644 index 000000000000..4c8c04e2cb66 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/pageload.client.test.ts @@ -0,0 +1,27 @@ +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/test-utils'; +import { APP_NAME } from '../constants'; + +test.describe('client - instrumentation API pageload', () => { + test('should send pageload transaction', async ({ page }) => { + const txPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === '/performance' && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + + await page.goto(`/performance`); + + const transaction = await txPromise; + + expect(transaction).toMatchObject({ + contexts: { + trace: { + span_id: expect.any(String), + trace_id: expect.any(String), + op: 'pageload', + }, + }, + transaction: '/performance', + type: 'transaction', + }); + }); +}); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/performance.server.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/performance.server.test.ts new file mode 100644 index 000000000000..c0803eed46d8 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/performance.server.test.ts @@ -0,0 +1,154 @@ +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/test-utils'; +import { APP_NAME } from '../constants'; + +test.describe('server - instrumentation API performance', () => { + test('should send server transaction on pageload with instrumentation API origin', async ({ page }) => { + const txPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === 'GET /performance'; + }); + + await page.goto(`/performance`); + + const transaction = await txPromise; + + expect(transaction).toMatchObject({ + contexts: { + trace: { + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + 'sentry.op': 'http.server', + 'sentry.origin': 'auto.http.react_router.instrumentation_api', + 'sentry.source': 'route', + }, + op: 'http.server', + origin: 'auto.http.react_router.instrumentation_api', + }, + }, + spans: expect.any(Array), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + transaction: 'GET /performance', + type: 'transaction', + transaction_info: { source: 'route' }, + platform: 'node', + request: { + url: expect.stringContaining('/performance'), + headers: expect.any(Object), + }, + event_id: expect.any(String), + environment: 'qa', + sdk: { + integrations: expect.arrayContaining([expect.any(String)]), + name: 'sentry.javascript.react-router', + version: expect.any(String), + packages: [ + { name: 'npm:@sentry/react-router', version: expect.any(String) }, + { name: 'npm:@sentry/node', version: expect.any(String) }, + ], + }, + tags: { + runtime: 'node', + }, + }); + }); + + test('should send server transaction on parameterized route with instrumentation API origin', async ({ page }) => { + const txPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === 'GET /performance/with/:param'; + }); + + await page.goto(`/performance/with/some-param`); + + const transaction = await txPromise; + + expect(transaction).toMatchObject({ + contexts: { + trace: { + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + 'sentry.op': 'http.server', + 'sentry.origin': 'auto.http.react_router.instrumentation_api', + 'sentry.source': 'route', + }, + op: 'http.server', + origin: 'auto.http.react_router.instrumentation_api', + }, + }, + spans: expect.any(Array), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + transaction: 'GET /performance/with/:param', + type: 'transaction', + transaction_info: { source: 'route' }, + platform: 'node', + request: { + url: expect.stringContaining('/performance/with/some-param'), + headers: expect.any(Object), + }, + event_id: expect.any(String), + environment: 'qa', + }); + }); + + test('should instrument server loader with instrumentation API origin', async ({ page }) => { + const txPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === 'GET /performance/server-loader'; + }); + + await page.goto(`/performance/server-loader`); + + const transaction = await txPromise; + + // Find the loader span + const loaderSpan = transaction?.spans?.find(span => span.data?.['sentry.op'] === 'function.react-router.loader'); + + expect(loaderSpan).toMatchObject({ + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + 'sentry.origin': 'auto.function.react_router.instrumentation_api', + 'sentry.op': 'function.react-router.loader', + }, + description: '/performance/server-loader', + parent_span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + status: 'ok', + op: 'function.react-router.loader', + origin: 'auto.function.react_router.instrumentation_api', + }); + }); + + test('should instrument server action with instrumentation API origin', async ({ page }) => { + const txPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === 'POST /performance/server-action'; + }); + + await page.goto(`/performance/server-action`); + await page.getByRole('button', { name: 'Submit' }).click(); + + const transaction = await txPromise; + + // Find the action span + const actionSpan = transaction?.spans?.find(span => span.data?.['sentry.op'] === 'function.react-router.action'); + + expect(actionSpan).toMatchObject({ + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + 'sentry.origin': 'auto.function.react_router.instrumentation_api', + 'sentry.op': 'function.react-router.action', + }, + description: '/performance/server-action', + parent_span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + status: 'ok', + op: 'function.react-router.action', + origin: 'auto.function.react_router.instrumentation_api', + }); + }); +}); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tsconfig.json b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tsconfig.json new file mode 100644 index 000000000000..a16df276e8bc --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tsconfig.json @@ -0,0 +1,20 @@ +{ + "compilerOptions": { + "lib": ["DOM", "DOM.Iterable", "ES2022"], + "types": ["node", "vite/client"], + "target": "ES2022", + "module": "ES2022", + "moduleResolution": "bundler", + "jsx": "react-jsx", + "rootDirs": [".", "./.react-router/types"], + "baseUrl": ".", + + "esModuleInterop": true, + "verbatimModuleSyntax": true, + "noEmit": true, + "resolveJsonModule": true, + "skipLibCheck": true, + "strict": true + }, + "include": ["**/*", "**/.server/**/*", "**/.client/**/*", ".react-router/types/**/*"] +} diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/vite.config.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/vite.config.ts new file mode 100644 index 000000000000..68ba30d69397 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/vite.config.ts @@ -0,0 +1,6 @@ +import { reactRouter } from '@react-router/dev/vite'; +import { defineConfig } from 'vite'; + +export default defineConfig({ + plugins: [reactRouter()], +}); diff --git a/packages/react-router/src/client/createClientInstrumentation.ts b/packages/react-router/src/client/createClientInstrumentation.ts new file mode 100644 index 000000000000..1c3a32ffbab9 --- /dev/null +++ b/packages/react-router/src/client/createClientInstrumentation.ts @@ -0,0 +1,201 @@ +import { startBrowserTracingNavigationSpan } from '@sentry/browser'; +import { + debug, + getClient, + GLOBAL_OBJ, + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, + startSpan, +} from '@sentry/core'; +import { DEBUG_BUILD } from '../common/debug-build'; +import type { ClientInstrumentation, InstrumentableRoute, InstrumentableRouter } from '../common/types'; +import { captureInstrumentationError, getPathFromRequest, getPattern, normalizeRoutePath } from '../common/utils'; + +const SENTRY_CLIENT_INSTRUMENTATION_FLAG = '__sentryReactRouterClientInstrumentationUsed'; +const SENTRY_NAVIGATE_HOOK_INVOKED_FLAG = '__sentryReactRouterNavigateHookInvoked'; + +type GlobalObjWithFlags = typeof GLOBAL_OBJ & { + [SENTRY_CLIENT_INSTRUMENTATION_FLAG]?: boolean; + [SENTRY_NAVIGATE_HOOK_INVOKED_FLAG]?: boolean; +}; + +/** + * Options for creating Sentry client instrumentation. + */ +export interface CreateSentryClientInstrumentationOptions { + /** + * Whether to capture errors from loaders/actions automatically. + * @default true + */ + captureErrors?: boolean; +} + +/** + * Creates a Sentry client instrumentation for React Router's instrumentation API. + * @experimental + */ +export function createSentryClientInstrumentation( + options: CreateSentryClientInstrumentationOptions = {}, +): ClientInstrumentation { + const { captureErrors = true } = options; + + (GLOBAL_OBJ as GlobalObjWithFlags)[SENTRY_CLIENT_INSTRUMENTATION_FLAG] = true; + DEBUG_BUILD && debug.log('React Router client instrumentation API enabled.'); + + return { + router(router: InstrumentableRouter) { + router.instrument({ + async navigate(callNavigate, info) { + (GLOBAL_OBJ as GlobalObjWithFlags)[SENTRY_NAVIGATE_HOOK_INVOKED_FLAG] = true; + + // Skip numeric navigations (history back/forward like navigate(-1)) + // since we can't resolve them to meaningful route names + if (typeof info.to === 'number') { + const result = await callNavigate(); + captureInstrumentationError(result, captureErrors, 'react_router.navigate', { + 'http.url': info.currentUrl, + }); + return; + } + + const client = getClient(); + const toPath = String(info.to); + + if (client) { + startBrowserTracingNavigationSpan(client, { + name: toPath, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react_router.instrumentation_api', + }, + }); + } + + const result = await callNavigate(); + captureInstrumentationError(result, captureErrors, 'react_router.navigate', { + 'http.url': toPath, + }); + }, + + async fetch(callFetch, info) { + await startSpan( + { + name: `Fetcher ${info.fetcherKey}`, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.react-router.fetcher', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.react_router.instrumentation_api', + }, + }, + async () => { + const result = await callFetch(); + captureInstrumentationError(result, captureErrors, 'react_router.fetcher', { + 'http.url': info.href, + }); + }, + ); + }, + }); + }, + + route(route: InstrumentableRoute) { + route.instrument({ + async loader(callLoader, info) { + const urlPath = getPathFromRequest(info.request); + const routePattern = normalizeRoutePath(getPattern(info)) || urlPath; + + await startSpan( + { + name: routePattern, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.react-router.client-loader', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.react_router.instrumentation_api', + }, + }, + async () => { + const result = await callLoader(); + captureInstrumentationError(result, captureErrors, 'react_router.client_loader', { + 'http.url': urlPath, + }); + }, + ); + }, + + async action(callAction, info) { + const urlPath = getPathFromRequest(info.request); + const routePattern = normalizeRoutePath(getPattern(info)) || urlPath; + + await startSpan( + { + name: routePattern, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.react-router.client-action', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.react_router.instrumentation_api', + }, + }, + async () => { + const result = await callAction(); + captureInstrumentationError(result, captureErrors, 'react_router.client_action', { + 'http.url': urlPath, + }); + }, + ); + }, + + async middleware(callMiddleware, info) { + const urlPath = getPathFromRequest(info.request); + const routePattern = normalizeRoutePath(getPattern(info)) || urlPath; + + await startSpan( + { + name: routePattern, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.react-router.client-middleware', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.react_router.instrumentation_api', + }, + }, + async () => { + const result = await callMiddleware(); + captureInstrumentationError(result, captureErrors, 'react_router.client_middleware', { + 'http.url': urlPath, + }); + }, + ); + }, + + async lazy(callLazy) { + await startSpan( + { + name: 'Lazy Route Load', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.react-router.client-lazy', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.react_router.instrumentation_api', + }, + }, + async () => { + const result = await callLazy(); + captureInstrumentationError(result, captureErrors, 'react_router.client_lazy', {}); + }, + ); + }, + }); + }, + }; +} + +/** + * Check if React Router's instrumentation API is being used on the client. + * @experimental + */ +export function isClientInstrumentationApiUsed(): boolean { + return !!(GLOBAL_OBJ as GlobalObjWithFlags)[SENTRY_CLIENT_INSTRUMENTATION_FLAG]; +} + +/** + * Check if React Router's instrumentation API's navigate hook was invoked. + * @experimental + */ +export function isNavigateHookInvoked(): boolean { + return !!(GLOBAL_OBJ as GlobalObjWithFlags)[SENTRY_NAVIGATE_HOOK_INVOKED_FLAG]; +} diff --git a/packages/react-router/src/client/hydratedRouter.ts b/packages/react-router/src/client/hydratedRouter.ts index 14cdf07a33c9..49655d4a3b8d 100644 --- a/packages/react-router/src/client/hydratedRouter.ts +++ b/packages/react-router/src/client/hydratedRouter.ts @@ -1,7 +1,7 @@ import { startBrowserTracingNavigationSpan } from '@sentry/browser'; import type { Span } from '@sentry/core'; import { - consoleSandbox, + debug, getActiveSpan, getClient, getRootSpan, @@ -13,6 +13,7 @@ import { } from '@sentry/core'; import type { DataRouter, RouterState } from 'react-router'; import { DEBUG_BUILD } from '../common/debug-build'; +import { isNavigateHookInvoked } from './createClientInstrumentation'; const GLOBAL_OBJ_WITH_DATA_ROUTER = GLOBAL_OBJ as typeof GLOBAL_OBJ & { __reactRouterDataRouter?: DataRouter; @@ -34,7 +35,6 @@ export function instrumentHydratedRouter(): void { if (router) { // The first time we hit the router, we try to update the pageload transaction - // todo: update pageload tx here const pageloadSpan = getActiveRootSpan(); if (pageloadSpan) { @@ -51,18 +51,18 @@ export function instrumentHydratedRouter(): void { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.react_router', }); } + } - // Patching navigate for creating accurate navigation transactions - if (typeof router.navigate === 'function') { - const originalNav = router.navigate.bind(router); - router.navigate = function sentryPatchedNavigate(...args) { - maybeCreateNavigationTransaction( - String(args[0]) || '', // will be updated anyway - 'url', // this also will be updated once we have the parameterized route - ); - return originalNav(...args); - }; - } + // Patching navigate for creating accurate navigation transactions + if (typeof router.navigate === 'function') { + const originalNav = router.navigate.bind(router); + router.navigate = function sentryPatchedNavigate(...args) { + // Skip if instrumentation API is handling navigation (prevents double-counting) + if (!isNavigateHookInvoked()) { + maybeCreateNavigationTransaction(String(args[0]) || '', 'url'); + } + return originalNav(...args); + }; } // Subscribe to router state changes to update navigation transactions with parameterized routes @@ -79,7 +79,8 @@ export function instrumentHydratedRouter(): void { if ( navigationSpanName && newState.navigation.state === 'idle' && // navigation has completed - normalizePathname(newState.location.pathname) === normalizePathname(navigationSpanName) // this event is for the currently active navigation + // this event is for the currently active navigation + normalizePathname(newState.location.pathname) === normalizePathname(navigationSpanName) ) { navigationSpan.updateName(parameterizedNavRoute); navigationSpan.setAttributes({ @@ -100,11 +101,7 @@ export function instrumentHydratedRouter(): void { const interval = setInterval(() => { if (trySubscribe() || retryCount >= MAX_RETRIES) { if (retryCount >= MAX_RETRIES) { - DEBUG_BUILD && - consoleSandbox(() => { - // eslint-disable-next-line no-console - console.warn('Unable to instrument React Router: router not found after hydration.'); - }); + DEBUG_BUILD && debug.warn('Unable to instrument React Router: router not found after hydration.'); } clearInterval(interval); } diff --git a/packages/react-router/src/client/index.ts b/packages/react-router/src/client/index.ts index ba5c1c1264cb..6734b21c8583 100644 --- a/packages/react-router/src/client/index.ts +++ b/packages/react-router/src/client/index.ts @@ -4,7 +4,11 @@ export * from '@sentry/browser'; export { init } from './sdk'; -export { reactRouterTracingIntegration } from './tracingIntegration'; +export { + reactRouterTracingIntegration, + type ReactRouterTracingIntegration, + type ReactRouterTracingIntegrationOptions, +} from './tracingIntegration'; export { captureReactException, reactErrorHandler, Profiler, withProfiler, useProfiler } from '@sentry/react'; @@ -19,3 +23,11 @@ export { ErrorBoundary, withErrorBoundary } from '@sentry/react'; * See https://docs.sentry.io/platforms/javascript/guides/react-router/#report-errors-from-error-boundaries */ export type { ErrorBoundaryProps, FallbackRender } from '@sentry/react'; + +// React Router instrumentation API for use with unstable_instrumentations (React Router 7.x) +export { + createSentryClientInstrumentation, + isClientInstrumentationApiUsed, + isNavigateHookInvoked, + type CreateSentryClientInstrumentationOptions, +} from './createClientInstrumentation'; diff --git a/packages/react-router/src/client/tracingIntegration.ts b/packages/react-router/src/client/tracingIntegration.ts index 01b71f36d92a..a711eb986508 100644 --- a/packages/react-router/src/client/tracingIntegration.ts +++ b/packages/react-router/src/client/tracingIntegration.ts @@ -1,17 +1,68 @@ import { browserTracingIntegration as originalBrowserTracingIntegration } from '@sentry/browser'; import type { Integration } from '@sentry/core'; +import type { ClientInstrumentation } from '../common/types'; +import { + createSentryClientInstrumentation, + type CreateSentryClientInstrumentationOptions, +} from './createClientInstrumentation'; import { instrumentHydratedRouter } from './hydratedRouter'; +/** + * Options for the React Router tracing integration. + */ +export interface ReactRouterTracingIntegrationOptions { + /** + * Options for React Router's instrumentation API. + * @experimental + */ + instrumentationOptions?: CreateSentryClientInstrumentationOptions; + + /** + * Enable React Router's instrumentation API. + * When true, prepares for use with HydratedRouter's `unstable_instrumentations` prop. + * @experimental + * @default false + */ + useInstrumentationAPI?: boolean; +} + +/** + * React Router tracing integration with support for the instrumentation API. + */ +export interface ReactRouterTracingIntegration extends Integration { + /** + * Client instrumentation for React Router's instrumentation API. + * Lazily initialized on first access. + * @experimental HydratedRouter doesn't invoke these hooks in Framework Mode yet. + */ + readonly clientInstrumentation: ClientInstrumentation; +} + /** * Browser tracing integration for React Router (Framework) applications. - * This integration will create navigation spans and enhance transactions names with parameterized routes. + * This integration will create navigation spans and enhance transaction names with parameterized routes. */ -export function reactRouterTracingIntegration(): Integration { +export function reactRouterTracingIntegration( + options: ReactRouterTracingIntegrationOptions = {}, +): ReactRouterTracingIntegration { const browserTracingIntegrationInstance = originalBrowserTracingIntegration({ // Navigation transactions are started within the hydrated router instrumentation instrumentNavigation: false, }); + let clientInstrumentationInstance: ClientInstrumentation | undefined; + + if (options.useInstrumentationAPI || options.instrumentationOptions) { + clientInstrumentationInstance = createSentryClientInstrumentation(options.instrumentationOptions); + } + + const getClientInstrumentation = (): ClientInstrumentation => { + if (!clientInstrumentationInstance) { + clientInstrumentationInstance = createSentryClientInstrumentation(options.instrumentationOptions); + } + return clientInstrumentationInstance; + }; + return { ...browserTracingIntegrationInstance, name: 'ReactRouterTracingIntegration', @@ -19,5 +70,8 @@ export function reactRouterTracingIntegration(): Integration { browserTracingIntegrationInstance.afterAllSetup(client); instrumentHydratedRouter(); }, + get clientInstrumentation(): ClientInstrumentation { + return getClientInstrumentation(); + }, }; } diff --git a/packages/react-router/src/common/types.ts b/packages/react-router/src/common/types.ts new file mode 100644 index 000000000000..23cbb174f167 --- /dev/null +++ b/packages/react-router/src/common/types.ts @@ -0,0 +1,96 @@ +/** + * Types for React Router's instrumentation API. + * + * Derived from React Router v7.x `unstable_instrumentations` API. + * The stable `instrumentations` API is planned for React Router v8. + * If React Router changes these types, this file must be updated. + * + * @see https://reactrouter.com/how-to/instrumentation + * @experimental + */ + +export type InstrumentationResult = { status: 'success'; error: undefined } | { status: 'error'; error: unknown }; + +export interface ReadonlyRequest { + method: string; + url: string; + headers: Pick; +} + +export interface RouteHandlerInstrumentationInfo { + readonly request: ReadonlyRequest; + readonly params: Record; + readonly pattern?: string; + readonly unstable_pattern?: string; + readonly context?: unknown; +} + +export interface RouterNavigationInstrumentationInfo { + readonly to: string | number; + readonly currentUrl: string; + readonly formMethod?: string; + readonly formEncType?: string; + readonly formData?: FormData; + readonly body?: unknown; +} + +export interface RouterFetchInstrumentationInfo { + readonly href: string; + readonly currentUrl: string; + readonly fetcherKey: string; + readonly formMethod?: string; + readonly formEncType?: string; + readonly formData?: FormData; + readonly body?: unknown; +} + +export interface RequestHandlerInstrumentationInfo { + readonly request: Request; + readonly context: unknown; +} + +export type InstrumentFunction = (handler: () => Promise, info: T) => Promise; + +export interface RouteInstrumentations { + lazy?: InstrumentFunction; + 'lazy.loader'?: InstrumentFunction; + 'lazy.action'?: InstrumentFunction; + 'lazy.middleware'?: InstrumentFunction; + middleware?: InstrumentFunction; + loader?: InstrumentFunction; + action?: InstrumentFunction; +} + +export interface RouterInstrumentations { + navigate?: InstrumentFunction; + fetch?: InstrumentFunction; +} + +export interface RequestHandlerInstrumentations { + request?: InstrumentFunction; +} + +export interface InstrumentableRoute { + id: string; + index: boolean | undefined; + path: string | undefined; + instrument(instrumentations: RouteInstrumentations): void; +} + +export interface InstrumentableRouter { + instrument(instrumentations: RouterInstrumentations): void; +} + +export interface InstrumentableRequestHandler { + instrument(instrumentations: RequestHandlerInstrumentations): void; +} + +export interface ClientInstrumentation { + router?(router: InstrumentableRouter): void; + route?(route: InstrumentableRoute): void; +} + +export interface ServerInstrumentation { + handler?(handler: InstrumentableRequestHandler): void; + route?(route: InstrumentableRoute): void; +} diff --git a/packages/react-router/src/common/utils.ts b/packages/react-router/src/common/utils.ts new file mode 100644 index 000000000000..36d1c4568f6c --- /dev/null +++ b/packages/react-router/src/common/utils.ts @@ -0,0 +1,72 @@ +import { captureException, debug } from '@sentry/core'; +import { DEBUG_BUILD } from './debug-build'; +import type { InstrumentationResult } from './types'; + +/** + * Extracts pathname from request URL. + * Falls back to '' with DEBUG warning if URL cannot be parsed. + */ +export function getPathFromRequest(request: { url: string }): string { + try { + return new URL(request.url).pathname; + } catch { + try { + // Fallback: use a dummy base URL since we only care about the pathname + return new URL(request.url, 'http://example.com').pathname; + } catch (error) { + DEBUG_BUILD && debug.warn('Failed to parse URL from request:', request.url, error); + return ''; + } + } +} + +/** + * Extracts route pattern from instrumentation info. + * Prefers `pattern` (planned for v8) over `unstable_pattern` (v7.x). + */ +export function getPattern(info: { pattern?: string; unstable_pattern?: string }): string | undefined { + return info.pattern ?? info.unstable_pattern; +} + +/** + * Normalizes route path by ensuring it starts with a slash. + * Returns undefined if the input is falsy. + */ +export function normalizeRoutePath(pattern?: string): string | undefined { + if (!pattern) { + return undefined; + } + return pattern.startsWith('/') ? pattern : `/${pattern}`; +} + +/** + * Captures an error from instrumentation result if conditions are met. + * Used by both client and server instrumentation to avoid duplication. + * + * Only captures actual Error instances - Response objects and ErrorResponse + * are expected control flow in React Router (redirects, 404s, etc). + */ +export function captureInstrumentationError( + result: InstrumentationResult, + captureErrors: boolean, + mechanismType: string, + data: Record, +): void { + if (result.status === 'error' && captureErrors && isError(result.error)) { + captureException(result.error, { + mechanism: { + type: mechanismType, + handled: false, + }, + data, + }); + } +} + +/** + * Checks if value is an Error instance. + * Response objects and ErrorResponse are not errors - they're expected control flow. + */ +function isError(value: unknown): value is Error { + return value instanceof Error; +} diff --git a/packages/react-router/src/server/createServerInstrumentation.ts b/packages/react-router/src/server/createServerInstrumentation.ts new file mode 100644 index 000000000000..bbc17bdc14a4 --- /dev/null +++ b/packages/react-router/src/server/createServerInstrumentation.ts @@ -0,0 +1,229 @@ +import { context } from '@opentelemetry/api'; +import { getRPCMetadata, RPCType } from '@opentelemetry/core'; +import { ATTR_HTTP_ROUTE } from '@opentelemetry/semantic-conventions'; +import { + debug, + flushIfServerless, + getActiveSpan, + getCurrentScope, + getRootSpan, + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, + startSpan, + updateSpanName, +} from '@sentry/core'; +import { DEBUG_BUILD } from '../common/debug-build'; +import type { InstrumentableRequestHandler, InstrumentableRoute, ServerInstrumentation } from '../common/types'; +import { captureInstrumentationError, getPathFromRequest, getPattern, normalizeRoutePath } from '../common/utils'; +import { markInstrumentationApiUsed } from './serverGlobals'; + +// Re-export for backward compatibility and external use +export { isInstrumentationApiUsed } from './serverGlobals'; + +/** + * Options for creating Sentry server instrumentation. + */ +export interface CreateSentryServerInstrumentationOptions { + /** + * Whether to capture errors from loaders/actions automatically. + * @default true + */ + captureErrors?: boolean; +} + +/** + * Creates a Sentry server instrumentation for React Router's instrumentation API. + * @experimental + */ +export function createSentryServerInstrumentation( + options: CreateSentryServerInstrumentationOptions = {}, +): ServerInstrumentation { + const { captureErrors = true } = options; + + markInstrumentationApiUsed(); + DEBUG_BUILD && debug.log('React Router server instrumentation API enabled.'); + + return { + handler(handler: InstrumentableRequestHandler) { + handler.instrument({ + async request(handleRequest, info) { + const pathname = getPathFromRequest(info.request); + const activeSpan = getActiveSpan(); + const existingRootSpan = activeSpan ? getRootSpan(activeSpan) : undefined; + + if (existingRootSpan) { + updateSpanName(existingRootSpan, `${info.request.method} ${pathname}`); + existingRootSpan.setAttributes({ + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.react_router.instrumentation_api', + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + }); + + try { + const result = await handleRequest(); + captureInstrumentationError(result, captureErrors, 'react_router.request_handler', { + 'http.method': info.request.method, + 'http.url': pathname, + }); + } finally { + await flushIfServerless(); + } + } else { + await startSpan( + { + name: `${info.request.method} ${pathname}`, + forceTransaction: true, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.react_router.instrumentation_api', + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + 'http.request.method': info.request.method, + 'url.path': pathname, + 'url.full': info.request.url, + }, + }, + async () => { + try { + const result = await handleRequest(); + captureInstrumentationError(result, captureErrors, 'react_router.request_handler', { + 'http.method': info.request.method, + 'http.url': pathname, + }); + } finally { + await flushIfServerless(); + } + }, + ); + } + }, + }); + }, + + route(route: InstrumentableRoute) { + route.instrument({ + async loader(callLoader, info) { + const urlPath = getPathFromRequest(info.request); + const pattern = getPattern(info); + const routePattern = normalizeRoutePath(pattern) || urlPath; + updateRootSpanWithRoute(info.request.method, pattern, urlPath); + + await startSpan( + { + name: routePattern, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.react-router.loader', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.react_router.instrumentation_api', + }, + }, + async () => { + const result = await callLoader(); + captureInstrumentationError(result, captureErrors, 'react_router.loader', { + 'http.method': info.request.method, + 'http.url': urlPath, + }); + }, + ); + }, + + async action(callAction, info) { + const urlPath = getPathFromRequest(info.request); + const pattern = getPattern(info); + const routePattern = normalizeRoutePath(pattern) || urlPath; + updateRootSpanWithRoute(info.request.method, pattern, urlPath); + + await startSpan( + { + name: routePattern, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.react-router.action', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.react_router.instrumentation_api', + }, + }, + async () => { + const result = await callAction(); + captureInstrumentationError(result, captureErrors, 'react_router.action', { + 'http.method': info.request.method, + 'http.url': urlPath, + }); + }, + ); + }, + + async middleware(callMiddleware, info) { + const urlPath = getPathFromRequest(info.request); + const pattern = getPattern(info); + const routePattern = normalizeRoutePath(pattern) || urlPath; + + // Update root span with parameterized route (same as loader/action) + updateRootSpanWithRoute(info.request.method, pattern, urlPath); + + await startSpan( + { + name: routePattern, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.react-router.middleware', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.react_router.instrumentation_api', + }, + }, + async () => { + const result = await callMiddleware(); + captureInstrumentationError(result, captureErrors, 'react_router.middleware', { + 'http.method': info.request.method, + 'http.url': urlPath, + }); + }, + ); + }, + + async lazy(callLazy) { + await startSpan( + { + name: 'Lazy Route Load', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.react-router.lazy', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.react_router.instrumentation_api', + }, + }, + async () => { + const result = await callLazy(); + captureInstrumentationError(result, captureErrors, 'react_router.lazy', {}); + }, + ); + }, + }); + }, + }; +} + +function updateRootSpanWithRoute(method: string, pattern: string | undefined, urlPath: string): void { + const activeSpan = getActiveSpan(); + if (!activeSpan) return; + const rootSpan = getRootSpan(activeSpan); + if (!rootSpan) return; + + // Skip update if URL path is invalid (failed to parse) + if (!urlPath || urlPath === '') { + DEBUG_BUILD && debug.warn('Cannot update span with invalid URL path:', urlPath); + return; + } + + const hasPattern = !!pattern; + const routeName = hasPattern ? normalizeRoutePath(pattern) || urlPath : urlPath; + + const rpcMetadata = getRPCMetadata(context.active()); + if (rpcMetadata?.type === RPCType.HTTP) { + rpcMetadata.route = routeName; + } + + const transactionName = `${method} ${routeName}`; + updateSpanName(rootSpan, transactionName); + rootSpan.setAttributes({ + [ATTR_HTTP_ROUTE]: routeName, + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: hasPattern ? 'route' : 'url', + }); + + // Also update the scope's transaction name so errors captured during this request + // have the correct transaction name (not the initial placeholder like "GET *") + getCurrentScope().setTransactionName(transactionName); +} diff --git a/packages/react-router/src/server/index.ts b/packages/react-router/src/server/index.ts index acca80a94d81..e0b8c8981632 100644 --- a/packages/react-router/src/server/index.ts +++ b/packages/react-router/src/server/index.ts @@ -11,3 +11,10 @@ export { wrapServerAction } from './wrapServerAction'; export { wrapServerLoader } from './wrapServerLoader'; export { createSentryHandleError, type SentryHandleErrorOptions } from './createSentryHandleError'; export { getMetaTagTransformer } from './getMetaTagTransformer'; + +// React Router instrumentation API support (works with both unstable_instrumentations and instrumentations) +export { + createSentryServerInstrumentation, + isInstrumentationApiUsed, + type CreateSentryServerInstrumentationOptions, +} from './createServerInstrumentation'; diff --git a/packages/react-router/src/server/instrumentation/reactRouter.ts b/packages/react-router/src/server/instrumentation/reactRouter.ts index 708b9857015b..2f24d2c7bcb7 100644 --- a/packages/react-router/src/server/instrumentation/reactRouter.ts +++ b/packages/react-router/src/server/instrumentation/reactRouter.ts @@ -15,6 +15,7 @@ import { } from '@sentry/core'; import type * as reactRouter from 'react-router'; import { DEBUG_BUILD } from '../../common/debug-build'; +import { isInstrumentationApiUsed } from '../serverGlobals'; import { getOpName, getSpanName, isDataRequest } from './util'; type ReactRouterModuleExports = typeof reactRouter; @@ -76,6 +77,13 @@ export class ReactRouterInstrumentation extends InstrumentationBase { return { name: INTEGRATION_NAME, setupOnce() { + // Skip OTEL patching if the instrumentation API is in use + if (isInstrumentationApiUsed()) { + return; + } + if ( (NODE_VERSION.major === 20 && NODE_VERSION.minor < 19) || // https://nodejs.org/en/blog/release/v20.19.0 (NODE_VERSION.major === 22 && NODE_VERSION.minor < 12) // https://nodejs.org/en/blog/release/v22.12.0 @@ -36,13 +42,17 @@ export const reactRouterServerIntegration = defineIntegration(() => { if ( event.type === 'transaction' && event.contexts?.trace?.data && - event.contexts.trace.data[ATTR_HTTP_ROUTE] === '*' && - // This means the name has been adjusted before, but the http.route remains, so we need to remove it - event.transaction !== 'GET *' && - event.transaction !== 'POST *' + event.contexts.trace.data[ATTR_HTTP_ROUTE] === '*' ) { - // eslint-disable-next-line @typescript-eslint/no-dynamic-delete - delete event.contexts.trace.data[ATTR_HTTP_ROUTE]; + const origin = event.contexts.trace.origin; + const isInstrumentationApiOrigin = origin?.includes('instrumentation_api'); + + // For instrumentation_api, always clean up bogus `*` route since we set better names + // For legacy, only clean up if the name has been adjusted (not METHOD *) + if (isInstrumentationApiOrigin || !event.transaction?.endsWith(' *')) { + // eslint-disable-next-line @typescript-eslint/no-dynamic-delete + delete event.contexts.trace.data[ATTR_HTTP_ROUTE]; + } } return event; diff --git a/packages/react-router/src/server/serverGlobals.ts b/packages/react-router/src/server/serverGlobals.ts new file mode 100644 index 000000000000..33f96ab5f45a --- /dev/null +++ b/packages/react-router/src/server/serverGlobals.ts @@ -0,0 +1,23 @@ +import { GLOBAL_OBJ } from '@sentry/core'; + +const SENTRY_SERVER_INSTRUMENTATION_FLAG = '__sentryReactRouterServerInstrumentationUsed'; + +type GlobalObjWithFlag = typeof GLOBAL_OBJ & { + [SENTRY_SERVER_INSTRUMENTATION_FLAG]?: boolean; +}; + +/** + * Mark that the React Router instrumentation API is being used on the server. + * @internal + */ +export function markInstrumentationApiUsed(): void { + (GLOBAL_OBJ as GlobalObjWithFlag)[SENTRY_SERVER_INSTRUMENTATION_FLAG] = true; +} + +/** + * Check if React Router's instrumentation API is being used on the server. + * @experimental + */ +export function isInstrumentationApiUsed(): boolean { + return !!(GLOBAL_OBJ as GlobalObjWithFlag)[SENTRY_SERVER_INSTRUMENTATION_FLAG]; +} diff --git a/packages/react-router/src/server/wrapSentryHandleRequest.ts b/packages/react-router/src/server/wrapSentryHandleRequest.ts index 2e788637988f..9bf634a68505 100644 --- a/packages/react-router/src/server/wrapSentryHandleRequest.ts +++ b/packages/react-router/src/server/wrapSentryHandleRequest.ts @@ -4,11 +4,14 @@ import { ATTR_HTTP_ROUTE } from '@opentelemetry/semantic-conventions'; import { flushIfServerless, getActiveSpan, + getCurrentScope, getRootSpan, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, + updateSpanName, } from '@sentry/core'; import type { AppLoadContext, EntryContext, RouterContextProvider } from 'react-router'; +import { isInstrumentationApiUsed } from './serverGlobals'; type OriginalHandleRequestWithoutMiddleware = ( request: Request, @@ -67,7 +70,8 @@ export function wrapSentryHandleRequest( const rootSpan = activeSpan ? getRootSpan(activeSpan) : undefined; if (parameterizedPath && rootSpan) { - const routeName = `/${parameterizedPath}`; + // Normalize route name - avoid "//" for root routes + const routeName = parameterizedPath.startsWith('/') ? parameterizedPath : `/${parameterizedPath}`; // The express instrumentation writes on the rpcMetadata and that ends up stomping on the `http.route` attribute. const rpcMetadata = getRPCMetadata(context.active()); @@ -76,12 +80,25 @@ export function wrapSentryHandleRequest( rpcMetadata.route = routeName; } - // The span exporter picks up the `http.route` (ATTR_HTTP_ROUTE) attribute to set the transaction name - rootSpan.setAttributes({ - [ATTR_HTTP_ROUTE]: routeName, - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.react_router.request_handler', - }); + const transactionName = `${request.method} ${routeName}`; + + updateSpanName(rootSpan, transactionName); + getCurrentScope().setTransactionName(transactionName); + + // Set route attributes - acts as fallback for lazy-only routes when using instrumentation API + // Don't override origin when instrumentation API is used (preserve instrumentation_api origin) + if (isInstrumentationApiUsed()) { + rootSpan.setAttributes({ + [ATTR_HTTP_ROUTE]: routeName, + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + }); + } else { + rootSpan.setAttributes({ + [ATTR_HTTP_ROUTE]: routeName, + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.react_router.request_handler', + }); + } } try { diff --git a/packages/react-router/src/server/wrapServerAction.ts b/packages/react-router/src/server/wrapServerAction.ts index e816c3c63886..263bdddb62cd 100644 --- a/packages/react-router/src/server/wrapServerAction.ts +++ b/packages/react-router/src/server/wrapServerAction.ts @@ -1,6 +1,7 @@ import { SEMATTRS_HTTP_TARGET } from '@opentelemetry/semantic-conventions'; import type { SpanAttributes } from '@sentry/core'; import { + debug, flushIfServerless, getActiveSpan, getRootSpan, @@ -12,12 +13,17 @@ import { updateSpanName, } from '@sentry/core'; import type { ActionFunctionArgs } from 'react-router'; +import { DEBUG_BUILD } from '../common/debug-build'; +import { isInstrumentationApiUsed } from './serverGlobals'; type SpanOptions = { name?: string; attributes?: SpanAttributes; }; +// Track if we've already warned about duplicate instrumentation +let hasWarnedAboutDuplicateActionInstrumentation = false; + /** * Wraps a React Router server action function with Sentry performance monitoring. * @param options - Optional span configuration options including name, operation, description and attributes @@ -37,8 +43,23 @@ type SpanOptions = { * ); * ``` */ -export function wrapServerAction(options: SpanOptions = {}, actionFn: (args: ActionFunctionArgs) => Promise) { - return async function (args: ActionFunctionArgs) { +export function wrapServerAction( + options: SpanOptions = {}, + actionFn: (args: ActionFunctionArgs) => Promise, +): (args: ActionFunctionArgs) => Promise { + return async function (args: ActionFunctionArgs): Promise { + // Skip instrumentation if instrumentation API is already handling it + if (isInstrumentationApiUsed()) { + if (DEBUG_BUILD && !hasWarnedAboutDuplicateActionInstrumentation) { + hasWarnedAboutDuplicateActionInstrumentation = true; + debug.warn( + 'wrapServerAction is redundant when using the instrumentation API. ' + + 'The action is already instrumented automatically. You can safely remove wrapServerAction.', + ); + } + return actionFn(args); + } + const name = options.name || 'Executing Server Action'; const active = getActiveSpan(); if (active) { diff --git a/packages/react-router/src/server/wrapServerLoader.ts b/packages/react-router/src/server/wrapServerLoader.ts index 7e5083d4d5c8..34bb5a58aa1e 100644 --- a/packages/react-router/src/server/wrapServerLoader.ts +++ b/packages/react-router/src/server/wrapServerLoader.ts @@ -1,6 +1,7 @@ import { SEMATTRS_HTTP_TARGET } from '@opentelemetry/semantic-conventions'; import type { SpanAttributes } from '@sentry/core'; import { + debug, flushIfServerless, getActiveSpan, getRootSpan, @@ -12,12 +13,17 @@ import { updateSpanName, } from '@sentry/core'; import type { LoaderFunctionArgs } from 'react-router'; +import { DEBUG_BUILD } from '../common/debug-build'; +import { isInstrumentationApiUsed } from './serverGlobals'; type SpanOptions = { name?: string; attributes?: SpanAttributes; }; +// Track if we've already warned about duplicate instrumentation +let hasWarnedAboutDuplicateLoaderInstrumentation = false; + /** * Wraps a React Router server loader function with Sentry performance monitoring. * @param options - Optional span configuration options including name, operation, description and attributes @@ -37,8 +43,23 @@ type SpanOptions = { * ); * ``` */ -export function wrapServerLoader(options: SpanOptions = {}, loaderFn: (args: LoaderFunctionArgs) => Promise) { - return async function (args: LoaderFunctionArgs) { +export function wrapServerLoader( + options: SpanOptions = {}, + loaderFn: (args: LoaderFunctionArgs) => Promise, +): (args: LoaderFunctionArgs) => Promise { + return async function (args: LoaderFunctionArgs): Promise { + // Skip instrumentation if instrumentation API is already handling it + if (isInstrumentationApiUsed()) { + if (DEBUG_BUILD && !hasWarnedAboutDuplicateLoaderInstrumentation) { + hasWarnedAboutDuplicateLoaderInstrumentation = true; + debug.warn( + 'wrapServerLoader is redundant when using the instrumentation API. ' + + 'The loader is already instrumented automatically. You can safely remove wrapServerLoader.', + ); + } + return loaderFn(args); + } + const name = options.name || 'Executing Server Loader'; const active = getActiveSpan(); diff --git a/packages/react-router/test/client/createClientInstrumentation.test.ts b/packages/react-router/test/client/createClientInstrumentation.test.ts new file mode 100644 index 000000000000..ef885816905f --- /dev/null +++ b/packages/react-router/test/client/createClientInstrumentation.test.ts @@ -0,0 +1,451 @@ +import * as browser from '@sentry/browser'; +import * as core from '@sentry/core'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { + createSentryClientInstrumentation, + isClientInstrumentationApiUsed, + isNavigateHookInvoked, +} from '../../src/client/createClientInstrumentation'; + +vi.mock('@sentry/core', async () => { + const actual = await vi.importActual('@sentry/core'); + return { + ...actual, + startSpan: vi.fn(), + captureException: vi.fn(), + getClient: vi.fn(), + GLOBAL_OBJ: globalThis, + SEMANTIC_ATTRIBUTE_SENTRY_OP: 'sentry.op', + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN: 'sentry.origin', + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE: 'sentry.source', + }; +}); + +vi.mock('@sentry/browser', () => ({ + startBrowserTracingNavigationSpan: vi.fn(), +})); + +describe('createSentryClientInstrumentation', () => { + beforeEach(() => { + vi.clearAllMocks(); + // Reset global flag + delete (globalThis as any).__sentryReactRouterClientInstrumentationUsed; + }); + + afterEach(() => { + delete (globalThis as any).__sentryReactRouterClientInstrumentationUsed; + }); + + it('should create a valid client instrumentation object', () => { + const instrumentation = createSentryClientInstrumentation(); + + expect(instrumentation).toBeDefined(); + expect(typeof instrumentation.router).toBe('function'); + expect(typeof instrumentation.route).toBe('function'); + }); + + it('should set the global flag when created', () => { + expect((globalThis as any).__sentryReactRouterClientInstrumentationUsed).toBeUndefined(); + + createSentryClientInstrumentation(); + + expect((globalThis as any).__sentryReactRouterClientInstrumentationUsed).toBe(true); + }); + + it('should instrument router navigate with browser tracing span', async () => { + const mockCallNavigate = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); + const mockInstrument = vi.fn(); + const mockClient = {}; + + (core.getClient as any).mockReturnValue(mockClient); + + const instrumentation = createSentryClientInstrumentation(); + instrumentation.router?.({ instrument: mockInstrument }); + + expect(mockInstrument).toHaveBeenCalled(); + const hooks = mockInstrument.mock.calls[0]![0]; + + // Call the navigate hook with proper info structure + await hooks.navigate(mockCallNavigate, { + currentUrl: '/home', + to: '/about', + }); + + expect(browser.startBrowserTracingNavigationSpan).toHaveBeenCalledWith(mockClient, { + name: '/about', + attributes: expect.objectContaining({ + 'sentry.source': 'url', + 'sentry.op': 'navigation', + 'sentry.origin': 'auto.navigation.react_router.instrumentation_api', + }), + }); + expect(mockCallNavigate).toHaveBeenCalled(); + }); + + it('should instrument router fetch with spans', async () => { + const mockCallFetch = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); + const mockInstrument = vi.fn(); + + (core.startSpan as any).mockImplementation((_opts: any, fn: any) => fn()); + + const instrumentation = createSentryClientInstrumentation(); + instrumentation.router?.({ instrument: mockInstrument }); + + const hooks = mockInstrument.mock.calls[0]![0]; + + // Call the fetch hook with proper info structure + await hooks.fetch(mockCallFetch, { + href: '/api/data', + currentUrl: '/home', + fetcherKey: 'fetcher-1', + }); + + expect(core.startSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'Fetcher fetcher-1', + attributes: expect.objectContaining({ + 'sentry.op': 'function.react-router.fetcher', + 'sentry.origin': 'auto.function.react_router.instrumentation_api', + }), + }), + expect.any(Function), + ); + expect(mockCallFetch).toHaveBeenCalled(); + }); + + it('should instrument route loader with spans', async () => { + const mockCallLoader = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); + const mockInstrument = vi.fn(); + + (core.startSpan as any).mockImplementation((_opts: any, fn: any) => fn()); + + const instrumentation = createSentryClientInstrumentation(); + // Route has id, index, path as required properties + instrumentation.route?.({ + id: 'test-route', + index: false, + path: '/test', + instrument: mockInstrument, + }); + + expect(mockInstrument).toHaveBeenCalled(); + const hooks = mockInstrument.mock.calls[0]![0]; + + // Call the loader hook with RouteHandlerInstrumentationInfo + await hooks.loader(mockCallLoader, { + request: { method: 'GET', url: 'http://example.com/users/123', headers: { get: () => null } }, + params: { id: '123' }, + unstable_pattern: '/users/:id', + context: undefined, + }); + + expect(core.startSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: '/users/:id', + attributes: expect.objectContaining({ + 'sentry.op': 'function.react-router.client-loader', + 'sentry.origin': 'auto.function.react_router.instrumentation_api', + }), + }), + expect.any(Function), + ); + expect(mockCallLoader).toHaveBeenCalled(); + }); + + it('should instrument route action with spans', async () => { + const mockCallAction = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); + const mockInstrument = vi.fn(); + + (core.startSpan as any).mockImplementation((_opts: any, fn: any) => fn()); + + const instrumentation = createSentryClientInstrumentation(); + instrumentation.route?.({ + id: 'test-route', + index: false, + path: '/test', + instrument: mockInstrument, + }); + + const hooks = mockInstrument.mock.calls[0]![0]; + + // Call the action hook with RouteHandlerInstrumentationInfo + await hooks.action(mockCallAction, { + request: { method: 'POST', url: 'http://example.com/users/123', headers: { get: () => null } }, + params: { id: '123' }, + unstable_pattern: '/users/:id', + context: undefined, + }); + + expect(core.startSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: '/users/:id', + attributes: expect.objectContaining({ + 'sentry.op': 'function.react-router.client-action', + 'sentry.origin': 'auto.function.react_router.instrumentation_api', + }), + }), + expect.any(Function), + ); + }); + + it('should capture errors when captureErrors is true (default)', async () => { + const mockError = new Error('Test error'); + // React Router returns an error result, not a rejection + const mockCallLoader = vi.fn().mockResolvedValue({ status: 'error', error: mockError }); + const mockInstrument = vi.fn(); + + (core.startSpan as any).mockImplementation((_opts: any, fn: any) => fn()); + + const instrumentation = createSentryClientInstrumentation(); + instrumentation.route?.({ + id: 'test-route', + index: false, + path: '/test', + instrument: mockInstrument, + }); + + const hooks = mockInstrument.mock.calls[0]![0]; + + await hooks.loader(mockCallLoader, { + request: { method: 'GET', url: 'http://example.com/test-path', headers: { get: () => null } }, + params: {}, + unstable_pattern: '/test-path', + context: undefined, + }); + + expect(core.captureException).toHaveBeenCalledWith(mockError, { + mechanism: { type: 'react_router.client_loader', handled: false }, + data: { + 'http.url': '/test-path', + }, + }); + }); + + it('should not capture errors when captureErrors is false', async () => { + const mockError = new Error('Test error'); + // React Router returns an error result, not a rejection + const mockCallLoader = vi.fn().mockResolvedValue({ status: 'error', error: mockError }); + const mockInstrument = vi.fn(); + + (core.startSpan as any).mockImplementation((_opts: any, fn: any) => fn()); + + const instrumentation = createSentryClientInstrumentation({ captureErrors: false }); + instrumentation.route?.({ + id: 'test-route', + index: false, + path: '/test', + instrument: mockInstrument, + }); + + const hooks = mockInstrument.mock.calls[0]![0]; + + await hooks.loader(mockCallLoader, { + request: { method: 'GET', url: 'http://example.com/test-path', headers: { get: () => null } }, + params: {}, + unstable_pattern: '/test-path', + context: undefined, + }); + + expect(core.captureException).not.toHaveBeenCalled(); + }); + + it('should capture navigate errors', async () => { + const mockError = new Error('Navigation error'); + // React Router returns an error result, not a rejection + const mockCallNavigate = vi.fn().mockResolvedValue({ status: 'error', error: mockError }); + const mockInstrument = vi.fn(); + + (core.getClient as any).mockReturnValue({}); + + const instrumentation = createSentryClientInstrumentation(); + instrumentation.router?.({ instrument: mockInstrument }); + + const hooks = mockInstrument.mock.calls[0]![0]; + + await hooks.navigate(mockCallNavigate, { + currentUrl: '/home', + to: '/about', + }); + + expect(core.captureException).toHaveBeenCalledWith(mockError, { + mechanism: { type: 'react_router.navigate', handled: false }, + data: { + 'http.url': '/about', + }, + }); + }); + + it('should fall back to URL pathname when unstable_pattern is undefined', async () => { + const mockCallLoader = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); + const mockInstrument = vi.fn(); + + (core.startSpan as any).mockImplementation((_opts: any, fn: any) => fn()); + + const instrumentation = createSentryClientInstrumentation(); + instrumentation.route?.({ + id: 'test-route', + index: false, + path: '/test', + instrument: mockInstrument, + }); + + const hooks = mockInstrument.mock.calls[0]![0]; + + // Call with undefined unstable_pattern - should fall back to pathname + await hooks.loader(mockCallLoader, { + request: { method: 'GET', url: 'http://example.com/users/123', headers: { get: () => null } }, + params: { id: '123' }, + unstable_pattern: undefined, + context: undefined, + }); + + expect(core.startSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: '/users/123', + }), + expect.any(Function), + ); + }); + + it('should instrument route middleware with spans', async () => { + const mockCallMiddleware = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); + const mockInstrument = vi.fn(); + + (core.startSpan as any).mockImplementation((_opts: any, fn: any) => fn()); + + const instrumentation = createSentryClientInstrumentation(); + instrumentation.route?.({ + id: 'test-route', + index: false, + path: '/users/:id', + instrument: mockInstrument, + }); + + const hooks = mockInstrument.mock.calls[0]![0]; + + await hooks.middleware(mockCallMiddleware, { + request: { method: 'GET', url: 'http://example.com/users/123', headers: { get: () => null } }, + params: { id: '123' }, + unstable_pattern: '/users/:id', + context: undefined, + }); + + expect(core.startSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: '/users/:id', + attributes: expect.objectContaining({ + 'sentry.op': 'function.react-router.client-middleware', + 'sentry.origin': 'auto.function.react_router.instrumentation_api', + }), + }), + expect.any(Function), + ); + }); + + it('should instrument lazy route loading with spans', async () => { + const mockCallLazy = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); + const mockInstrument = vi.fn(); + + (core.startSpan as any).mockImplementation((_opts: any, fn: any) => fn()); + + const instrumentation = createSentryClientInstrumentation(); + instrumentation.route?.({ + id: 'test-route', + index: false, + path: '/users/:id', + instrument: mockInstrument, + }); + + const hooks = mockInstrument.mock.calls[0]![0]; + + await hooks.lazy(mockCallLazy, undefined); + + expect(core.startSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'Lazy Route Load', + attributes: expect.objectContaining({ + 'sentry.op': 'function.react-router.client-lazy', + 'sentry.origin': 'auto.function.react_router.instrumentation_api', + }), + }), + expect.any(Function), + ); + }); +}); + +describe('isClientInstrumentationApiUsed', () => { + beforeEach(() => { + delete (globalThis as any).__sentryReactRouterClientInstrumentationUsed; + }); + + afterEach(() => { + delete (globalThis as any).__sentryReactRouterClientInstrumentationUsed; + }); + + it('should return false when flag is not set', () => { + expect(isClientInstrumentationApiUsed()).toBe(false); + }); + + it('should return true when flag is set', () => { + (globalThis as any).__sentryReactRouterClientInstrumentationUsed = true; + expect(isClientInstrumentationApiUsed()).toBe(true); + }); + + it('should return true after createSentryClientInstrumentation is called', () => { + expect(isClientInstrumentationApiUsed()).toBe(false); + createSentryClientInstrumentation(); + expect(isClientInstrumentationApiUsed()).toBe(true); + }); +}); + +describe('isNavigateHookInvoked', () => { + beforeEach(() => { + delete (globalThis as any).__sentryReactRouterNavigateHookInvoked; + delete (globalThis as any).__sentryReactRouterClientInstrumentationUsed; + }); + + afterEach(() => { + delete (globalThis as any).__sentryReactRouterNavigateHookInvoked; + delete (globalThis as any).__sentryReactRouterClientInstrumentationUsed; + }); + + it('should return false when flag is not set', () => { + expect(isNavigateHookInvoked()).toBe(false); + }); + + it('should return true when flag is set', () => { + (globalThis as any).__sentryReactRouterNavigateHookInvoked = true; + expect(isNavigateHookInvoked()).toBe(true); + }); + + it('should return false after createSentryClientInstrumentation is called (before navigate)', () => { + createSentryClientInstrumentation(); + // Flag should not be set just by creating instrumentation + // It only gets set when the navigate hook is actually invoked + expect(isNavigateHookInvoked()).toBe(false); + }); + + it('should return true after navigate hook is invoked', async () => { + const mockCallNavigate = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); + const mockInstrument = vi.fn(); + + (core.getClient as any).mockReturnValue({}); + + const instrumentation = createSentryClientInstrumentation(); + instrumentation.router?.({ instrument: mockInstrument }); + + // Before navigation, flag should be false + expect(isNavigateHookInvoked()).toBe(false); + + const hooks = mockInstrument.mock.calls[0]![0]; + + // Call the navigate hook + await hooks.navigate(mockCallNavigate, { + currentUrl: '/home', + to: '/about', + }); + + // After navigation, flag should be true + expect(isNavigateHookInvoked()).toBe(true); + }); +}); diff --git a/packages/react-router/test/client/hydratedRouter.test.ts b/packages/react-router/test/client/hydratedRouter.test.ts index 3e798e829566..fdfdb1b92929 100644 --- a/packages/react-router/test/client/hydratedRouter.test.ts +++ b/packages/react-router/test/client/hydratedRouter.test.ts @@ -11,6 +11,9 @@ vi.mock('@sentry/core', async () => { getRootSpan: vi.fn(), spanToJSON: vi.fn(), getClient: vi.fn(), + debug: { + warn: vi.fn(), + }, SEMANTIC_ATTRIBUTE_SENTRY_OP: 'op', SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN: 'origin', SEMANTIC_ATTRIBUTE_SENTRY_SOURCE: 'source', @@ -108,4 +111,48 @@ describe('instrumentHydratedRouter', () => { expect(mockNavigationSpan.updateName).not.toHaveBeenCalled(); expect(mockNavigationSpan.setAttributes).not.toHaveBeenCalled(); }); + + it('skips navigation span creation when instrumentation API navigate hook has been invoked', () => { + // Simulate that the instrumentation API's navigate hook has been invoked + // (meaning React Router is invoking the hooks and we should avoid double-counting) + (globalThis as any).__sentryReactRouterNavigateHookInvoked = true; + + instrumentHydratedRouter(); + mockRouter.navigate('/bar'); + + // Should not create a navigation span because instrumentation API is handling it + expect(browser.startBrowserTracingNavigationSpan).not.toHaveBeenCalled(); + + // Clean up + delete (globalThis as any).__sentryReactRouterNavigateHookInvoked; + }); + + it('creates navigation span when instrumentation API navigate hook has not been invoked', () => { + // Ensure the flag is not set (default state) + delete (globalThis as any).__sentryReactRouterNavigateHookInvoked; + + instrumentHydratedRouter(); + mockRouter.navigate('/bar'); + + // Should create a navigation span because instrumentation API is not handling it + expect(browser.startBrowserTracingNavigationSpan).toHaveBeenCalled(); + }); + + it('should warn when router is not found after max retries', () => { + vi.useFakeTimers(); + + // Remove the router to simulate it not being available + delete (globalThis as any).__reactRouterDataRouter; + + instrumentHydratedRouter(); + + // Advance timers past MAX_RETRIES (40 retries × 50ms = 2000ms) + vi.advanceTimersByTime(2100); + + expect(core.debug.warn).toHaveBeenCalledWith( + 'Unable to instrument React Router: router not found after hydration.', + ); + + vi.useRealTimers(); + }); }); diff --git a/packages/react-router/test/client/tracingIntegration.test.ts b/packages/react-router/test/client/tracingIntegration.test.ts index 2469c9b29db6..b11ccc4c0b0b 100644 --- a/packages/react-router/test/client/tracingIntegration.test.ts +++ b/packages/react-router/test/client/tracingIntegration.test.ts @@ -1,12 +1,22 @@ import * as sentryBrowser from '@sentry/browser'; import type { Client } from '@sentry/core'; +import { GLOBAL_OBJ } from '@sentry/core'; import { afterEach, describe, expect, it, vi } from 'vitest'; import * as hydratedRouterModule from '../../src/client/hydratedRouter'; import { reactRouterTracingIntegration } from '../../src/client/tracingIntegration'; +// Global flag used by client instrumentation API +const SENTRY_CLIENT_INSTRUMENTATION_FLAG = '__sentryReactRouterClientInstrumentationUsed'; + +type GlobalObjWithFlag = typeof GLOBAL_OBJ & { + [SENTRY_CLIENT_INSTRUMENTATION_FLAG]?: boolean; +}; + describe('reactRouterTracingIntegration', () => { afterEach(() => { vi.clearAllMocks(); + // Clean up global flag between tests + (GLOBAL_OBJ as GlobalObjWithFlag)[SENTRY_CLIENT_INSTRUMENTATION_FLAG] = undefined; }); it('returns an integration with the correct name and properties', () => { @@ -28,4 +38,91 @@ describe('reactRouterTracingIntegration', () => { expect(browserTracingSpy).toHaveBeenCalled(); expect(instrumentSpy).toHaveBeenCalled(); }); + + describe('clientInstrumentation', () => { + it('provides clientInstrumentation property', () => { + const integration = reactRouterTracingIntegration(); + + expect(integration.clientInstrumentation).toBeDefined(); + }); + + it('lazily creates clientInstrumentation only when accessed', () => { + const integration = reactRouterTracingIntegration(); + + // Flag should not be set yet (lazy initialization) + expect((GLOBAL_OBJ as GlobalObjWithFlag)[SENTRY_CLIENT_INSTRUMENTATION_FLAG]).toBeUndefined(); + + // Access the instrumentation + const instrumentation = integration.clientInstrumentation; + + // Now the flag should be set + expect((GLOBAL_OBJ as GlobalObjWithFlag)[SENTRY_CLIENT_INSTRUMENTATION_FLAG]).toBe(true); + expect(instrumentation).toBeDefined(); + expect(typeof instrumentation.router).toBe('function'); + expect(typeof instrumentation.route).toBe('function'); + }); + + it('returns the same clientInstrumentation instance on multiple accesses', () => { + const integration = reactRouterTracingIntegration(); + + const first = integration.clientInstrumentation; + const second = integration.clientInstrumentation; + + expect(first).toBe(second); + }); + + it('passes options to createSentryClientInstrumentation', () => { + const integration = reactRouterTracingIntegration({ + instrumentationOptions: { + captureErrors: false, + }, + }); + + const instrumentation = integration.clientInstrumentation; + + // The instrumentation is created - we can verify by checking it has the expected shape + expect(instrumentation).toBeDefined(); + expect(typeof instrumentation.router).toBe('function'); + expect(typeof instrumentation.route).toBe('function'); + }); + + it('eagerly creates instrumentation when useInstrumentationAPI is true', () => { + // Flag should not be set before creating integration + expect((GLOBAL_OBJ as GlobalObjWithFlag)[SENTRY_CLIENT_INSTRUMENTATION_FLAG]).toBeUndefined(); + + // Create integration with useInstrumentationAPI: true + reactRouterTracingIntegration({ useInstrumentationAPI: true }); + + // Flag should be set immediately (eager initialization), not waiting for getter access + expect((GLOBAL_OBJ as GlobalObjWithFlag)[SENTRY_CLIENT_INSTRUMENTATION_FLAG]).toBe(true); + }); + + it('eagerly creates instrumentation when instrumentationOptions is provided', () => { + expect((GLOBAL_OBJ as GlobalObjWithFlag)[SENTRY_CLIENT_INSTRUMENTATION_FLAG]).toBeUndefined(); + + reactRouterTracingIntegration({ instrumentationOptions: {} }); + + // Flag should be set immediately due to options presence + expect((GLOBAL_OBJ as GlobalObjWithFlag)[SENTRY_CLIENT_INSTRUMENTATION_FLAG]).toBe(true); + }); + + it('calls instrumentHydratedRouter when useInstrumentationAPI is true', () => { + vi.spyOn(sentryBrowser, 'browserTracingIntegration').mockImplementation(() => ({ + setup: vi.fn(), + afterAllSetup: vi.fn(), + name: 'BrowserTracing', + })); + const instrumentSpy = vi.spyOn(hydratedRouterModule, 'instrumentHydratedRouter').mockImplementation(() => null); + + // Create with useInstrumentationAPI - flag is set eagerly + const integration = reactRouterTracingIntegration({ useInstrumentationAPI: true }); + + // afterAllSetup runs + integration.afterAllSetup?.({} as Client); + + // instrumentHydratedRouter is called for both pageload and navigation handling + // (In Framework Mode, HydratedRouter doesn't invoke client hooks, so legacy instrumentation remains active) + expect(instrumentSpy).toHaveBeenCalled(); + }); + }); }); diff --git a/packages/react-router/test/common/utils.test.ts b/packages/react-router/test/common/utils.test.ts new file mode 100644 index 000000000000..4dc0cbf288d2 --- /dev/null +++ b/packages/react-router/test/common/utils.test.ts @@ -0,0 +1,144 @@ +import * as core from '@sentry/core'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { + captureInstrumentationError, + getPathFromRequest, + getPattern, + normalizeRoutePath, +} from '../../src/common/utils'; + +vi.mock('@sentry/core', async () => { + const actual = await vi.importActual('@sentry/core'); + return { + ...actual, + captureException: vi.fn(), + }; +}); + +describe('getPathFromRequest', () => { + it('should extract pathname from valid absolute URL', () => { + const request = { url: 'http://example.com/users/123' }; + expect(getPathFromRequest(request)).toBe('/users/123'); + }); + + it('should extract pathname from relative URL using dummy base', () => { + const request = { url: '/api/data' }; + expect(getPathFromRequest(request)).toBe('/api/data'); + }); + + it('should handle malformed URLs by treating them as relative paths', () => { + // The dummy base URL fallback handles most strings as relative paths + // This verifies the fallback works even for unusual URL strings + const request = { url: ':::invalid:::' }; + expect(getPathFromRequest(request)).toBe('/:::invalid:::'); + }); + + it('should handle URL with query string', () => { + const request = { url: 'http://example.com/search?q=test' }; + expect(getPathFromRequest(request)).toBe('/search'); + }); + + it('should handle URL with fragment', () => { + const request = { url: 'http://example.com/page#section' }; + expect(getPathFromRequest(request)).toBe('/page'); + }); + + it('should handle root path', () => { + const request = { url: 'http://example.com/' }; + expect(getPathFromRequest(request)).toBe('/'); + }); +}); + +describe('getPattern', () => { + it('should prefer stable pattern over unstable_pattern', () => { + const info = { pattern: '/users/:id', unstable_pattern: '/old/:id' }; + expect(getPattern(info)).toBe('/users/:id'); + }); + + it('should fall back to unstable_pattern when pattern is undefined', () => { + const info = { unstable_pattern: '/users/:id' }; + expect(getPattern(info)).toBe('/users/:id'); + }); + + it('should return undefined when neither is available', () => { + const info = {}; + expect(getPattern(info)).toBeUndefined(); + }); +}); + +describe('normalizeRoutePath', () => { + it('should add leading slash if missing', () => { + expect(normalizeRoutePath('users/:id')).toBe('/users/:id'); + }); + + it('should keep existing leading slash', () => { + expect(normalizeRoutePath('/users/:id')).toBe('/users/:id'); + }); + + it('should return undefined for falsy input', () => { + expect(normalizeRoutePath(undefined)).toBeUndefined(); + expect(normalizeRoutePath('')).toBeUndefined(); + }); +}); + +describe('captureInstrumentationError', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('should capture error when result is error and captureErrors is true', () => { + const error = new Error('test error'); + const result = { status: 'error' as const, error }; + const data = { 'http.url': '/test' }; + + captureInstrumentationError(result, true, 'react_router.loader', data); + + expect(core.captureException).toHaveBeenCalledWith(error, { + mechanism: { type: 'react_router.loader', handled: false }, + data, + }); + }); + + it('should not capture error when captureErrors is false', () => { + const error = new Error('test error'); + const result = { status: 'error' as const, error }; + + captureInstrumentationError(result, false, 'react_router.loader', {}); + + expect(core.captureException).not.toHaveBeenCalled(); + }); + + it('should not capture when result is success', () => { + const result = { status: 'success' as const, error: undefined }; + + captureInstrumentationError(result, true, 'react_router.loader', {}); + + expect(core.captureException).not.toHaveBeenCalled(); + }); + + it('should not capture Response objects (redirects are expected control flow)', () => { + const response = new Response(null, { status: 302 }); + const result = { status: 'error' as const, error: response }; + + captureInstrumentationError(result, true, 'react_router.loader', {}); + + expect(core.captureException).not.toHaveBeenCalled(); + }); + + it('should not capture non-Error objects (e.g., ErrorResponse)', () => { + const errorResponse = { status: 404, data: 'Not found' }; + const result = { status: 'error' as const, error: errorResponse }; + + captureInstrumentationError(result, true, 'react_router.loader', {}); + + expect(core.captureException).not.toHaveBeenCalled(); + }); + + it('should not capture string errors', () => { + const result = { status: 'error' as const, error: 'Something went wrong' }; + + captureInstrumentationError(result, true, 'react_router.loader', {}); + + expect(core.captureException).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/react-router/test/server/createServerInstrumentation.test.ts b/packages/react-router/test/server/createServerInstrumentation.test.ts new file mode 100644 index 000000000000..89135f5d7061 --- /dev/null +++ b/packages/react-router/test/server/createServerInstrumentation.test.ts @@ -0,0 +1,432 @@ +import * as core from '@sentry/core'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { + createSentryServerInstrumentation, + isInstrumentationApiUsed, +} from '../../src/server/createServerInstrumentation'; + +vi.mock('@sentry/core', async () => { + const actual = await vi.importActual('@sentry/core'); + return { + ...actual, + startSpan: vi.fn(), + captureException: vi.fn(), + flushIfServerless: vi.fn(), + getActiveSpan: vi.fn(), + getRootSpan: vi.fn(), + updateSpanName: vi.fn(), + GLOBAL_OBJ: globalThis, + SEMANTIC_ATTRIBUTE_SENTRY_OP: 'sentry.op', + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN: 'sentry.origin', + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE: 'sentry.source', + }; +}); + +describe('createSentryServerInstrumentation', () => { + beforeEach(() => { + vi.clearAllMocks(); + // Reset global flag + delete (globalThis as any).__sentryReactRouterServerInstrumentationUsed; + }); + + afterEach(() => { + delete (globalThis as any).__sentryReactRouterServerInstrumentationUsed; + }); + + it('should create a valid server instrumentation object', () => { + const instrumentation = createSentryServerInstrumentation(); + + expect(instrumentation).toBeDefined(); + expect(typeof instrumentation.handler).toBe('function'); + expect(typeof instrumentation.route).toBe('function'); + }); + + it('should set the global flag when created', () => { + expect((globalThis as any).__sentryReactRouterServerInstrumentationUsed).toBeUndefined(); + + createSentryServerInstrumentation(); + + expect((globalThis as any).__sentryReactRouterServerInstrumentationUsed).toBe(true); + }); + + it('should update root span with handler request attributes', async () => { + const mockRequest = new Request('http://example.com/test-path'); + const mockHandleRequest = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); + const mockInstrument = vi.fn(); + const mockSetAttributes = vi.fn(); + const mockRootSpan = { setAttributes: mockSetAttributes }; + + (core.getActiveSpan as any).mockReturnValue({}); + (core.getRootSpan as any).mockReturnValue(mockRootSpan); + + const instrumentation = createSentryServerInstrumentation(); + instrumentation.handler?.({ instrument: mockInstrument }); + + expect(mockInstrument).toHaveBeenCalled(); + const hooks = mockInstrument.mock.calls[0]![0]; + + // Call the request hook with RequestHandlerInstrumentationInfo + await hooks.request(mockHandleRequest, { request: mockRequest, context: undefined }); + + // Should update the root span name and attributes + expect(core.updateSpanName).toHaveBeenCalledWith(mockRootSpan, 'GET /test-path'); + expect(mockSetAttributes).toHaveBeenCalledWith({ + 'sentry.op': 'http.server', + 'sentry.origin': 'auto.http.react_router.instrumentation_api', + 'sentry.source': 'url', + }); + expect(mockHandleRequest).toHaveBeenCalled(); + expect(core.flushIfServerless).toHaveBeenCalled(); + }); + + it('should create own root span when no active span exists', async () => { + const mockRequest = new Request('http://example.com/api/users'); + const mockHandleRequest = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); + const mockInstrument = vi.fn(); + + // No active span exists + (core.getActiveSpan as any).mockReturnValue(undefined); + (core.startSpan as any).mockImplementation((_opts: any, fn: any) => fn()); + + const instrumentation = createSentryServerInstrumentation(); + instrumentation.handler?.({ instrument: mockInstrument }); + + const hooks = mockInstrument.mock.calls[0]![0]; + + await hooks.request(mockHandleRequest, { request: mockRequest, context: undefined }); + + // Should create a new root span with forceTransaction + expect(core.startSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'GET /api/users', + forceTransaction: true, + attributes: expect.objectContaining({ + 'sentry.op': 'http.server', + 'sentry.origin': 'auto.http.react_router.instrumentation_api', + 'sentry.source': 'url', + 'http.request.method': 'GET', + 'url.path': '/api/users', + 'url.full': 'http://example.com/api/users', + }), + }), + expect.any(Function), + ); + expect(mockHandleRequest).toHaveBeenCalled(); + expect(core.flushIfServerless).toHaveBeenCalled(); + }); + + it('should capture errors in handler when no root span exists', async () => { + const mockRequest = new Request('http://example.com/api/users'); + const mockError = new Error('Handler error'); + const mockHandleRequest = vi.fn().mockResolvedValue({ status: 'error', error: mockError }); + const mockInstrument = vi.fn(); + + (core.getActiveSpan as any).mockReturnValue(undefined); + (core.startSpan as any).mockImplementation((_opts: any, fn: any) => fn()); + + const instrumentation = createSentryServerInstrumentation(); + instrumentation.handler?.({ instrument: mockInstrument }); + + const hooks = mockInstrument.mock.calls[0]![0]; + + await hooks.request(mockHandleRequest, { request: mockRequest, context: undefined }); + + expect(core.captureException).toHaveBeenCalledWith(mockError, { + mechanism: { type: 'react_router.request_handler', handled: false }, + data: { + 'http.method': 'GET', + 'http.url': '/api/users', + }, + }); + }); + + it('should handle invalid URL gracefully and still call handler', async () => { + // Create a request object with an invalid URL that will fail URL parsing + const mockRequest = { url: 'not-a-valid-url', method: 'GET' } as unknown as Request; + const mockHandleRequest = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); + const mockInstrument = vi.fn(); + + const instrumentation = createSentryServerInstrumentation(); + instrumentation.handler?.({ instrument: mockInstrument }); + + const hooks = mockInstrument.mock.calls[0]![0]; + + await hooks.request(mockHandleRequest, { request: mockRequest, context: undefined }); + + // Handler should still be called even if URL parsing fails + expect(mockHandleRequest).toHaveBeenCalled(); + expect(core.flushIfServerless).toHaveBeenCalled(); + }); + + it('should handle relative URLs by using a dummy base', async () => { + const mockRequest = { url: '/relative/path', method: 'GET' } as unknown as Request; + const mockHandleRequest = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); + const mockInstrument = vi.fn(); + const mockSetAttributes = vi.fn(); + const mockRootSpan = { setAttributes: mockSetAttributes }; + + (core.getActiveSpan as any).mockReturnValue({}); + (core.getRootSpan as any).mockReturnValue(mockRootSpan); + + const instrumentation = createSentryServerInstrumentation(); + instrumentation.handler?.({ instrument: mockInstrument }); + + const hooks = mockInstrument.mock.calls[0]![0]; + + await hooks.request(mockHandleRequest, { request: mockRequest, context: undefined }); + + expect(core.updateSpanName).toHaveBeenCalledWith(mockRootSpan, 'GET /relative/path'); + }); + + it('should instrument route loader with spans', async () => { + const mockCallLoader = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); + const mockInstrument = vi.fn(); + + (core.startSpan as any).mockImplementation((_opts: any, fn: any) => fn()); + (core.getActiveSpan as any).mockReturnValue({}); + (core.getRootSpan as any).mockReturnValue({ setAttributes: vi.fn() }); + + const instrumentation = createSentryServerInstrumentation(); + instrumentation.route?.({ + id: 'test-route', + index: false, + path: '/users/:id', + instrument: mockInstrument, + }); + + expect(mockInstrument).toHaveBeenCalled(); + const hooks = mockInstrument.mock.calls[0]![0]; + + // Call the loader hook with RouteHandlerInstrumentationInfo + await hooks.loader(mockCallLoader, { + request: { method: 'GET', url: 'http://example.com/users/123', headers: { get: () => null } }, + params: { id: '123' }, + unstable_pattern: '/users/:id', + context: undefined, + }); + + expect(core.startSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: '/users/:id', + attributes: expect.objectContaining({ + 'sentry.op': 'function.react-router.loader', + 'sentry.origin': 'auto.function.react_router.instrumentation_api', + }), + }), + expect.any(Function), + ); + expect(mockCallLoader).toHaveBeenCalled(); + expect(core.updateSpanName).toHaveBeenCalled(); + }); + + it('should instrument route action with spans', async () => { + const mockCallAction = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); + const mockInstrument = vi.fn(); + + (core.startSpan as any).mockImplementation((_opts: any, fn: any) => fn()); + (core.getActiveSpan as any).mockReturnValue({}); + (core.getRootSpan as any).mockReturnValue({ setAttributes: vi.fn() }); + + const instrumentation = createSentryServerInstrumentation(); + instrumentation.route?.({ + id: 'test-route', + index: false, + path: '/users/:id', + instrument: mockInstrument, + }); + + const hooks = mockInstrument.mock.calls[0]![0]; + + // Call the action hook with RouteHandlerInstrumentationInfo + await hooks.action(mockCallAction, { + request: { method: 'POST', url: 'http://example.com/users/123', headers: { get: () => null } }, + params: { id: '123' }, + unstable_pattern: '/users/:id', + context: undefined, + }); + + expect(core.startSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: '/users/:id', + attributes: expect.objectContaining({ + 'sentry.op': 'function.react-router.action', + 'sentry.origin': 'auto.function.react_router.instrumentation_api', + }), + }), + expect.any(Function), + ); + }); + + it('should instrument route middleware with spans', async () => { + const mockCallMiddleware = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); + const mockInstrument = vi.fn(); + const mockSetAttributes = vi.fn(); + const mockRootSpan = { setAttributes: mockSetAttributes }; + + (core.startSpan as any).mockImplementation((_opts: any, fn: any) => fn()); + (core.getActiveSpan as any).mockReturnValue({}); + (core.getRootSpan as any).mockReturnValue(mockRootSpan); + + const instrumentation = createSentryServerInstrumentation(); + instrumentation.route?.({ + id: 'test-route', + index: false, + path: '/users/:id', + instrument: mockInstrument, + }); + + const hooks = mockInstrument.mock.calls[0]![0]; + + // Call the middleware hook with RouteHandlerInstrumentationInfo + await hooks.middleware(mockCallMiddleware, { + request: { method: 'GET', url: 'http://example.com/users/123', headers: { get: () => null } }, + params: { id: '123' }, + unstable_pattern: '/users/:id', + context: undefined, + }); + + expect(core.startSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: '/users/:id', + attributes: expect.objectContaining({ + 'sentry.op': 'function.react-router.middleware', + 'sentry.origin': 'auto.function.react_router.instrumentation_api', + }), + }), + expect.any(Function), + ); + + // Verify updateRootSpanWithRoute was called (same as loader/action) + // This updates the root span name and sets http.route for parameterized routes + expect(core.updateSpanName).toHaveBeenCalledWith(mockRootSpan, 'GET /users/:id'); + expect(mockSetAttributes).toHaveBeenCalledWith( + expect.objectContaining({ + 'http.route': '/users/:id', + 'sentry.source': 'route', + }), + ); + }); + + it('should instrument lazy route loading with spans', async () => { + const mockCallLazy = vi.fn().mockResolvedValue({ status: 'success', error: undefined }); + const mockInstrument = vi.fn(); + + (core.startSpan as any).mockImplementation((_opts: any, fn: any) => fn()); + + const instrumentation = createSentryServerInstrumentation(); + instrumentation.route?.({ + id: 'test-route', + index: false, + path: '/users/:id', + instrument: mockInstrument, + }); + + const hooks = mockInstrument.mock.calls[0]![0]; + + // Call the lazy hook - info is undefined for lazy loading + await hooks.lazy(mockCallLazy, undefined); + + expect(core.startSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'Lazy Route Load', + attributes: expect.objectContaining({ + 'sentry.op': 'function.react-router.lazy', + 'sentry.origin': 'auto.function.react_router.instrumentation_api', + }), + }), + expect.any(Function), + ); + expect(mockCallLazy).toHaveBeenCalled(); + }); + + it('should capture errors when captureErrors is true (default)', async () => { + const mockError = new Error('Test error'); + // React Router returns an error result, not a rejection + const mockCallLoader = vi.fn().mockResolvedValue({ status: 'error', error: mockError }); + const mockInstrument = vi.fn(); + + (core.startSpan as any).mockImplementation((_opts: any, fn: any) => fn()); + (core.getActiveSpan as any).mockReturnValue({}); + (core.getRootSpan as any).mockReturnValue({ setAttributes: vi.fn() }); + + const instrumentation = createSentryServerInstrumentation(); + instrumentation.route?.({ + id: 'test-route', + index: false, + path: '/test', + instrument: mockInstrument, + }); + + const hooks = mockInstrument.mock.calls[0]![0]; + + await hooks.loader(mockCallLoader, { + request: { method: 'GET', url: 'http://example.com/test', headers: { get: () => null } }, + params: {}, + unstable_pattern: '/test', + context: undefined, + }); + + expect(core.captureException).toHaveBeenCalledWith(mockError, { + mechanism: { type: 'react_router.loader', handled: false }, + data: { + 'http.method': 'GET', + 'http.url': '/test', + }, + }); + }); + + it('should not capture errors when captureErrors is false', async () => { + const mockError = new Error('Test error'); + // React Router returns an error result, not a rejection + const mockCallLoader = vi.fn().mockResolvedValue({ status: 'error', error: mockError }); + const mockInstrument = vi.fn(); + + (core.startSpan as any).mockImplementation((_opts: any, fn: any) => fn()); + (core.getActiveSpan as any).mockReturnValue({}); + (core.getRootSpan as any).mockReturnValue({ setAttributes: vi.fn() }); + + const instrumentation = createSentryServerInstrumentation({ captureErrors: false }); + instrumentation.route?.({ + id: 'test-route', + index: false, + path: '/test', + instrument: mockInstrument, + }); + + const hooks = mockInstrument.mock.calls[0]![0]; + + await hooks.loader(mockCallLoader, { + request: { method: 'GET', url: 'http://example.com/test', headers: { get: () => null } }, + params: {}, + unstable_pattern: '/test', + context: undefined, + }); + + expect(core.captureException).not.toHaveBeenCalled(); + }); +}); + +describe('isInstrumentationApiUsed', () => { + beforeEach(() => { + delete (globalThis as any).__sentryReactRouterServerInstrumentationUsed; + }); + + afterEach(() => { + delete (globalThis as any).__sentryReactRouterServerInstrumentationUsed; + }); + + it('should return false when flag is not set', () => { + expect(isInstrumentationApiUsed()).toBe(false); + }); + + it('should return true when flag is set', () => { + (globalThis as any).__sentryReactRouterServerInstrumentationUsed = true; + expect(isInstrumentationApiUsed()).toBe(true); + }); + + it('should return true after createSentryServerInstrumentation is called', () => { + expect(isInstrumentationApiUsed()).toBe(false); + createSentryServerInstrumentation(); + expect(isInstrumentationApiUsed()).toBe(true); + }); +}); diff --git a/packages/react-router/test/server/instrumentation/reactRouterServer.test.ts b/packages/react-router/test/server/instrumentation/reactRouterServer.test.ts index fb5141f8830d..93e0a91a1c2b 100644 --- a/packages/react-router/test/server/instrumentation/reactRouterServer.test.ts +++ b/packages/react-router/test/server/instrumentation/reactRouterServer.test.ts @@ -18,6 +18,7 @@ vi.mock('@sentry/core', async () => { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN: 'sentry.origin', SEMANTIC_ATTRIBUTE_SENTRY_SOURCE: 'sentry.source', startSpan: vi.fn((opts, fn) => fn({})), + GLOBAL_OBJ: {}, }; }); diff --git a/packages/react-router/test/server/wrapSentryHandleRequest.test.ts b/packages/react-router/test/server/wrapSentryHandleRequest.test.ts index 45b4ca1062df..71875d1aa887 100644 --- a/packages/react-router/test/server/wrapSentryHandleRequest.test.ts +++ b/packages/react-router/test/server/wrapSentryHandleRequest.test.ts @@ -24,11 +24,16 @@ vi.mock('@sentry/core', () => ({ getRootSpan: vi.fn(), getTraceMetaTags: vi.fn(), flushIfServerless: vi.fn(), + updateSpanName: vi.fn(), + getCurrentScope: vi.fn(() => ({ setTransactionName: vi.fn() })), + GLOBAL_OBJ: globalThis, })); describe('wrapSentryHandleRequest', () => { beforeEach(() => { vi.clearAllMocks(); + // Reset global flag for unstable instrumentation + delete (globalThis as any).__sentryReactRouterServerInstrumentationUsed; }); test('should call original handler with same parameters', async () => { @@ -175,6 +180,39 @@ describe('wrapSentryHandleRequest', () => { mockError, ); }); + + test('should set route attributes as fallback when instrumentation API is used (for lazy-only routes)', async () => { + // Set the global flag indicating instrumentation API is in use + (globalThis as any).__sentryReactRouterServerInstrumentationUsed = true; + + const originalHandler = vi.fn().mockResolvedValue('test'); + const wrappedHandler = wrapSentryHandleRequest(originalHandler); + + const mockActiveSpan = {}; + const mockRootSpan = { setAttributes: vi.fn() }; + const mockRpcMetadata = { type: RPCType.HTTP, route: '/some-path' }; + + (getActiveSpan as unknown as ReturnType).mockReturnValue(mockActiveSpan); + (getRootSpan as unknown as ReturnType).mockReturnValue(mockRootSpan); + const getRPCMetadata = vi.fn().mockReturnValue(mockRpcMetadata); + (vi.importActual('@opentelemetry/core') as unknown as { getRPCMetadata: typeof getRPCMetadata }).getRPCMetadata = + getRPCMetadata; + + const routerContext = { + staticHandlerContext: { + matches: [{ route: { path: 'some-path' } }], + }, + } as any; + + await wrappedHandler(new Request('https://nacho.queso'), 200, new Headers(), routerContext, {} as any); + + // Should set route attributes without origin (to preserve instrumentation_api origin) + expect(mockRootSpan.setAttributes).toHaveBeenCalledWith({ + [ATTR_HTTP_ROUTE]: '/some-path', + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + }); + expect(mockRpcMetadata.route).toBe('/some-path'); + }); }); describe('getMetaTagTransformer', () => { diff --git a/packages/react-router/test/server/wrapServerAction.test.ts b/packages/react-router/test/server/wrapServerAction.test.ts index 5eb92ef53b3b..9b01d229bc5a 100644 --- a/packages/react-router/test/server/wrapServerAction.test.ts +++ b/packages/react-router/test/server/wrapServerAction.test.ts @@ -1,6 +1,6 @@ import * as core from '@sentry/core'; import type { ActionFunctionArgs } from 'react-router'; -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { wrapServerAction } from '../../src/server/wrapServerAction'; vi.mock('@sentry/core', async () => { @@ -9,12 +9,21 @@ vi.mock('@sentry/core', async () => { ...actual, startSpan: vi.fn(), flushIfServerless: vi.fn(), + debug: { + warn: vi.fn(), + }, }; }); describe('wrapServerAction', () => { beforeEach(() => { vi.clearAllMocks(); + // Reset the global flag and warning state + delete (globalThis as any).__sentryReactRouterServerInstrumentationUsed; + }); + + afterEach(() => { + delete (globalThis as any).__sentryReactRouterServerInstrumentationUsed; }); it('should wrap an action function with default options', async () => { @@ -107,4 +116,36 @@ describe('wrapServerAction', () => { await expect(wrappedAction(mockArgs)).rejects.toBe(mockError); }); + + it('should skip span creation and warn when instrumentation API is used', async () => { + // Reset modules to get a fresh copy with unset warning flag + vi.resetModules(); + // @ts-expect-error - Dynamic import for module reset works at runtime but vitest's typecheck doesn't fully support it + const { wrapServerAction: freshWrapServerAction } = await import('../../src/server/wrapServerAction'); + + // Set the global flag indicating instrumentation API is in use + (globalThis as any).__sentryReactRouterServerInstrumentationUsed = true; + + const mockActionFn = vi.fn().mockResolvedValue('result'); + const mockArgs = { request: new Request('http://test.com') } as ActionFunctionArgs; + + const wrappedAction = freshWrapServerAction({}, mockActionFn); + + // Call multiple times + await wrappedAction(mockArgs); + await wrappedAction(mockArgs); + await wrappedAction(mockArgs); + + // Should warn about redundant wrapper via debug.warn, but only once + expect(core.debug.warn).toHaveBeenCalledTimes(1); + expect(core.debug.warn).toHaveBeenCalledWith( + expect.stringContaining('wrapServerAction is redundant when using the instrumentation API'), + ); + + // Should not create spans (instrumentation API handles it) + expect(core.startSpan).not.toHaveBeenCalled(); + + // Should still execute the action function + expect(mockActionFn).toHaveBeenCalledTimes(3); + }); }); diff --git a/packages/react-router/test/server/wrapServerLoader.test.ts b/packages/react-router/test/server/wrapServerLoader.test.ts index b375d9b4da51..c4491a301bf7 100644 --- a/packages/react-router/test/server/wrapServerLoader.test.ts +++ b/packages/react-router/test/server/wrapServerLoader.test.ts @@ -1,6 +1,6 @@ import * as core from '@sentry/core'; import type { LoaderFunctionArgs } from 'react-router'; -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { wrapServerLoader } from '../../src/server/wrapServerLoader'; vi.mock('@sentry/core', async () => { @@ -9,12 +9,21 @@ vi.mock('@sentry/core', async () => { ...actual, startSpan: vi.fn(), flushIfServerless: vi.fn(), + debug: { + warn: vi.fn(), + }, }; }); describe('wrapServerLoader', () => { beforeEach(() => { vi.clearAllMocks(); + // Reset the global flag and warning state + delete (globalThis as any).__sentryReactRouterServerInstrumentationUsed; + }); + + afterEach(() => { + delete (globalThis as any).__sentryReactRouterServerInstrumentationUsed; }); it('should wrap a loader function with default options', async () => { @@ -107,4 +116,36 @@ describe('wrapServerLoader', () => { await expect(wrappedLoader(mockArgs)).rejects.toBe(mockError); }); + + it('should skip span creation and warn when instrumentation API is used', async () => { + // Reset modules to get a fresh copy with unset warning flag + vi.resetModules(); + // @ts-expect-error - Dynamic import for module reset works at runtime but vitest's typecheck doesn't fully support it + const { wrapServerLoader: freshWrapServerLoader } = await import('../../src/server/wrapServerLoader'); + + // Set the global flag indicating instrumentation API is in use + (globalThis as any).__sentryReactRouterServerInstrumentationUsed = true; + + const mockLoaderFn = vi.fn().mockResolvedValue('result'); + const mockArgs = { request: new Request('http://test.com') } as LoaderFunctionArgs; + + const wrappedLoader = freshWrapServerLoader({}, mockLoaderFn); + + // Call multiple times + await wrappedLoader(mockArgs); + await wrappedLoader(mockArgs); + await wrappedLoader(mockArgs); + + // Should warn about redundant wrapper via debug.warn, but only once + expect(core.debug.warn).toHaveBeenCalledTimes(1); + expect(core.debug.warn).toHaveBeenCalledWith( + expect.stringContaining('wrapServerLoader is redundant when using the instrumentation API'), + ); + + // Should not create spans (instrumentation API handles it) + expect(core.startSpan).not.toHaveBeenCalled(); + + // Should still execute the loader function + expect(mockLoaderFn).toHaveBeenCalledTimes(3); + }); });