Skip to content

Commit dff53f6

Browse files
committed
fix(nextjs): Register the route provider before the pageload span is named
The provider was registered after `reactInit` returned, but the pageload span is named in `browserTracingIntegration`'s `afterAllSetup`, which runs inside it. So every App Router pageload lost its parameterized name. Registering it from a default integration's `setup` runs it before any `afterAllSetup` while still not depending on tracing.
1 parent f56eb6d commit dff53f6

5 files changed

Lines changed: 78 additions & 11 deletions

File tree

‎packages/nextjs/src/client/index.ts‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
/* eslint-disable import/export */
44
import type { Client, EventProcessor, Integration } from '@sentry/core';
55
import { addEventProcessor, applySdkMetadata, consoleSandbox, getGlobalScope, GLOBAL_OBJ } from '@sentry/core';
6-
import { setRouteProvider } from '@sentry/core/browser';
76
import type { BrowserOptions } from '@sentry/react';
87
import { getDefaultIntegrations as getReactDefaultIntegrations, init as reactInit } from '@sentry/react';
98
import { DEBUG_BUILD } from '../common/debug-build';
@@ -13,7 +12,7 @@ import { isRedirectNavigationError } from '../common/nextNavigationErrorUtils';
1312
import { browserTracingIntegration } from './browserTracingIntegration';
1413
import { nextjsClientStackFrameNormalizationIntegration } from './clientNormalizationIntegration';
1514
import { removeIsrSsgTraceMetaTags } from './routing/isrRoutingTracing';
16-
import { createNextRouteProvider } from './routing/routeProvider';
15+
import { nextjsRouteProviderIntegration } from './routing/routeProvider';
1716
import { applyTunnelRouteOption } from './tunnelRoute';
1817

