diff --git a/packages/sveltekit/src/client/browserTracingIntegration.ts b/packages/sveltekit/src/client/browserTracingIntegration.ts new file mode 100644 index 000000000000..9968f8b6de5f --- /dev/null +++ b/packages/sveltekit/src/client/browserTracingIntegration.ts @@ -0,0 +1,14 @@ +import { BrowserTracing as OriginalBrowserTracing } from '@sentry/svelte'; +import { svelteKitRoutingInstrumentation } from './router'; + +/** + * A custom BrowserTracing integration for Sveltekit. + */ +export class BrowserTracing extends OriginalBrowserTracing { + public constructor(options?: ConstructorParameters[0]) { + super({ + routingInstrumentation: svelteKitRoutingInstrumentation, + ...options, + }); + } +} diff --git a/packages/sveltekit/src/client/sdk.ts b/packages/sveltekit/src/client/sdk.ts index daf8cd3ed7d2..7b9c608a862d 100644 --- a/packages/sveltekit/src/client/sdk.ts +++ b/packages/sveltekit/src/client/sdk.ts @@ -1,9 +1,10 @@ import { applySdkMetadata, hasTracingEnabled } from '@sentry/core'; import type { BrowserOptions } from '@sentry/svelte'; -import { BrowserTracing, WINDOW, getCurrentScope, init as initSvelteSdk } from '@sentry/svelte'; -import { addOrUpdateIntegration } from '@sentry/utils'; +import { getDefaultIntegrations as getDefaultSvelteIntegrations } from '@sentry/svelte'; +import { WINDOW, getCurrentScope, init as initSvelteSdk } from '@sentry/svelte'; +import type { Integration } from '@sentry/types'; -import { svelteKitRoutingInstrumentation } from './router'; +import { BrowserTracing } from './browserTracingIntegration'; type WindowWithSentryFetchProxy = typeof WINDOW & { _sentryFetchProxy?: typeof fetch; @@ -18,15 +19,20 @@ declare const __SENTRY_TRACING__: boolean; * @param options Configuration options for the SDK. */ export function init(options: BrowserOptions): void { - applySdkMetadata(options, 'sveltekit', ['sveltekit', 'svelte']); + const opts = { + defaultIntegrations: getDefaultIntegrations(options), + ...options, + }; - addClientIntegrations(options); + applySdkMetadata(opts, 'sveltekit', ['sveltekit', 'svelte']); + + fixBrowserTracingIntegration(opts); // 1. Switch window.fetch to our fetch proxy we injected earlier const actualFetch = switchToFetchProxy(); // 2. Initialize the SDK which will instrument our proxy - initSvelteSdk(options); + initSvelteSdk(opts); // 3. Restore the original fetch now that our proxy is instrumented if (actualFetch) { @@ -36,24 +42,49 @@ export function init(options: BrowserOptions): void { getCurrentScope().setTag('runtime', 'browser'); } -function addClientIntegrations(options: BrowserOptions): void { - let integrations = options.integrations || []; +// TODO v8: Remove this again +// We need to handle BrowserTracing passed to `integrations` that comes from `@sentry/tracing`, not `@sentry/sveltekit` :( +function fixBrowserTracingIntegration(options: BrowserOptions): void { + const { integrations } = options; + if (!integrations) { + return; + } + + if (Array.isArray(integrations)) { + options.integrations = maybeUpdateBrowserTracingIntegration(integrations); + } else { + options.integrations = defaultIntegrations => { + const userFinalIntegrations = integrations(defaultIntegrations); + + return maybeUpdateBrowserTracingIntegration(userFinalIntegrations); + }; + } +} + +function maybeUpdateBrowserTracingIntegration(integrations: Integration[]): Integration[] { + const browserTracing = integrations.find(integration => integration.name === 'BrowserTracing'); + // If BrowserTracing was added, but it is not our forked version, + // replace it with our forked version with the same options + if (browserTracing && !(browserTracing instanceof BrowserTracing)) { + const options: ConstructorParameters[0] = (browserTracing as BrowserTracing).options; + // This option is overwritten by the custom integration + delete options.routingInstrumentation; + integrations[integrations.indexOf(browserTracing)] = new BrowserTracing(options); + } + + return integrations; +} - // This evaluates to true unless __SENTRY_TRACING__ is text-replaced with "false", - // in which case everything inside will get treeshaken away +function getDefaultIntegrations(options: BrowserOptions): Integration[] | undefined { + // This evaluates to true unless __SENTRY_TRACING__ is text-replaced with "false", in which case everything inside + // will get treeshaken away if (typeof __SENTRY_TRACING__ === 'undefined' || __SENTRY_TRACING__) { if (hasTracingEnabled(options)) { - const defaultBrowserTracingIntegration = new BrowserTracing({ - routingInstrumentation: svelteKitRoutingInstrumentation, - }); - - integrations = addOrUpdateIntegration(defaultBrowserTracingIntegration, integrations, { - 'options.routingInstrumentation': svelteKitRoutingInstrumentation, - }); + return [...getDefaultSvelteIntegrations(options), new BrowserTracing()]; } } - options.integrations = integrations; + return undefined; } /** diff --git a/packages/sveltekit/src/server/rewriteFramesIntegration.ts b/packages/sveltekit/src/server/rewriteFramesIntegration.ts new file mode 100644 index 000000000000..9187630e96ec --- /dev/null +++ b/packages/sveltekit/src/server/rewriteFramesIntegration.ts @@ -0,0 +1,77 @@ +import { defineIntegration } from '@sentry/core'; +import { rewriteFramesIntegration as originalRewriteFramesIntegration } from '@sentry/integrations'; +import type { IntegrationFn, StackFrame } from '@sentry/types'; +import { GLOBAL_OBJ, basename, escapeStringForRegex, join } from '@sentry/utils'; +import { WRAPPED_MODULE_SUFFIX } from '../vite/autoInstrument'; +import type { GlobalWithSentryValues } from '../vite/injectGlobalValues'; + +type StackFrameIteratee = (frame: StackFrame) => StackFrame; +interface RewriteFramesOptions { + root?: string; + prefix?: string; + iteratee?: StackFrameIteratee; +} + +export const customRewriteFramesIntegration = ((options?: RewriteFramesOptions) => { + return originalRewriteFramesIntegration({ + iteratee: rewriteFramesIteratee, + ...options, + }); +}) satisfies IntegrationFn; + +export const rewriteFramesIntegration = defineIntegration(customRewriteFramesIntegration); + +/** + * A custom iteratee function for the `RewriteFrames` integration. + * + * Does the same as the default iteratee, but also removes the `module` property from the + * frame to improve issue grouping. + * + * For some reason, our stack trace processing pipeline isn't able to resolve the bundled + * module name to the original file name correctly, leading to individual error groups for + * each module. Removing the `module` field makes the grouping algorithm fall back to the + * `filename` field, which is correctly resolved and hence grouping works as expected. + * + * Exported for tests only. + */ +export function rewriteFramesIteratee(frame: StackFrame): StackFrame { + if (!frame.filename) { + return frame; + } + const globalWithSentryValues: GlobalWithSentryValues = GLOBAL_OBJ; + const svelteKitBuildOutDir = globalWithSentryValues.__sentry_sveltekit_output_dir; + const prefix = 'app:///'; + + // Check if the frame filename begins with `/` or a Windows-style prefix such as `C:\` + const isWindowsFrame = /^[a-zA-Z]:\\/.test(frame.filename); + const startsWithSlash = /^\//.test(frame.filename); + if (isWindowsFrame || startsWithSlash) { + const filename = isWindowsFrame + ? frame.filename + .replace(/^[a-zA-Z]:/, '') // remove Windows-style prefix + .replace(/\\/g, '/') // replace all `\\` instances with `/` + : frame.filename; + + let strippedFilename; + if (svelteKitBuildOutDir) { + strippedFilename = filename.replace( + // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- not end user input + escaped anyway + new RegExp(`^.*${escapeStringForRegex(join(svelteKitBuildOutDir, 'server'))}/`), + '', + ); + } else { + strippedFilename = basename(filename); + } + frame.filename = `${prefix}${strippedFilename}`; + } + + delete frame.module; + + // In dev-mode, the WRAPPED_MODULE_SUFFIX is still present in the frame's file name. + // We need to remove it to make sure that the frame's filename matches the actual file + if (frame.filename.endsWith(WRAPPED_MODULE_SUFFIX)) { + frame.filename = frame.filename.slice(0, -WRAPPED_MODULE_SUFFIX.length); + } + + return frame; +} diff --git a/packages/sveltekit/src/server/sdk.ts b/packages/sveltekit/src/server/sdk.ts index 1001e922a031..c6b3bd26dffe 100644 --- a/packages/sveltekit/src/server/sdk.ts +++ b/packages/sveltekit/src/server/sdk.ts @@ -1,29 +1,23 @@ import { applySdkMetadata, getCurrentScope } from '@sentry/core'; -import { RewriteFrames } from '@sentry/integrations'; import type { NodeOptions } from '@sentry/node'; +import { getDefaultIntegrations as getDefaultNodeIntegrations } from '@sentry/node'; import { init as initNodeSdk } from '@sentry/node'; -import { addOrUpdateIntegration } from '@sentry/utils'; -import { rewriteFramesIteratee } from './utils'; +import { rewriteFramesIntegration } from './rewriteFramesIntegration'; /** * * @param options */ export function init(options: NodeOptions): void { - applySdkMetadata(options, 'sveltekit', ['sveltekit', 'node']); + const opts = { + defaultIntegrations: [...getDefaultNodeIntegrations(options), rewriteFramesIntegration()], + ...options, + }; - addServerIntegrations(options); + applySdkMetadata(opts, 'sveltekit', ['sveltekit', 'node']); - initNodeSdk(options); + initNodeSdk(opts); getCurrentScope().setTag('runtime', 'node'); } - -function addServerIntegrations(options: NodeOptions): void { - options.integrations = addOrUpdateIntegration( - // eslint-disable-next-line deprecation/deprecation - new RewriteFrames({ iteratee: rewriteFramesIteratee }), - options.integrations || [], - ); -} diff --git a/packages/sveltekit/src/server/utils.ts b/packages/sveltekit/src/server/utils.ts index de4444f28992..4106f7f4a09c 100644 --- a/packages/sveltekit/src/server/utils.ts +++ b/packages/sveltekit/src/server/utils.ts @@ -1,11 +1,8 @@ import { flush } from '@sentry/node'; -import type { StackFrame } from '@sentry/types'; -import { GLOBAL_OBJ, basename, escapeStringForRegex, join, logger, tracingContextFromHeaders } from '@sentry/utils'; +import { logger, tracingContextFromHeaders } from '@sentry/utils'; import type { RequestEvent } from '@sveltejs/kit'; import { DEBUG_BUILD } from '../common/debug-build'; -import { WRAPPED_MODULE_SUFFIX } from '../vite/autoInstrument'; -import type { GlobalWithSentryValues } from '../vite/injectGlobalValues'; /** * Takes a request event and extracts traceparent and DSC data @@ -19,59 +16,6 @@ export function getTracePropagationData(event: RequestEvent): ReturnType { const platformSupportsStreaming = !process.env.LAMBDA_TASK_ROOT && !process.env.VERCEL; diff --git a/packages/sveltekit/test/client/sdk.test.ts b/packages/sveltekit/test/client/sdk.test.ts index 28f053823e3b..10292658bc54 100644 --- a/packages/sveltekit/test/client/sdk.test.ts +++ b/packages/sveltekit/test/client/sdk.test.ts @@ -61,10 +61,7 @@ describe('Sentry client SDK', () => { ...tracingOptions, }); - const integrationsToInit = svelteInit.mock.calls[0][0].integrations; const browserTracing = getClient()?.getIntegrationByName('BrowserTracing'); - - expect(integrationsToInit).toContainEqual(expect.objectContaining({ name: 'BrowserTracing' })); expect(browserTracing).toBeDefined(); }); @@ -77,10 +74,7 @@ describe('Sentry client SDK', () => { ...tracingOptions, }); - const integrationsToInit = svelteInit.mock.calls[0][0].integrations; const browserTracing = getClient()?.getIntegrationByName('BrowserTracing'); - - expect(integrationsToInit).not.toContainEqual(expect.objectContaining({ name: 'BrowserTracing' })); expect(browserTracing).toBeUndefined(); }); @@ -96,10 +90,7 @@ describe('Sentry client SDK', () => { enableTracing: true, }); - const integrationsToInit = svelteInit.mock.calls[0][0].integrations; const browserTracing = getClient()?.getIntegrationByName('BrowserTracing'); - - expect(integrationsToInit).not.toContainEqual(expect.objectContaining({ name: 'BrowserTracing' })); expect(browserTracing).toBeUndefined(); // @ts-expect-error this is fine in the test @@ -113,12 +104,9 @@ describe('Sentry client SDK', () => { enableTracing: true, }); - const integrationsToInit = svelteInit.mock.calls[0][0].integrations; - const browserTracing = getClient()?.getIntegrationByName('BrowserTracing') as BrowserTracing; const options = browserTracing.options; - expect(integrationsToInit).toContainEqual(expect.objectContaining({ name: 'BrowserTracing' })); expect(browserTracing).toBeDefined(); // This shows that the user-configured options are still here diff --git a/packages/sveltekit/test/server/rewriteFramesIntegration.ts b/packages/sveltekit/test/server/rewriteFramesIntegration.ts new file mode 100644 index 000000000000..b2acdcf9ccaf --- /dev/null +++ b/packages/sveltekit/test/server/rewriteFramesIntegration.ts @@ -0,0 +1,88 @@ +import { RewriteFrames } from '@sentry/integrations'; +import type { Event, StackFrame } from '@sentry/types'; +import { basename } from '@sentry/utils'; + +import { rewriteFramesIteratee } from '../../src/server/rewriteFramesIntegration'; +import type { GlobalWithSentryValues } from '../../src/vite/injectGlobalValues'; + +describe('rewriteFramesIteratee', () => { + it('removes the module property from the frame', () => { + const frame: StackFrame = { + filename: '/some/path/to/server/chunks/3-ab34d22f.js', + module: '3-ab34d22f.js', + }; + + const result = rewriteFramesIteratee(frame); + + expect(result).not.toHaveProperty('module'); + }); + + it('does the same filename modification as the default RewriteFrames iteratee if no output dir is available', () => { + const frame: StackFrame = { + filename: '/some/path/to/server/chunks/3-ab34d22f.js', + lineno: 1, + colno: 1, + module: '3-ab34d22f.js', + }; + + // eslint-disable-next-line deprecation/deprecation + const originalRewriteFrames = new RewriteFrames(); + // eslint-disable-next-line deprecation/deprecation + const rewriteFrames = new RewriteFrames({ iteratee: rewriteFramesIteratee }); + + const event: Event = { + exception: { + values: [ + { + stacktrace: { + frames: [frame], + }, + }, + ], + }, + }; + + const originalResult = originalRewriteFrames.processEvent(event); + const result = rewriteFrames.processEvent(event); + + expect(result.exception?.values?.[0]?.stacktrace?.frames?.[0]).toEqual({ + filename: 'app:///3-ab34d22f.js', + lineno: 1, + colno: 1, + }); + + expect(result).toStrictEqual(originalResult); + }); + + it.each([ + ['adapter-node', 'build', '/absolute/path/to/build/server/chunks/3-ab34d22f.js', 'app:///chunks/3-ab34d22f.js'], + [ + 'adapter-auto', + '.svelte-kit/output', + '/absolute/path/to/.svelte-kit/output/server/entries/pages/page.ts.js', + 'app:///entries/pages/page.ts.js', + ], + ])( + 'removes the absolut path to the server output dir, if the output dir is available (%s)', + (_, outputDir, frameFilename, modifiedFilename) => { + (globalThis as unknown as GlobalWithSentryValues).__sentry_sveltekit_output_dir = outputDir; + + const frame: StackFrame = { + filename: frameFilename, + lineno: 1, + colno: 1, + module: basename(frameFilename), + }; + + const result = rewriteFramesIteratee({ ...frame }); + + expect(result).toStrictEqual({ + filename: modifiedFilename, + lineno: 1, + colno: 1, + }); + + delete (globalThis as unknown as GlobalWithSentryValues).__sentry_sveltekit_output_dir; + }, + ); +}); diff --git a/packages/sveltekit/test/server/sdk.test.ts b/packages/sveltekit/test/server/sdk.test.ts index b4ecf75a744a..99dcb37516e1 100644 --- a/packages/sveltekit/test/server/sdk.test.ts +++ b/packages/sveltekit/test/server/sdk.test.ts @@ -1,5 +1,6 @@ import * as SentryNode from '@sentry/node'; -import { SDK_VERSION } from '@sentry/node'; +import type { NodeClient } from '@sentry/node'; +import { SDK_VERSION, getClient } from '@sentry/node'; import { GLOBAL_OBJ } from '@sentry/utils'; import { init } from '../../src/server/sdk'; @@ -46,5 +47,14 @@ describe('Sentry server SDK', () => { // @ts-expect-error need access to protected _tags attribute expect(currentScope._tags).toEqual({ runtime: 'node' }); }); + + it('adds rewriteFramesIntegration by default', () => { + init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + }); + + const rewriteFramesIntegration = getClient()?.getIntegrationByName('RewriteFrames'); + expect(rewriteFramesIntegration).toBeDefined(); + }); }); }); diff --git a/packages/sveltekit/test/server/utils.test.ts b/packages/sveltekit/test/server/utils.test.ts index aa2f9be62f94..8e5c064c338c 100644 --- a/packages/sveltekit/test/server/utils.test.ts +++ b/packages/sveltekit/test/server/utils.test.ts @@ -1,9 +1,4 @@ -import { RewriteFrames } from '@sentry/integrations'; -import type { Event, StackFrame } from '@sentry/types'; -import { basename } from '@sentry/utils'; - -import type { GlobalWithSentryValues } from '../../src/server/utils'; -import { getTracePropagationData, rewriteFramesIteratee } from '../../src/server/utils'; +import { getTracePropagationData } from '../../src/server/utils'; const MOCK_REQUEST_EVENT: any = { request: { @@ -58,85 +53,3 @@ describe('getTracePropagationData', () => { expect(dynamicSamplingContext).toBeUndefined(); }); }); - -describe('rewriteFramesIteratee', () => { - it('removes the module property from the frame', () => { - const frame: StackFrame = { - filename: '/some/path/to/server/chunks/3-ab34d22f.js', - module: '3-ab34d22f.js', - }; - - const result = rewriteFramesIteratee(frame); - - expect(result).not.toHaveProperty('module'); - }); - - it('does the same filename modification as the default RewriteFrames iteratee if no output dir is available', () => { - const frame: StackFrame = { - filename: '/some/path/to/server/chunks/3-ab34d22f.js', - lineno: 1, - colno: 1, - module: '3-ab34d22f.js', - }; - - // eslint-disable-next-line deprecation/deprecation - const originalRewriteFrames = new RewriteFrames(); - // eslint-disable-next-line deprecation/deprecation - const rewriteFrames = new RewriteFrames({ iteratee: rewriteFramesIteratee }); - - const event: Event = { - exception: { - values: [ - { - stacktrace: { - frames: [frame], - }, - }, - ], - }, - }; - - const originalResult = originalRewriteFrames.processEvent(event); - const result = rewriteFrames.processEvent(event); - - expect(result.exception?.values?.[0]?.stacktrace?.frames?.[0]).toEqual({ - filename: 'app:///3-ab34d22f.js', - lineno: 1, - colno: 1, - }); - - expect(result).toStrictEqual(originalResult); - }); - - it.each([ - ['adapter-node', 'build', '/absolute/path/to/build/server/chunks/3-ab34d22f.js', 'app:///chunks/3-ab34d22f.js'], - [ - 'adapter-auto', - '.svelte-kit/output', - '/absolute/path/to/.svelte-kit/output/server/entries/pages/page.ts.js', - 'app:///entries/pages/page.ts.js', - ], - ])( - 'removes the absolut path to the server output dir, if the output dir is available (%s)', - (_, outputDir, frameFilename, modifiedFilename) => { - (globalThis as GlobalWithSentryValues).__sentry_sveltekit_output_dir = outputDir; - - const frame: StackFrame = { - filename: frameFilename, - lineno: 1, - colno: 1, - module: basename(frameFilename), - }; - - const result = rewriteFramesIteratee({ ...frame }); - - expect(result).toStrictEqual({ - filename: modifiedFilename, - lineno: 1, - colno: 1, - }); - - delete (globalThis as GlobalWithSentryValues).__sentry_sveltekit_output_dir; - }, - ); -}); diff --git a/packages/utils/src/userIntegrations.ts b/packages/utils/src/userIntegrations.ts index d897284d4c0d..345c43ef04d7 100644 --- a/packages/utils/src/userIntegrations.ts +++ b/packages/utils/src/userIntegrations.ts @@ -50,6 +50,8 @@ function setNestedKey(obj: Record, keyPath: string, value: unknown) * @param userIntegrations Integrations defined by the user. * @param forcedOptions Options with which to patch an existing user-derived instance on the integration. * @returns A final integrations array. + * + * @deprecated This will be removed in v8. */ export function addOrUpdateIntegration( defaultIntegrationInstance: Integration, diff --git a/packages/utils/test/userIntegrations.test.ts b/packages/utils/test/userIntegrations.test.ts index a009378064f1..96782fc414d6 100644 --- a/packages/utils/test/userIntegrations.test.ts +++ b/packages/utils/test/userIntegrations.test.ts @@ -76,12 +76,14 @@ function runTest(testOptions: { let integrations; if (typeof userIntegrations === 'function') { + // eslint-disable-next-line deprecation/deprecation const wrappedUserIntegrationsFunction = addOrUpdateIntegration(forcedDogIntegrationInstance, userIntegrations, { dogName: 'Charlie', descriptor: 'goofy', }); integrations = wrappedUserIntegrationsFunction(underlyingDefaultIntegrations); } else { + // eslint-disable-next-line deprecation/deprecation integrations = addOrUpdateIntegration( forcedDogIntegrationInstance, userIntegrations,