From f7b87a9e5a0d625acba4009db90e765419120e6c Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 4 Apr 2025 10:41:16 +0200 Subject: [PATCH 1/4] uh oh --- .../nextjs-turbo/instrumentation-client.ts | 2 + packages/nextjs/src/client/index.ts | 1 + .../appRouterRoutingInstrumentation.ts | 59 ++++++++++++++++--- 3 files changed, 53 insertions(+), 9 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nextjs-turbo/instrumentation-client.ts b/dev-packages/e2e-tests/test-applications/nextjs-turbo/instrumentation-client.ts index 85bd765c9c44..4870c64e7959 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-turbo/instrumentation-client.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-turbo/instrumentation-client.ts @@ -7,3 +7,5 @@ Sentry.init({ tracesSampleRate: 1.0, sendDefaultPii: true, }); + +export const onRouterTransitionStart = Sentry.captureRouterTransitionStart; diff --git a/packages/nextjs/src/client/index.ts b/packages/nextjs/src/client/index.ts index 9bc1c713525b..e8712a3dca65 100644 --- a/packages/nextjs/src/client/index.ts +++ b/packages/nextjs/src/client/index.ts @@ -16,6 +16,7 @@ export * from '@sentry/react'; export * from '../common'; export { captureUnderscoreErrorException } from '../common/pages-router-instrumentation/_error'; export { browserTracingIntegration } from './browserTracingIntegration'; +export { captureRouterTransitionStart } from './routing/appRouterRoutingInstrumentation'; let clientIsInitialized = false; diff --git a/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts b/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts index 11f3351dcd15..d02da812a171 100644 --- a/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts +++ b/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts @@ -9,6 +9,9 @@ import { WINDOW, startBrowserTracingNavigationSpan, startBrowserTracingPageLoadS export const INCOMPLETE_APP_ROUTER_INSTRUMENTATION_TRANSACTION_NAME = 'incomplete-app-router-transaction'; +let navigationRoutingMode: 'router-patch' | 'transition-start-hook' = 'router-patch'; +const currentRouterPatchingNavigationSpanRef: NavigationSpanRef = { current: undefined }; + /** Instruments the Next.js app router for pageloads. */ export function appRouterInstrumentPageLoad(client: Client): void { const origin = browserPerformanceTimeOrigin(); @@ -61,14 +64,39 @@ const GLOBAL_OBJ_WITH_NEXT_ROUTER = GLOBAL_OBJ as typeof GLOBAL_OBJ & { /** Instruments the Next.js app router for navigation. */ export function appRouterInstrumentNavigation(client: Client): void { - const currentNavigationSpanRef: NavigationSpanRef = { current: undefined }; + routerTransitionHandler = (href, navigationType) => { + const pathname = new URL(href, WINDOW.location.href).pathname; + + if (navigationRoutingMode === 'router-patch') { + navigationRoutingMode = 'transition-start-hook'; + } + + const currentNavigationSpan = currentRouterPatchingNavigationSpanRef.current; + if (currentNavigationSpan) { + currentNavigationSpan.updateName(pathname); + currentNavigationSpan.setAttributes({ + 'navigation.type': `router.${navigationType}`, + }); + currentRouterPatchingNavigationSpanRef.current = undefined; + } else { + startBrowserTracingNavigationSpan(client, { + name: pathname, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.nextjs.app_router_instrumentation', + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + 'navigation.type': `router.${navigationType}`, + }, + }); + } + }; WINDOW.addEventListener('popstate', () => { - if (currentNavigationSpanRef.current?.isRecording()) { - currentNavigationSpanRef.current.updateName(WINDOW.location.pathname); - currentNavigationSpanRef.current.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'url'); + if (currentRouterPatchingNavigationSpanRef.current?.isRecording()) { + currentRouterPatchingNavigationSpanRef.current.updateName(WINDOW.location.pathname); + currentRouterPatchingNavigationSpanRef.current.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'url'); } else { - currentNavigationSpanRef.current = startBrowserTracingNavigationSpan(client, { + currentRouterPatchingNavigationSpanRef.current = startBrowserTracingNavigationSpan(client, { name: WINDOW.location.pathname, attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', @@ -94,7 +122,7 @@ export function appRouterInstrumentNavigation(client: Client): void { clearInterval(checkForRouterAvailabilityInterval); routerPatched = true; - patchRouter(client, router, currentNavigationSpanRef); + patchRouter(client, router, currentRouterPatchingNavigationSpanRef); // If the router at any point gets overridden - patch again (['nd', 'next'] as const).forEach(globalValueName => { @@ -103,7 +131,7 @@ export function appRouterInstrumentNavigation(client: Client): void { GLOBAL_OBJ_WITH_NEXT_ROUTER[globalValueName] = new Proxy(globalValue, { set(target, p, newValue) { if (p === 'router' && typeof newValue === 'object' && newValue !== null) { - patchRouter(client, newValue, currentNavigationSpanRef); + patchRouter(client, newValue, currentRouterPatchingNavigationSpanRef); } // @ts-expect-error we cannot possibly type this @@ -139,6 +167,10 @@ function patchRouter(client: Client, router: NextRouter, currentNavigationSpanRe // @ts-expect-error Weird type error related to not knowing how to associate return values with the individual functions - we can just ignore router[routerFunctionName] = new Proxy(router[routerFunctionName], { apply(target, thisArg, argArray) { + if (navigationRoutingMode !== 'router-patch') { + return target.apply(thisArg, argArray); + } + let transactionName = INCOMPLETE_APP_ROUTER_INSTRUMENTATION_TRANSACTION_NAME; const transactionAttributes: Record = { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', @@ -148,11 +180,9 @@ function patchRouter(client: Client, router: NextRouter, currentNavigationSpanRe if (routerFunctionName === 'push') { transactionName = transactionNameifyRouterArgument(argArray[0]); - transactionAttributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] = 'url'; transactionAttributes['navigation.type'] = 'router.push'; } else if (routerFunctionName === 'replace') { transactionName = transactionNameifyRouterArgument(argArray[0]); - transactionAttributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] = 'url'; transactionAttributes['navigation.type'] = 'router.replace'; } else if (routerFunctionName === 'back') { transactionAttributes['navigation.type'] = 'router.back'; @@ -171,3 +201,14 @@ function patchRouter(client: Client, router: NextRouter, currentNavigationSpanRe } }); } + +let routerTransitionHandler: undefined | ((href: string, navigationType: string) => void) = undefined; + +/** + * A handler for Next.js' `onRouterTransitionStart` hook in `instrumentation-client.ts` to record navigation spans in Sentry. + */ +export function captureRouterTransitionStart(href: string, navigationType: string): void { + if (routerTransitionHandler) { + routerTransitionHandler(href, navigationType); + } +} From 192480592f68e129253542888b6e90258006ce0d Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 7 Apr 2025 11:04:40 +0200 Subject: [PATCH 2/4] Update tests --- .../instrumentation-client.ts} | 4 ++-- .../nextjs-13/sentry.client.config.ts | 2 ++ .../instrumentation-client.ts} | 4 ++-- .../nextjs-14/sentry.client.config.ts | 2 ++ .../nextjs-15/instrumentation-client.ts | 11 +++++++++++ .../nextjs-app-dir/instrumentation-client.ts | 11 +++++++++++ .../test-applications/nextjs-app-dir/next-env.d.ts | 2 +- .../tests/client-app-routing-instrumentation.test.ts | 9 ++++++--- 8 files changed, 37 insertions(+), 8 deletions(-) rename dev-packages/e2e-tests/test-applications/{nextjs-15/sentry.client.config.ts => nextjs-13/instrumentation-client.ts} (78%) rename dev-packages/e2e-tests/test-applications/{nextjs-app-dir/sentry.client.config.ts => nextjs-14/instrumentation-client.ts} (78%) create mode 100644 dev-packages/e2e-tests/test-applications/nextjs-15/instrumentation-client.ts create mode 100644 dev-packages/e2e-tests/test-applications/nextjs-app-dir/instrumentation-client.ts diff --git a/dev-packages/e2e-tests/test-applications/nextjs-15/sentry.client.config.ts b/dev-packages/e2e-tests/test-applications/nextjs-13/instrumentation-client.ts similarity index 78% rename from dev-packages/e2e-tests/test-applications/nextjs-15/sentry.client.config.ts rename to dev-packages/e2e-tests/test-applications/nextjs-13/instrumentation-client.ts index f2c7e4aef94d..4870c64e7959 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-15/sentry.client.config.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-13/instrumentation-client.ts @@ -1,5 +1,3 @@ -'use client'; - import * as Sentry from '@sentry/nextjs'; Sentry.init({ @@ -9,3 +7,5 @@ Sentry.init({ tracesSampleRate: 1.0, sendDefaultPii: true, }); + +export const onRouterTransitionStart = Sentry.captureRouterTransitionStart; diff --git a/dev-packages/e2e-tests/test-applications/nextjs-13/sentry.client.config.ts b/dev-packages/e2e-tests/test-applications/nextjs-13/sentry.client.config.ts index f2c7e4aef94d..b5d7387fa8ea 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-13/sentry.client.config.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-13/sentry.client.config.ts @@ -9,3 +9,5 @@ Sentry.init({ tracesSampleRate: 1.0, sendDefaultPii: true, }); + +export const onRouterTransitionStart = Sentry.captureRouterTransitionStart; diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/sentry.client.config.ts b/dev-packages/e2e-tests/test-applications/nextjs-14/instrumentation-client.ts similarity index 78% rename from dev-packages/e2e-tests/test-applications/nextjs-app-dir/sentry.client.config.ts rename to dev-packages/e2e-tests/test-applications/nextjs-14/instrumentation-client.ts index f2c7e4aef94d..4870c64e7959 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/sentry.client.config.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-14/instrumentation-client.ts @@ -1,5 +1,3 @@ -'use client'; - import * as Sentry from '@sentry/nextjs'; Sentry.init({ @@ -9,3 +7,5 @@ Sentry.init({ tracesSampleRate: 1.0, sendDefaultPii: true, }); + +export const onRouterTransitionStart = Sentry.captureRouterTransitionStart; diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/sentry.client.config.ts b/dev-packages/e2e-tests/test-applications/nextjs-14/sentry.client.config.ts index f2c7e4aef94d..b5d7387fa8ea 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-14/sentry.client.config.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-14/sentry.client.config.ts @@ -9,3 +9,5 @@ Sentry.init({ tracesSampleRate: 1.0, sendDefaultPii: true, }); + +export const onRouterTransitionStart = Sentry.captureRouterTransitionStart; diff --git a/dev-packages/e2e-tests/test-applications/nextjs-15/instrumentation-client.ts b/dev-packages/e2e-tests/test-applications/nextjs-15/instrumentation-client.ts new file mode 100644 index 000000000000..4870c64e7959 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-15/instrumentation-client.ts @@ -0,0 +1,11 @@ +import * as Sentry from '@sentry/nextjs'; + +Sentry.init({ + environment: 'qa', // dynamic sampling bias to keep transactions + dsn: process.env.NEXT_PUBLIC_E2E_TEST_DSN, + tunnel: `http://localhost:3031/`, // proxy server + tracesSampleRate: 1.0, + sendDefaultPii: true, +}); + +export const onRouterTransitionStart = Sentry.captureRouterTransitionStart; diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/instrumentation-client.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/instrumentation-client.ts new file mode 100644 index 000000000000..4870c64e7959 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/instrumentation-client.ts @@ -0,0 +1,11 @@ +import * as Sentry from '@sentry/nextjs'; + +Sentry.init({ + environment: 'qa', // dynamic sampling bias to keep transactions + dsn: process.env.NEXT_PUBLIC_E2E_TEST_DSN, + tunnel: `http://localhost:3031/`, // proxy server + tracesSampleRate: 1.0, + sendDefaultPii: true, +}); + +export const onRouterTransitionStart = Sentry.captureRouterTransitionStart; diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/next-env.d.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/next-env.d.ts index fd36f9494e2c..725dd6f24515 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/next-env.d.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/next-env.d.ts @@ -3,4 +3,4 @@ /// // NOTE: This file should not be edited -// see https://nextjs.org/docs/basic-features/typescript for more information. +// see https://nextjs.org/docs/app/building-your-application/configuring/typescript for more information. diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/client-app-routing-instrumentation.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/client-app-routing-instrumentation.test.ts index abfe9b323d0f..d5d4450bdaf2 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/client-app-routing-instrumentation.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/client-app-routing-instrumentation.test.ts @@ -118,7 +118,8 @@ test('Creates a navigation transaction for `router.forward()`', async ({ page }) return ( transactionEvent?.transaction === `/navigation/42/router-push` && transactionEvent.contexts?.trace?.op === 'navigation' && - transactionEvent.contexts.trace.data?.['navigation.type'] === 'router.forward' + (transactionEvent.contexts.trace.data?.['navigation.type'] === 'router.forward' || + transactionEvent.contexts.trace.data?.['navigation.type'] === 'router.traverse') ); }); @@ -169,7 +170,8 @@ test('Creates a navigation transaction for browser-back', async ({ page }) => { return ( transactionEvent?.transaction === `/navigation/42/browser-back` && transactionEvent.contexts?.trace?.op === 'navigation' && - transactionEvent.contexts.trace.data?.['navigation.type'] === 'browser.popstate' + (transactionEvent.contexts.trace.data?.['navigation.type'] === 'browser.popstate' || + transactionEvent.contexts.trace.data?.['navigation.type'] === 'router.traverse') ); }); @@ -187,7 +189,8 @@ test('Creates a navigation transaction for browser-forward', async ({ page }) => return ( transactionEvent?.transaction === `/navigation/42/router-push` && transactionEvent.contexts?.trace?.op === 'navigation' && - transactionEvent.contexts.trace.data?.['navigation.type'] === 'browser.popstate' + (transactionEvent.contexts.trace.data?.['navigation.type'] === 'browser.popstate' || + transactionEvent.contexts.trace.data?.['navigation.type'] === 'router.traverse') ); }); From 4a168d271bb56a3cac14c206a9fcc8eeca59db97 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 7 Apr 2025 11:33:54 +0200 Subject: [PATCH 3/4] . --- .../nextjs-13/sentry.client.config.ts | 13 -------- .../nextjs-14/sentry.client.config.ts | 13 -------- ...client-app-routing-instrumentation.test.ts | 2 +- .../appRouterRoutingInstrumentation.ts | 1 - packages/nextjs/src/config/webpack.ts | 2 +- .../nextjs/src/config/withSentryConfig.ts | 32 +++++++++++++++++++ 6 files changed, 34 insertions(+), 29 deletions(-) delete mode 100644 dev-packages/e2e-tests/test-applications/nextjs-13/sentry.client.config.ts delete mode 100644 dev-packages/e2e-tests/test-applications/nextjs-14/sentry.client.config.ts diff --git a/dev-packages/e2e-tests/test-applications/nextjs-13/sentry.client.config.ts b/dev-packages/e2e-tests/test-applications/nextjs-13/sentry.client.config.ts deleted file mode 100644 index b5d7387fa8ea..000000000000 --- a/dev-packages/e2e-tests/test-applications/nextjs-13/sentry.client.config.ts +++ /dev/null @@ -1,13 +0,0 @@ -'use client'; - -import * as Sentry from '@sentry/nextjs'; - -Sentry.init({ - environment: 'qa', // dynamic sampling bias to keep transactions - dsn: process.env.NEXT_PUBLIC_E2E_TEST_DSN, - tunnel: `http://localhost:3031/`, // proxy server - tracesSampleRate: 1.0, - sendDefaultPii: true, -}); - -export const onRouterTransitionStart = Sentry.captureRouterTransitionStart; diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/sentry.client.config.ts b/dev-packages/e2e-tests/test-applications/nextjs-14/sentry.client.config.ts deleted file mode 100644 index b5d7387fa8ea..000000000000 --- a/dev-packages/e2e-tests/test-applications/nextjs-14/sentry.client.config.ts +++ /dev/null @@ -1,13 +0,0 @@ -'use client'; - -import * as Sentry from '@sentry/nextjs'; - -Sentry.init({ - environment: 'qa', // dynamic sampling bias to keep transactions - dsn: process.env.NEXT_PUBLIC_E2E_TEST_DSN, - tunnel: `http://localhost:3031/`, // proxy server - tracesSampleRate: 1.0, - sendDefaultPii: true, -}); - -export const onRouterTransitionStart = Sentry.captureRouterTransitionStart; diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/client-app-routing-instrumentation.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/client-app-routing-instrumentation.test.ts index d5d4450bdaf2..6e5aedbeb1c6 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/client-app-routing-instrumentation.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/client-app-routing-instrumentation.test.ts @@ -106,7 +106,7 @@ test('Creates a navigation transaction for `router.back()`', async ({ page }) => contexts: { trace: { data: { - 'navigation.type': 'router.back', + 'navigation.type': expect.stringMatching(/router\.(back|traverse)/), // back is Next.js < 13.3.0, traverse >= 13.3.0 }, }, }, diff --git a/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts b/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts index d02da812a171..192ed13936d9 100644 --- a/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts +++ b/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts @@ -99,7 +99,6 @@ export function appRouterInstrumentNavigation(client: Client): void { currentRouterPatchingNavigationSpanRef.current = startBrowserTracingNavigationSpan(client, { name: WINDOW.location.pathname, attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.nextjs.app_router_instrumentation', [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', 'navigation.type': 'browser.popstate', diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 9155b0cd2b60..4ef3096adba6 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -344,7 +344,7 @@ export function constructWebpackConfigFunction( if (clientSentryConfigFileName) { // eslint-disable-next-line no-console console.warn( - `[@sentry/nextjs] DEPRECATION WARNING: It is recommended renaming your \`${clientSentryConfigFileName}\` file, or moving its content to \`instrumentation-client.ts\`. When using Turbopack \`${clientSentryConfigFileName}\` will no longer work. Read more about the \`instrumentation-client.ts\` file: https://nextjs.org/docs/app/api-reference/config/next-config-js/clientInstrumentationHook`, + `[@sentry/nextjs] ACTION REQUIRED: It is recommended renaming your \`${clientSentryConfigFileName}\` file, or moving its content to \`instrumentation-client.ts\`. When using Turbopack \`${clientSentryConfigFileName}\` will no longer work. Read more about the \`instrumentation-client.ts\` file: https://nextjs.org/docs/app/api-reference/config/next-config-js/clientInstrumentationHook`, ); } } diff --git a/packages/nextjs/src/config/withSentryConfig.ts b/packages/nextjs/src/config/withSentryConfig.ts index 4b24d8370393..72b85b452e6b 100644 --- a/packages/nextjs/src/config/withSentryConfig.ts +++ b/packages/nextjs/src/config/withSentryConfig.ts @@ -1,3 +1,4 @@ +/* eslint-disable max-lines */ /* eslint-disable complexity */ import { isThenable, parseSemver } from '@sentry/core'; @@ -12,6 +13,8 @@ import type { } from './types'; import { constructWebpackConfigFunction } from './webpack'; import { getNextjsVersion } from './util'; +import * as fs from 'fs'; +import * as path from 'path'; let showedExportModeTunnelWarning = false; @@ -155,6 +158,18 @@ function getFinalConfigObject( } } + // We wanna check whether the user added a `onRouterTransitionStart` handler to their client instrumentation file. + const instrumentationClientFileContents = getInstrumentationClientFileContents(); + if ( + instrumentationClientFileContents !== undefined && + !instrumentationClientFileContents.includes('onRouterTransitionStart') + ) { + // eslint-disable-next-line no-console + console.warn( + '[@sentry/nextjs] ACTION REQUIRED: To instrument navigations, the Sentry SDK requires you to export an `onRouterTransitionStart` hook from your `instrumentation-client.(js|ts)` file. You can do so by adding `export const onRouterTransitionStart = Sentry.captureRouterTransitionStart;` to the file.', + ); + } + if (nextJsVersion) { const { major, minor, patch, prerelease } = parseSemver(nextJsVersion); const isSupportedVersion = @@ -361,3 +376,20 @@ function getGitRevision(): string | undefined { } return gitRevision; } + +function getInstrumentationClientFileContents(): string | void { + const potentialInstrumentationClientFileLocations = [ + ['src', 'instrumentation-client.ts'], + ['src', 'instrumentation-client.js'], + ['instrumentation-client.ts'], + ['instrumentation-client.ts'], + ]; + + for (const pathSegments of potentialInstrumentationClientFileLocations) { + try { + return fs.readFileSync(path.join(process.cwd(), ...pathSegments), 'utf-8'); + } catch { + // noop + } + } +} From 263006b6f39ed59f14d5dcaeebc2e9d17ad11cf1 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 7 Apr 2025 13:10:23 +0200 Subject: [PATCH 4/4] lil bit of explanation --- ...client-app-routing-instrumentation.test.ts | 2 +- .../appRouterRoutingInstrumentation.ts | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/client-app-routing-instrumentation.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/client-app-routing-instrumentation.test.ts index 6e5aedbeb1c6..8069a1d1395b 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/client-app-routing-instrumentation.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/client-app-routing-instrumentation.test.ts @@ -106,7 +106,7 @@ test('Creates a navigation transaction for `router.back()`', async ({ page }) => contexts: { trace: { data: { - 'navigation.type': expect.stringMatching(/router\.(back|traverse)/), // back is Next.js < 13.3.0, traverse >= 13.3.0 + 'navigation.type': expect.stringMatching(/router\.(back|traverse)/), // back is Next.js < 15.3.0, traverse >= 15.3.0 }, }, }, diff --git a/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts b/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts index 192ed13936d9..55ed7dccc847 100644 --- a/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts +++ b/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts @@ -9,7 +9,26 @@ import { WINDOW, startBrowserTracingNavigationSpan, startBrowserTracingPageLoadS export const INCOMPLETE_APP_ROUTER_INSTRUMENTATION_TRANSACTION_NAME = 'incomplete-app-router-transaction'; +/** + * This mutable keeps track of what router navigation instrumentation mechanism we are using. + * + * The default one is 'router-patch' which is a way of instrumenting that worked up until Next.js 15.3.0 was released. + * For this method we took the global router instance and simply monkey patched all the router methods like push(), replace(), and so on. + * This worked because Next.js itself called the router methods for things like the component. + * Vercel decided that it is not good to call these public API methods from within the framework so they switched to an internal system that completely bypasses our monkey patching. This happened in 15.3.0. + * + * We raised with Vercel that this breaks our SDK so together with them we came up with an API for `instrumentation-client.ts` called `onRouterTransitionStart` that is called whenever a navigation is kicked off. + * + * Now we have the problem of version compatibility. + * For older Next.js versions we cannot use the new hook so we need to always patch the router. + * For newer Next.js versions we cannot know whether the user actually registered our handler for the `onRouterTransitionStart` hook, so we need to wait until it was called at least once before switching the instrumentation mechanism. + * The problem is, that the user may still have registered a hook and then call a patched router method. + * First, the monkey patched router method will be called, starting a navigation span, then the hook will also called. + * We need to handle this case and not create two separate navigation spans but instead update the current navigation span and then switch to the new instrumentation mode. + * This is all denoted by this `navigationRoutingMode` variable. + */ let navigationRoutingMode: 'router-patch' | 'transition-start-hook' = 'router-patch'; + const currentRouterPatchingNavigationSpanRef: NavigationSpanRef = { current: undefined }; /** Instruments the Next.js app router for pageloads. */