1918
export * from '@sentry/react';
@@ -81,11 +80,6 @@ export function init(options: BrowserOptions): Client | undefined {
8180

8281
const client = reactInit(opts);
8382

84-
// Registered here rather than from `browserTracingIntegration` so route parameterization does not
85-
// depend on tracing: the route manifests are injected at build time, so anything that needs a route
86-
// name (bfcache metrics, web vitals) can resolve one even with tracing disabled.
87-
setRouteProvider(createNextRouteProvider(), client);
88-
8983
const filterNextRedirectError: EventProcessor = (event, hint) =>
9084
isRedirectNavigationError(hint?.originalException) || event.exception?.values?.[0]?.value === 'NEXT_REDIRECT'
9185
? null
@@ -112,6 +106,7 @@ export function init(options: BrowserOptions): Client | undefined {
112106

113107
function getDefaultIntegrations(options: BrowserOptions): Integration[] {
114108
const customDefaultIntegrations = getReactDefaultIntegrations(options);
109+
customDefaultIntegrations.push(nextjsRouteProviderIntegration());
115110
// This evaluates to true unless __SENTRY_TRACING__ is text-replaced with "false",
116111
// in which case everything inside will get tree-shaken away
117112
if (typeof __SENTRY_TRACING__ === 'undefined' || __SENTRY_TRACING__) {

‎packages/nextjs/src/client/routing/routeProvider.ts‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1+
import { defineIntegration } from '@sentry/core';
12
import type { RouteProvider } from '@sentry/core/browser';
2-
import { createUrlRouteProvider } from '@sentry/core/browser';
3+
import { createUrlRouteProvider, setRouteProvider } from '@sentry/core/browser';
34
import { maybeParameterizeRoute, stripBasePath, stripTrailingSlash } from './parameterization';
45
import { getNextRouteFromPathname } from './pagesRouterRoutingInstrumentation';
56

@@ -24,3 +25,18 @@ function resolveNextRoute(url: URL): string | undefined {
2425
export function createNextRouteProvider(): RouteProvider {
2526
return createUrlRouteProvider(resolveNextRoute);
2627
}
28+
29+
/**
30+
* Registers the Next.js route provider.
31+
*
32+
* An integration rather than part of `browserTracingIntegration` so route parameterization does not depend
33+
* on tracing: the route manifests are injected at build time, so anything that needs a route name (bfcache
34+
* metrics, web vitals) can resolve one even with tracing disabled. Registered in `setup` because the pageload
35+
* span is named in `browserTracingIntegration`'s `afterAllSetup`.
36+
*/
37+
export const nextjsRouteProviderIntegration = defineIntegration(() => ({
38+
name: 'NextjsRouteProvider',
39+
setup(client) {
40+
setRouteProvider(createNextRouteProvider(), client);
41+
},
42+
}));

‎packages/nextjs/test/client/appRouterRoutingInstrumentation.test.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,13 @@ import '@sentry/core';
1111
import '@sentry/react';
1212
import '../../src/client/routing/appRouterRoutingInstrumentation';
1313
import type * as AppRouterInstrumentation from '../../src/client/routing/appRouterRoutingInstrumentation';
14+
import type * as RouteProvider from '../../src/client/routing/routeProvider';
1415
import type { RouteManifest } from '../../src/config/manifest/types';
1516

1617
type Core = typeof SentryCore;
1718
type React = typeof SentryReact;
1819
type Instrumentation = typeof AppRouterInstrumentation;
20+
type RouteProviderModule = typeof RouteProvider;
1921

2022
interface NextRouter {
2123
back: () => void;
@@ -61,14 +63,18 @@ async function setup(traceLifecycle: 'stream' | 'static'): Promise<{
6163
const core: Core = await import('@sentry/core');
6264
const react: React = await import('@sentry/react');
6365
const instrumentation: Instrumentation = await import('../../src/client/routing/appRouterRoutingInstrumentation');
66+
const routeProvider: RouteProviderModule = await import('../../src/client/routing/routeProvider');
6467

6568
const client = new react.BrowserClient({
6669
dsn: 'http://examplePublicKey@localhost/0',
6770
transport: () => core.createTransport({ recordDroppedEvent: () => undefined }, () => core.resolvedSyncPromise({})),
6871
stackParser: () => [],
6972
tracesSampleRate: 1,
7073
traceLifecycle,
71-
integrations: [react.browserTracingIntegration({ instrumentPageLoad: false, instrumentNavigation: false })],
74+
integrations: [
75+
routeProvider.nextjsRouteProviderIntegration(),
76+
react.browserTracingIntegration({ instrumentPageLoad: false, instrumentNavigation: false }),
77+
],
7278
});
7379
core.setCurrentClient(client);
7480
client.init();

‎packages/nextjs/test/client/routeProvider.test.ts‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,11 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest';
44
import { BrowserClient, setCurrentClient } from '@sentry/react';
55
import { createNextRouteProvider } from '../../src/client/routing/routeProvider';
66

7-
const globalWithManifest = GLOBAL_OBJ as typeof GLOBAL_OBJ & { _sentryRouteManifest?: string };
7+
const globalWithManifest = GLOBAL_OBJ as typeof GLOBAL_OBJ & {
8+
_sentryRouteManifest?: string;
9+
_sentryBasePath?: string;
10+
__BUILD_MANIFEST?: { sortedPages?: string[] };
11+
};
812

913
let originalDocument: unknown;
1014

@@ -43,6 +47,8 @@ describe('createNextRouteProvider', () => {
4347

4448
afterEach(() => {
4549
delete globalWithManifest._sentryRouteManifest;
50+
delete globalWithManifest._sentryBasePath;
51+
delete globalWithManifest.__BUILD_MANIFEST;
4652
(GLOBAL_OBJ as { document?: unknown }).document = originalDocument;
4753
});
4854

@@ -67,4 +73,26 @@ describe('createNextRouteProvider', () => {
6773

6874
expect(resolveRoute('https://example.com/nope/deep', client)).toBeUndefined();
6975
});
76+
77+
describe('Pages Router', () => {
78+
beforeEach(() => {
79+
globalWithManifest.__BUILD_MANIFEST = { sortedPages: ['/', '/_app', '/_error', '/posts/[slug]'] };
80+
});
81+
82+
it('falls back to the Pages Router manifest when the App Router manifest has no match', () => {
83+
const client = makeClient();
84+
setRouteProvider(createNextRouteProvider(), client);
85+
86+
expect(resolveRoute('https://example.com/posts/hello', client)).toBe('/posts/[slug]');
87+
});
88+
89+
it('strips `basePath` before matching, since Pages Router routes are generated without it', () => {
90+
globalWithManifest._sentryBasePath = '/docs';
91+
const client = makeClient();
92+
setRouteProvider(createNextRouteProvider(), client);
93+
94+
expect(resolveRoute('https://example.com/docs/posts/hello', client)).toBe('/posts/[slug]');
95+
expect(resolveRoute('https://example.com/docs', client)).toBe('/');
96+
});
97+
});
7098
});

‎packages/nextjs/test/clientSdk.test.ts‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { Integration } from '@sentry/core';
2-
import { debug, getMainCarrier, SentryNonRecordingSpan } from '@sentry/core';
2+
import { debug, getMainCarrier, GLOBAL_OBJ, SentryNonRecordingSpan, spanToJSON } from '@sentry/core';
33
import * as SentryReact from '@sentry/react';
44
import { getClient, WINDOW } from '@sentry/react';
55
import { JSDOM } from 'jsdom';
@@ -188,6 +188,28 @@ describe('Client init()', () => {
188188
delete globalThis.__SENTRY_TRACING__;
189189
});
190190

191+
it('names the pageload span after the parameterized route', () => {
192+
const globalWithManifest = GLOBAL_OBJ as typeof GLOBAL_OBJ & { _sentryRouteManifest?: string };
193+
globalWithManifest._sentryRouteManifest = JSON.stringify({
194+
staticRoutes: [{ path: '/' }],
195+
dynamicRoutes: [],
196+
isrRoutes: [],
197+
});
198+
199+
init({ dsn: TEST_DSN, tracesSampleRate: 1.0 });
200+
201+
expect(spanToJSON(SentryReact.getActiveSpan()!)).toMatchObject({
202+
name: '/',
203+
attributes: {
204+
'sentry.op': 'pageload',
205+
'sentry.segment.name.source': 'route',
206+
'url.template': '/',
207+
},
208+
});
209+
210+
delete globalWithManifest._sentryRouteManifest;
211+
});
212+
191213
it("doesn't run Next.js router instrumentation for bot user agents", () => {
192214
Object.defineProperty(WINDOW, 'navigator', {
193215
value: {

0 commit comments

Comments
 (0)