From 5f9e86e2f7ce82ec77de7c5434c386345c594ff6 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 24 Sep 2026 11:04:03 +0200 Subject: [PATCH] fix(nextjs): Don't prepend basePath to absolute router navigation URLs Only root-relative hrefs get the basePath prefix, mirroring Next.js' own addPathPrefix. Previously, absolute URLs passed to router.push/replace (e.g. from a redirect() to an absolute URL) produced span names like /my-apphttp://host/login. Fixes #24672 Co-Authored-By: Claude Opus 5.5 (1M context) --- .../app/navigation/page.tsx | 9 +++ .../tests/routing-basepath-span.test.ts | 18 ++++++ .../appRouterRoutingInstrumentation.ts | 6 +- .../appRouterRoutingInstrumentation.test.ts | 56 ++++++++++++++++++- 4 files changed, 86 insertions(+), 3 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nextjs-15-basepath/app/navigation/page.tsx b/dev-packages/e2e-tests/test-applications/nextjs-15-basepath/app/navigation/page.tsx index 918c03de3d0a..21bdce977a7f 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-15-basepath/app/navigation/page.tsx +++ b/dev-packages/e2e-tests/test-applications/nextjs-15-basepath/app/navigation/page.tsx @@ -17,6 +17,15 @@ export default function Page() { router.push() +
  • + +
  • Normal Link
    • diff --git a/dev-packages/e2e-tests/test-applications/nextjs-15-basepath/tests/routing-basepath-span.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-15-basepath/tests/routing-basepath-span.test.ts index fbdb962d4262..c3c62b908f2c 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-15-basepath/tests/routing-basepath-span.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-15-basepath/tests/routing-basepath-span.test.ts @@ -81,3 +81,21 @@ test('Creates a navigation span for basePath with prefix', async ({ page expect(await navigationSpanPromise).toBeDefined(); }); + +test('Does not prepend basePath to absolute router.push URLs', async ({ page }) => { + const navigationSpanPromise = waitForStreamedSpan('nextjs-15-basepath', span => { + return getSpanOp(span) === 'navigation' && span.is_segment; + }); + + await page.goto('/my-app/navigation'); + await page.waitForTimeout(1000); + await page.getByText('Absolute URL push').click(); + + const navigationSpan = await navigationSpanPromise; + + expect(navigationSpan.name).toBe('/my-app/navigation/:param/router-push'); + expect(navigationSpan.attributes).toMatchObject({ + 'sentry.segment.name.source': { value: 'route', type: 'string' }, + 'url.path': { value: '/my-app/navigation/42/router-push', type: 'string' }, + }); +}); diff --git a/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts b/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts index eb3edbf3eced..07258ff72416 100644 --- a/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts +++ b/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts @@ -156,7 +156,7 @@ const globalWithInjectedBasePath = GLOBAL_OBJ as typeof GLOBAL_OBJ & { export function appRouterInstrumentNavigation(client: Client): void { routerTransitionHandler = (href, navigationType) => { const basePath = process.env._sentryBasePath ?? globalWithInjectedBasePath._sentryBasePath; - const normalizedHref = basePath && !href.startsWith(basePath) ? `${basePath}${href}` : href; + const normalizedHref = basePath && href.startsWith('/') && !href.startsWith(basePath) ? `${basePath}${href}` : href; const unparameterizedPathname = stripTrailingSlash(new URL(normalizedHref, WINDOW.location.href).pathname); const parameterizedPathname = maybeParameterizeRoute(unparameterizedPathname); // With span streaming, span names have to be low cardinality, so we can't fall back to the URL. @@ -304,7 +304,9 @@ function patchRouter(client: Client, router: NextRouter, currentNavigationSpanRe const href = argArray[0]; const basePath = process.env._sentryBasePath ?? globalWithInjectedBasePath._sentryBasePath; const normalizedHref = - basePath && typeof href === 'string' && !href.startsWith(basePath) ? `${basePath}${href}` : href; + basePath && typeof href === 'string' && href.startsWith('/') && !href.startsWith(basePath) + ? `${basePath}${href}` + : href; const transactionName = stripTrailingSlash(transactionNameifyRouterArgument(normalizedHref)); const parameterizedPathname = maybeParameterizeRoute(transactionName); diff --git a/packages/nextjs/test/client/appRouterRoutingInstrumentation.test.ts b/packages/nextjs/test/client/appRouterRoutingInstrumentation.test.ts index e54e9867cc89..37adaa05c25a 100644 --- a/packages/nextjs/test/client/appRouterRoutingInstrumentation.test.ts +++ b/packages/nextjs/test/client/appRouterRoutingInstrumentation.test.ts @@ -27,6 +27,7 @@ interface NextRouter { const globalWithNext = globalThis as typeof globalThis & { next?: { router?: NextRouter }; _sentryRouteManifest?: string; + _sentryBasePath?: string; }; const manifest: RouteManifest = { @@ -38,6 +39,12 @@ const manifest: RouteManifest = { paramNames: ['param'], hasOptionalPrefix: false, }, + { + path: '/my-app/navigation/:param/router-push', + regex: '^/my-app/navigation/([^/]+)/router-push$', + paramNames: ['param'], + hasOptionalPrefix: false, + }, ], isrRoutes: [], }; @@ -56,6 +63,7 @@ async function setup(traceLifecycle: 'stream' | 'static'): Promise<{ core: Core; router: NextRouter; client: Client; + instrumentation: Instrumentation; }> { vi.resetModules(); const core: Core = await import('@sentry/core'); @@ -80,7 +88,7 @@ async function setup(traceLifecycle: 'stream' | 'static'): Promise<{ instrumentation.appRouterInstrumentNavigation(client); await vi.waitFor(() => expect(router.back).not.toBe(originalBack)); - return { core, router, client }; + return { core, router, client, instrumentation }; } describe('appRouterInstrumentNavigation (router-patch mode)', () => { @@ -207,3 +215,49 @@ describe('appRouterInstrumentNavigation (router-patch mode)', () => { }); }); }); + +describe('appRouterInstrumentNavigation with basePath', () => { + beforeEach(() => { + globalWithNext._sentryRouteManifest = JSON.stringify(manifest); + globalWithNext._sentryBasePath = '/my-app'; + window.history.replaceState({}, '', '/my-app/navigation'); + }); + + afterEach(() => { + delete globalWithNext.next; + delete globalWithNext._sentryRouteManifest; + delete globalWithNext._sentryBasePath; + }); + + it.each([ + ['a root-relative path without basePath', '/navigation/42/router-push'], + ['a root-relative path with basePath', '/my-app/navigation/42/router-push'], + ['an absolute URL', 'http://localhost:3000/my-app/navigation/42/router-push'], + ])('names the router-patch navigation span correctly for %s', async (_, href) => { + const { core, router } = await setup('static'); + + router.push(href); + + const span = core.getActiveSpan(); + expect(span).toBeDefined(); + const spanJson = core.spanToJSON(span!); + expect(spanJson.name).toBe('/my-app/navigation/:param/router-push'); + expect(spanJson.attributes).toEqual( + expect.objectContaining({ 'url.full': 'http://localhost:3000/my-app/navigation/42/router-push' }), + ); + }); + + it.each([ + ['a root-relative path without basePath', '/navigation/42/router-push'], + ['a root-relative path with basePath', '/my-app/navigation/42/router-push'], + ['an absolute URL', 'http://localhost:3000/my-app/navigation/42/router-push'], + ])('names the transition-start-hook navigation span correctly for %s', async (_, href) => { + const { core, instrumentation } = await setup('static'); + + instrumentation.captureRouterTransitionStart(href, 'push'); + + const span = core.getActiveSpan(); + expect(span).toBeDefined(); + expect(core.spanToJSON(span!).name).toBe('/my-app/navigation/:param/router-push'); + }); +});