Skip to content

Commit 4c0481b

Browse files
mydeaAbhiPrasad
mydea
and
AbhiPrasad
authored
fix(next): Fix custom integrations (#10220)
The usage of this was not really working well to begin with, and even worse with the new functional integrations. Because if the user adds the integration themselves (e.g. `integrations: [new RewriteFrames()]`), it will not actually get the correct iteratee at all. Overall it is much cleaner anyhow to just fork the integrations properly and use them instead of the default one - then we can rely on the standard behavior of merging integrations etc. We need to do the same for basically all usages of `addOrUpdateIntegration`, as that actually does not work at all anymore with the functional integrations 😬 (and in many instances never really worked properly if users passed in a custom integration themselves). Co-authored-by: Abhijeet Prasad <[email protected]>
1 parent 33d1cb0 commit 4c0481b

12 files changed

+361
-230
lines changed

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { BrowserTracing as OriginalBrowserTracing, defaultRequestInstrumentationOptions } from '@sentry/react';
2+
import { nextRouterInstrumentation } from '../index.client';
3+
4+
/**
5+
* A custom BrowserTracing integration for Next.js.
6+
*/
7+
export class BrowserTracing extends OriginalBrowserTracing {
8+
public constructor(options?: ConstructorParameters<typeof OriginalBrowserTracing>[0]) {
9+
super({
10+
// eslint-disable-next-line deprecation/deprecation
11+
tracingOrigins:
12+
process.env.NODE_ENV === 'development'
13+
? [
14+
// Will match any URL that contains "localhost" but not "webpack.hot-update.json" - The webpack dev-server
15+
// has cors and it doesn't like extra headers when it's accessed from a different URL.
16+
// TODO(v8): Ideally we rework our tracePropagationTargets logic so this hack won't be necessary anymore (see issue #9764)
17+
/^(?=.*localhost)(?!.*webpack\.hot-update\.json).*/,
18+
/^\/(?!\/)/,
19+
]
20+
: // eslint-disable-next-line deprecation/deprecation
21+
[...defaultRequestInstrumentationOptions.tracingOrigins, /^(api\/)/],
22+
routingInstrumentation: nextRouterInstrumentation,
23+
...options,
24+
});
25+
}
26+
}

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

Lines changed: 52 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,28 @@
11
import { hasTracingEnabled } from '@sentry/core';
2-
import { RewriteFrames } from '@sentry/integrations';
32
import type { BrowserOptions } from '@sentry/react';
43
import {
5-
BrowserTracing,
6-
Integrations,
7-
defaultRequestInstrumentationOptions,
4+
Integrations as OriginalIntegrations,
85
getCurrentScope,
6+
getDefaultIntegrations as getReactDefaultIntegrations,
97
init as reactInit,
108
} from '@sentry/react';
11-
import type { EventProcessor } from '@sentry/types';
12-
import { addOrUpdateIntegration } from '@sentry/utils';
9+
import type { EventProcessor, Integration } from '@sentry/types';
1310

1411
import { devErrorSymbolicationEventProcessor } from '../common/devErrorSymbolicationEventProcessor';
1512
import { getVercelEnv } from '../common/getVercelEnv';
1613
import { buildMetadata } from '../common/metadata';
17-
import { nextRouterInstrumentation } from './routing/nextRoutingInstrumentation';
14+
import { BrowserTracing } from './browserTracingIntegration';
15+
import { rewriteFramesIntegration } from './rewriteFramesIntegration';
1816
import { applyTunnelRouteOption } from './tunnelRoute';
1917

2018
export * from '@sentry/react';
2119
export { nextRouterInstrumentation } from './routing/nextRoutingInstrumentation';
2220
export { captureUnderscoreErrorException } from '../common/_error';
2321

24-
export { Integrations };
22+
export const Integrations = {
23+
...OriginalIntegrations,
24+
BrowserTracing,
25+
};
2526

2627
// Previously we expected users to import `BrowserTracing` like this:
2728
//
@@ -33,27 +34,24 @@ export { Integrations };
3334
//
3435
// import { BrowserTracing } from '@sentry/nextjs';
3536
// const instance = new BrowserTracing();
36-
export { BrowserTracing };
37+
export { BrowserTracing, rewriteFramesIntegration };
3738

3839
// Treeshakable guard to remove all code related to tracing
3940
declare const __SENTRY_TRACING__: boolean;
4041

41-
const globalWithInjectedValues = global as typeof global & {
42-
__rewriteFramesAssetPrefixPath__: string;
43-
};
44-
4542
/** Inits the Sentry NextJS SDK on the browser with the React SDK. */
4643
export function init(options: BrowserOptions): void {
4744
const opts = {
4845
environment: getVercelEnv(true) || process.env.NODE_ENV,
46+
defaultIntegrations: getDefaultIntegrations(options),
4947
...options,
5048
};
5149

50+
fixBrowserTracingIntegration(opts);
51+
5252
applyTunnelRouteOption(opts);
5353
buildMetadata(opts, ['nextjs', 'react']);
5454

55-
addClientIntegrations(opts);
56-
5755
reactInit(opts);
5856

5957
const scope = getCurrentScope();
@@ -68,72 +66,53 @@ export function init(options: BrowserOptions): void {
6866
}
6967
}
7068

71-
function addClientIntegrations(options: BrowserOptions): void {
72-
let integrations = options.integrations || [];
73-
74-
// This value is injected at build time, based on the output directory specified in the build config. Though a default
75-
// is set there, we set it here as well, just in case something has gone wrong with the injection.
76-
const assetPrefixPath = globalWithInjectedValues.__rewriteFramesAssetPrefixPath__ || '';
77-
78-
// eslint-disable-next-line deprecation/deprecation
79-
const defaultRewriteFramesIntegration = new RewriteFrames({
80-
// Turn `<origin>/<path>/_next/static/...` into `app:///_next/static/...`
81-
iteratee: frame => {
82-
try {
83-
const { origin } = new URL(frame.filename as string);
84-
frame.filename = frame.filename?.replace(origin, 'app://').replace(assetPrefixPath, '');
85-
} catch (err) {
86-
// Filename wasn't a properly formed URL, so there's nothing we can do
87-
}
88-
89-
// We need to URI-decode the filename because Next.js has wildcard routes like "/users/[id].js" which show up as "/users/%5id%5.js" in Error stacktraces.
90-
// The corresponding sources that Next.js generates have proper brackets so we also need proper brackets in the frame so that source map resolving works.
91-
if (frame.filename && frame.filename.startsWith('app:///_next')) {
92-
frame.filename = decodeURI(frame.filename);
93-
}
94-
95-
if (
96-
frame.filename &&
97-
frame.filename.match(
98-
/^app:\/\/\/_next\/static\/chunks\/(main-|main-app-|polyfills-|webpack-|framework-|framework\.)[0-9a-f]+\.js$/,
99-
)
100-
) {
101-
// We don't care about these frames. It's Next.js internal code.
102-
frame.in_app = false;
103-
}
104-
105-
return frame;
106-
},
107-
});
108-
integrations = addOrUpdateIntegration(defaultRewriteFramesIntegration, integrations);
69+
// TODO v8: Remove this again
70+
// We need to handle BrowserTracing passed to `integrations` that comes from `@sentry/tracing`, not `@sentry/sveltekit` :(
71+
function fixBrowserTracingIntegration(options: BrowserOptions): void {
72+
const { integrations } = options;
73+
if (!integrations) {
74+
return;
75+
}
76+
77+
if (Array.isArray(integrations)) {
78+
options.integrations = maybeUpdateBrowserTracingIntegration(integrations);
79+
} else {
80+
options.integrations = defaultIntegrations => {
81+
const userFinalIntegrations = integrations(defaultIntegrations);
82+
83+
return maybeUpdateBrowserTracingIntegration(userFinalIntegrations);
84+
};
85+
}
86+
}
87+
88+
function maybeUpdateBrowserTracingIntegration(integrations: Integration[]): Integration[] {
89+
const browserTracing = integrations.find(integration => integration.name === 'BrowserTracing');
90+
// If BrowserTracing was added, but it is not our forked version,
91+
// replace it with our forked version with the same options
92+
if (browserTracing && !(browserTracing instanceof BrowserTracing)) {
93+
const options: ConstructorParameters<typeof BrowserTracing>[0] = (browserTracing as BrowserTracing).options;
94+
// These two options are overwritten by the custom integration
95+
delete options.routingInstrumentation;
96+
// eslint-disable-next-line deprecation/deprecation
97+
delete options.tracingOrigins;
98+
integrations[integrations.indexOf(browserTracing)] = new BrowserTracing(options);
99+
}
100+
101+
return integrations;
102+
}
103+
104+
function getDefaultIntegrations(options: BrowserOptions): Integration[] {
105+
const customDefaultIntegrations = [...getReactDefaultIntegrations(options), rewriteFramesIntegration()];
109106

110107
// This evaluates to true unless __SENTRY_TRACING__ is text-replaced with "false", in which case everything inside
111108
// will get treeshaken away
112109
if (typeof __SENTRY_TRACING__ === 'undefined' || __SENTRY_TRACING__) {
113110
if (hasTracingEnabled(options)) {
114-
const defaultBrowserTracingIntegration = new BrowserTracing({
115-
// eslint-disable-next-line deprecation/deprecation
116-
tracingOrigins:
117-
process.env.NODE_ENV === 'development'
118-
? [
119-
// Will match any URL that contains "localhost" but not "webpack.hot-update.json" - The webpack dev-server
120-
// has cors and it doesn't like extra headers when it's accessed from a different URL.
121-
// TODO(v8): Ideally we rework our tracePropagationTargets logic so this hack won't be necessary anymore (see issue #9764)
122-
/^(?=.*localhost)(?!.*webpack\.hot-update\.json).*/,
123-
/^\/(?!\/)/,
124-
]
125-
: // eslint-disable-next-line deprecation/deprecation
126-
[...defaultRequestInstrumentationOptions.tracingOrigins, /^(api\/)/],
127-
routingInstrumentation: nextRouterInstrumentation,
128-
});
129-
130-
integrations = addOrUpdateIntegration(defaultBrowserTracingIntegration, integrations, {
131-
'options.routingInstrumentation': nextRouterInstrumentation,
132-
});
111+
customDefaultIntegrations.push(new BrowserTracing());
133112
}
134113
}
135114

136-
options.integrations = integrations;
115+
return customDefaultIntegrations;
137116
}
138117

139118
/**

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

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import { defineIntegration } from '@sentry/core';
2+
import { rewriteFramesIntegration as originalRewriteFramesIntegration } from '@sentry/integrations';
3+
import type { IntegrationFn, StackFrame } from '@sentry/types';
4+
5+
const globalWithInjectedValues = global as typeof global & {
6+
__rewriteFramesAssetPrefixPath__: string;
7+
};
8+
9+
type StackFrameIteratee = (frame: StackFrame) => StackFrame;
10+
11+
interface RewriteFramesOptions {
12+
root?: string;
13+
prefix?: string;
14+
iteratee?: StackFrameIteratee;
15+
}
16+
17+
export const customRewriteFramesIntegration = ((options?: RewriteFramesOptions) => {
18+
// This value is injected at build time, based on the output directory specified in the build config. Though a default
19+
// is set there, we set it here as well, just in case something has gone wrong with the injection.
20+
const assetPrefixPath = globalWithInjectedValues.__rewriteFramesAssetPrefixPath__ || '';
21+
22+
return originalRewriteFramesIntegration({
23+
// Turn `<origin>/<path>/_next/static/...` into `app:///_next/static/...`
24+
iteratee: frame => {
25+
try {
26+
const { origin } = new URL(frame.filename as string);
27+
frame.filename = frame.filename?.replace(origin, 'app://').replace(assetPrefixPath, '');
28+
} catch (err) {
29+
// Filename wasn't a properly formed URL, so there's nothing we can do
30+
}
31+
32+
// We need to URI-decode the filename because Next.js has wildcard routes like "/users/[id].js" which show up as "/users/%5id%5.js" in Error stacktraces.
33+
// The corresponding sources that Next.js generates have proper brackets so we also need proper brackets in the frame so that source map resolving works.
34+
if (frame.filename && frame.filename.startsWith('app:///_next')) {
35+
frame.filename = decodeURI(frame.filename);
36+
}
37+
38+
if (
39+
frame.filename &&
40+
frame.filename.match(
41+
/^app:\/\/\/_next\/static\/chunks\/(main-|main-app-|polyfills-|webpack-|framework-|framework\.)[0-9a-f]+\.js$/,
42+
)
43+
) {
44+
// We don't care about these frames. It's Next.js internal code.
45+
frame.in_app = false;
46+
}
47+
48+
return frame;
49+
},
50+
...options,
51+
});
52+
}) satisfies IntegrationFn;
53+
54+
export const rewriteFramesIntegration = defineIntegration(customRewriteFramesIntegration);

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

Lines changed: 6 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,14 @@
11
import { SDK_VERSION, addTracingExtensions } from '@sentry/core';
2-
import { RewriteFrames } from '@sentry/integrations';
32
import type { SdkMetadata } from '@sentry/types';
4-
import { GLOBAL_OBJ, addOrUpdateIntegration, escapeStringForRegex } from '@sentry/utils';
53
import type { VercelEdgeOptions } from '@sentry/vercel-edge';
6-
import { init as vercelEdgeInit } from '@sentry/vercel-edge';
4+
import { getDefaultIntegrations, init as vercelEdgeInit } from '@sentry/vercel-edge';
75

86
import { isBuild } from '../common/utils/isBuild';
7+
import { rewriteFramesIntegration } from './rewriteFramesIntegration';
98

109
export type EdgeOptions = VercelEdgeOptions;
1110

12-
const globalWithInjectedValues = GLOBAL_OBJ as typeof GLOBAL_OBJ & {
13-
__rewriteFramesDistDir__?: string;
14-
fetch: (...args: unknown[]) => unknown;
15-
};
11+
export { rewriteFramesIntegration };
1612

1713
/** Inits the Sentry NextJS SDK on the Edge Runtime. */
1814
export function init(options: VercelEdgeOptions = {}): void {
@@ -22,8 +18,11 @@ export function init(options: VercelEdgeOptions = {}): void {
2218
return;
2319
}
2420

21+
const customDefaultIntegrations = [...getDefaultIntegrations(options), rewriteFramesIntegration()];
22+
2523
const opts = {
2624
_metadata: {} as SdkMetadata,
25+
defaultIntegrations: customDefaultIntegrations,
2726
...options,
2827
};
2928

@@ -38,32 +37,6 @@ export function init(options: VercelEdgeOptions = {}): void {
3837
version: SDK_VERSION,
3938
};
4039

41-
let integrations = opts.integrations || [];
42-
43-
// This value is injected at build time, based on the output directory specified in the build config. Though a default
44-
// is set there, we set it here as well, just in case something has gone wrong with the injection.
45-
const distDirName = globalWithInjectedValues.__rewriteFramesDistDir__;
46-
if (distDirName) {
47-
const distDirAbsPath = distDirName.replace(/(\/|\\)$/, ''); // We strip trailing slashes because "app:///_next" also doesn't have one
48-
49-
// Normally we would use `path.resolve` to obtain the absolute path we will strip from the stack frame to align with
50-
// the uploaded artifacts, however we don't have access to that API in edge so we need to be a bit more lax.
51-
// eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- user input is escaped
52-
const SOURCEMAP_FILENAME_REGEX = new RegExp(`.*${escapeStringForRegex(distDirAbsPath)}`);
53-
54-
// eslint-disable-next-line deprecation/deprecation
55-
const defaultRewriteFramesIntegration = new RewriteFrames({
56-
iteratee: frame => {
57-
frame.filename = frame.filename?.replace(SOURCEMAP_FILENAME_REGEX, 'app:///_next');
58-
return frame;
59-
},
60-
});
61-
62-
integrations = addOrUpdateIntegration(defaultRewriteFramesIntegration, integrations);
63-
}
64-
65-
opts.integrations = integrations;
66-
6740
vercelEdgeInit(opts);
6841
}
6942

‎packages/nextjs/src/edge/rewriteFramesIntegration.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import { defineIntegration } from '@sentry/core';
2+
import {
3+
RewriteFrames as OriginalRewriteFrames,
4+
rewriteFramesIntegration as originalRewriteFramesIntegration,
5+
} from '@sentry/integrations';
6+
import type { IntegrationFn, StackFrame } from '@sentry/types';
7+
import { GLOBAL_OBJ, escapeStringForRegex } from '@sentry/utils';
8+
9+
const globalWithInjectedValues = GLOBAL_OBJ as typeof GLOBAL_OBJ & {
10+
__rewriteFramesDistDir__?: string;
11+
};
12+
13+
type StackFrameIteratee = (frame: StackFrame) => StackFrame;
14+
interface RewriteFramesOptions {
15+
root?: string;
16+
prefix?: string;
17+
iteratee?: StackFrameIteratee;
18+
}
19+
20+
export const customRewriteFramesIntegration = ((options?: RewriteFramesOptions) => {
21+
// This value is injected at build time, based on the output directory specified in the build config. Though a default
22+
// is set there, we set it here as well, just in case something has gone wrong with the injection.
23+
const distDirName = globalWithInjectedValues.__rewriteFramesDistDir__;
24+
25+
if (distDirName) {
26+
const distDirAbsPath = distDirName.replace(/(\/|\\)$/, ''); // We strip trailing slashes because "app:///_next" also doesn't have one
27+
28+
// Normally we would use `path.resolve` to obtain the absolute path we will strip from the stack frame to align with
29+
// the uploaded artifacts, however we don't have access to that API in edge so we need to be a bit more lax.
30+
// eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- user input is escaped
31+
const SOURCEMAP_FILENAME_REGEX = new RegExp(`.*${escapeStringForRegex(distDirAbsPath)}`);
32+
33+
return originalRewriteFramesIntegration({
34+
iteratee: frame => {
35+
frame.filename = frame.filename?.replace(SOURCEMAP_FILENAME_REGEX, 'app:///_next');
36+
return frame;
37+
},
38+
...options,
39+
});
40+
}
41+
42+
// Do nothing if we can't find a distDirName
43+
return {
44+
// eslint-disable-next-line deprecation/deprecation
45+
name: OriginalRewriteFrames.id,
46+
// eslint-disable-next-line @typescript-eslint/no-empty-function
47+
setupOnce: () => {},
48+
processEvent: event => event,
49+
};
50+
}) satisfies IntegrationFn;
51+
52+
export const rewriteFramesIntegration = defineIntegration(customRewriteFramesIntegration);

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ export declare const defaultIntegrations: Integration[];
2828
export declare const getDefaultIntegrations: (options: Options) => Integration[];
2929
export declare const defaultStackParser: StackParser;
3030

31+
// eslint-disable-next-line deprecation/deprecation
32+
export declare const rewriteFramesIntegration: typeof clientSdk.rewriteFramesIntegration;
33+
3134
export declare function getSentryRelease(fallback?: string): string | undefined;
3235

3336
export declare const ErrorBoundary: typeof clientSdk.ErrorBoundary;

0 commit comments

Comments
 (0)