From 6bdda331b4f521dc3a6f113e6498e53101907e89 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 6 Nov 2023 11:44:02 +0000 Subject: [PATCH 1/3] feat(nextjs): Add automatic sourcemapping for Next.js edge --- packages/bun/src/client.ts | 2 +- .../core/test/lib/serverruntimeclient.test.ts | 4 +-- packages/deno/src/client.ts | 2 +- .../deno/test/__snapshots__/mod.test.ts.snap | 4 +-- packages/nextjs/src/config/webpack.ts | 31 +++++++++---------- packages/nextjs/src/edge/index.ts | 23 ++++++++++++++ packages/nextjs/src/server/index.ts | 25 +++++++-------- packages/vercel-edge/src/client.ts | 2 +- 8 files changed, 55 insertions(+), 38 deletions(-) diff --git a/packages/bun/src/client.ts b/packages/bun/src/client.ts index b8cebbe7463e..27d6cebd0630 100644 --- a/packages/bun/src/client.ts +++ b/packages/bun/src/client.ts @@ -30,7 +30,7 @@ export class BunClient extends ServerRuntimeClient { const clientOptions: ServerRuntimeClientOptions = { ...options, - platform: 'bun', + platform: 'javascript', runtime: { name: 'bun', version: Bun.version }, serverName: options.serverName || global.process.env.SENTRY_NAME || os.hostname(), }; diff --git a/packages/core/test/lib/serverruntimeclient.test.ts b/packages/core/test/lib/serverruntimeclient.test.ts index a0cd2e5a3f96..4ffed6c68f81 100644 --- a/packages/core/test/lib/serverruntimeclient.test.ts +++ b/packages/core/test/lib/serverruntimeclient.test.ts @@ -22,13 +22,13 @@ describe('ServerRuntimeClient', () => { describe('_prepareEvent', () => { test('adds platform to event', () => { const options = getDefaultClientOptions({ dsn: PUBLIC_DSN }); - const client = new ServerRuntimeClient({ ...options, platform: 'edge' }); + const client = new ServerRuntimeClient({ ...options, platform: 'blargh' }); const event: Event = {}; const hint: EventHint = {}; (client as any)._prepareEvent(event, hint); - expect(event.platform).toEqual('edge'); + expect(event.platform).toEqual('blargh'); }); test('adds server_name to event', () => { diff --git a/packages/deno/src/client.ts b/packages/deno/src/client.ts index 3eb7db655428..1db45a5cb960 100644 --- a/packages/deno/src/client.ts +++ b/packages/deno/src/client.ts @@ -34,7 +34,7 @@ export class DenoClient extends ServerRuntimeClient { const clientOptions: ServerRuntimeClientOptions = { ...options, - platform: 'deno', + platform: 'javascript', runtime: { name: 'deno', version: Deno.version.deno }, serverName: options.serverName || getHostName(), }; diff --git a/packages/deno/test/__snapshots__/mod.test.ts.snap b/packages/deno/test/__snapshots__/mod.test.ts.snap index 67823708e07e..dfa86aa5fd72 100644 --- a/packages/deno/test/__snapshots__/mod.test.ts.snap +++ b/packages/deno/test/__snapshots__/mod.test.ts.snap @@ -135,7 +135,7 @@ snapshot[`captureException 1`] = ` }, ], }, - platform: "deno", + platform: "javascript", sdk: { integrations: [ "InboundFilters", @@ -195,7 +195,7 @@ snapshot[`captureMessage 1`] = ` event_id: "{{id}}", level: "info", message: "Some error message", - platform: "deno", + platform: "javascript", sdk: { integrations: [ "InboundFilters", diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index f4dd1d7ecfae..61c6852936e0 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -686,24 +686,20 @@ export function getWebpackPluginOptions( userPluginOptions: Partial, userSentryOptions: UserSentryOptions, ): SentryWebpackPluginOptions { - const { buildId, isServer, webpack, config, dir: projectDir } = buildContext; + const { buildId, isServer, config, dir: projectDir } = buildContext; const userNextConfig = config as NextConfigObject; const distDirAbsPath = path.resolve(projectDir, userNextConfig.distDir || '.next'); // `.next` is the default directory - const isWebpack5 = webpack.version.startsWith('5'); const isServerless = userNextConfig.target === 'experimental-serverless-trace'; const hasSentryProperties = fs.existsSync(path.resolve(projectDir, 'sentry.properties')); const urlPrefix = '~/_next'; const serverInclude = isServerless ? [{ paths: [`${distDirAbsPath}/serverless/`], urlPrefix: `${urlPrefix}/serverless` }] - : [ - { paths: [`${distDirAbsPath}/server/pages/`], urlPrefix: `${urlPrefix}/server/pages` }, - { paths: [`${distDirAbsPath}/server/app/`], urlPrefix: `${urlPrefix}/server/app` }, - ].concat( - isWebpack5 ? [{ paths: [`${distDirAbsPath}/server/chunks/`], urlPrefix: `${urlPrefix}/server/chunks` }] : [], - ); + : [{ paths: [`${distDirAbsPath}/server/`], urlPrefix: `${urlPrefix}/server` }]; + + const serverIgnore: string[] = []; const clientInclude = userSentryOptions.widenClientFileUpload ? [{ paths: [`${distDirAbsPath}/static/chunks`], urlPrefix: `${urlPrefix}/static/chunks` }] @@ -712,15 +708,16 @@ export function getWebpackPluginOptions( { paths: [`${distDirAbsPath}/static/chunks/app`], urlPrefix: `${urlPrefix}/static/chunks/app` }, ]; + // Widening the upload scope is necessarily going to lead to us uploading files we don't need to (ones which + // don't include any user code). In order to lessen that where we can, exclude the internal nextjs files we know + // will be there. + const clientIgnore = userSentryOptions.widenClientFileUpload + ? ['framework-*', 'framework.*', 'main-*', 'polyfills-*', 'webpack-*'] + : []; + const defaultPluginOptions = dropUndefinedKeys({ include: isServer ? serverInclude : clientInclude, - ignore: - isServer || !userSentryOptions.widenClientFileUpload - ? [] - : // Widening the upload scope is necessarily going to lead to us uploading files we don't need to (ones which - // don't include any user code). In order to lessen that where we can, exclude the internal nextjs files we know - // will be there. - ['framework-*', 'framework.*', 'main-*', 'polyfills-*', 'webpack-*'], + ignore: isServer ? serverIgnore : clientIgnore, url: process.env.SENTRY_URL, org: process.env.SENTRY_ORG, project: process.env.SENTRY_PROJECT, @@ -961,7 +958,7 @@ function addValueInjectionLoader( ...isomorphicValues, // Make sure that if we have a windows path, the backslashes are interpreted as such (rather than as escape // characters) - __rewriteFramesDistDir__: userNextConfig.distDir?.replace(/\\/g, '\\\\') || '.next', + __rewriteFramesDistDir__: path.resolve(userNextConfig.distDir?.replace(/\\/g, '\\\\') || '.next'), }; const clientValues = { @@ -975,7 +972,7 @@ function addValueInjectionLoader( newConfig.module.rules.push( { - test: /sentry\.server\.config\.(jsx?|tsx?)/, + test: /sentry\.(server|edge)\.config\.(jsx?|tsx?)/, use: [ { loader: path.resolve(__dirname, 'loaders/valueInjectionLoader.js'), diff --git a/packages/nextjs/src/edge/index.ts b/packages/nextjs/src/edge/index.ts index 336a84b6b85c..5edea797853d 100644 --- a/packages/nextjs/src/edge/index.ts +++ b/packages/nextjs/src/edge/index.ts @@ -1,10 +1,16 @@ import { SDK_VERSION } from '@sentry/core'; +import { RewriteFrames } from '@sentry/integrations'; import type { SdkMetadata } from '@sentry/types'; +import { addOrUpdateIntegration, GLOBAL_OBJ } from '@sentry/utils'; import type { VercelEdgeOptions } from '@sentry/vercel-edge'; import { init as vercelEdgeInit } from '@sentry/vercel-edge'; export type EdgeOptions = VercelEdgeOptions; +const globalWithInjectedValues = GLOBAL_OBJ as typeof GLOBAL_OBJ & { + __rewriteFramesDistDir__?: string; +}; + /** Inits the Sentry NextJS SDK on the Edge Runtime. */ export function init(options: VercelEdgeOptions = {}): void { const opts = { @@ -23,6 +29,23 @@ export function init(options: VercelEdgeOptions = {}): void { version: SDK_VERSION, }; + let integrations = opts.integrations || []; + + // This value is injected at build time, based on the output directory specified in the build config. Though a default + // is set there, we set it here as well, just in case something has gone wrong with the injection. + const distDirName = globalWithInjectedValues.__rewriteFramesDistDir__; + if (distDirName) { + const defaultRewriteFramesIntegration = new RewriteFrames({ + iteratee: frame => { + frame.filename = frame.filename?.replace(distDirName, 'app:///_next'); + return frame; + }, + }); + integrations = addOrUpdateIntegration(defaultRewriteFramesIntegration, integrations); + } + + opts.integrations = integrations; + vercelEdgeInit(opts); } diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts index 1734f9ae24da..9205689739e9 100644 --- a/packages/nextjs/src/server/index.ts +++ b/packages/nextjs/src/server/index.ts @@ -50,7 +50,7 @@ export function showReportDialog(): void { } const globalWithInjectedValues = global as typeof global & { - __rewriteFramesDistDir__: string; + __rewriteFramesDistDir__?: string; }; // TODO (v8): Remove this @@ -119,19 +119,16 @@ function addServerIntegrations(options: NodeOptions): void { // This value is injected at build time, based on the output directory specified in the build config. Though a default // is set there, we set it here as well, just in case something has gone wrong with the injection. - const distDirName = globalWithInjectedValues.__rewriteFramesDistDir__ || '.next'; - // nextjs always puts the build directory at the project root level, which is also where you run `next start` from, so - // we can read in the project directory from the currently running process - const distDirAbsPath = path.resolve(process.cwd(), distDirName); - const SOURCEMAP_FILENAME_REGEX = new RegExp(escapeStringForRegex(distDirAbsPath)); - - const defaultRewriteFramesIntegration = new RewriteFrames({ - iteratee: frame => { - frame.filename = frame.filename?.replace(SOURCEMAP_FILENAME_REGEX, 'app:///_next'); - return frame; - }, - }); - integrations = addOrUpdateIntegration(defaultRewriteFramesIntegration, integrations); + const distDirName = globalWithInjectedValues.__rewriteFramesDistDir__; + if (distDirName) { + const defaultRewriteFramesIntegration = new RewriteFrames({ + iteratee: frame => { + frame.filename = frame.filename?.replace(distDirName, 'app:///_next'); + return frame; + }, + }); + integrations = addOrUpdateIntegration(defaultRewriteFramesIntegration, integrations); + } const defaultOnUncaughtExceptionIntegration: IntegrationWithExclusionOption = new Integrations.OnUncaughtException({ exitEvenIfOtherHandlersAreRegistered: false, diff --git a/packages/vercel-edge/src/client.ts b/packages/vercel-edge/src/client.ts index 448ecb199dce..2ded776719b4 100644 --- a/packages/vercel-edge/src/client.ts +++ b/packages/vercel-edge/src/client.ts @@ -33,7 +33,7 @@ export class VercelEdgeClient extends ServerRuntimeClient Date: Mon, 6 Nov 2023 12:23:30 +0000 Subject: [PATCH 2/3] ci --- packages/nextjs/src/server/index.ts | 3 +-- packages/nextjs/test/config/loaders.test.ts | 2 +- .../nextjs/test/config/webpack/sentryWebpackPlugin.test.ts | 7 ++----- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts index 9205689739e9..705c57cbde1c 100644 --- a/packages/nextjs/src/server/index.ts +++ b/packages/nextjs/src/server/index.ts @@ -3,8 +3,7 @@ import type { NodeOptions } from '@sentry/node'; import { configureScope, getCurrentHub, init as nodeInit, Integrations } from '@sentry/node'; import type { EventProcessor } from '@sentry/types'; import type { IntegrationWithExclusionOption } from '@sentry/utils'; -import { addOrUpdateIntegration, escapeStringForRegex, logger } from '@sentry/utils'; -import * as path from 'path'; +import { addOrUpdateIntegration, logger } from '@sentry/utils'; import { devErrorSymbolicationEventProcessor } from '../common/devErrorSymbolicationEventProcessor'; import { getVercelEnv } from '../common/getVercelEnv'; diff --git a/packages/nextjs/test/config/loaders.test.ts b/packages/nextjs/test/config/loaders.test.ts index 94a6a5ec0654..62616c58c3a1 100644 --- a/packages/nextjs/test/config/loaders.test.ts +++ b/packages/nextjs/test/config/loaders.test.ts @@ -72,7 +72,7 @@ describe('webpack loaders', () => { }); expect(finalWebpackConfig.module.rules).toContainEqual({ - test: /sentry\.server\.config\.(jsx?|tsx?)/, + test: /sentry\.(server|edge)\.config\.(jsx?|tsx?)/, use: [ { loader: expect.stringEndingWith('valueInjectionLoader.js'), diff --git a/packages/nextjs/test/config/webpack/sentryWebpackPlugin.test.ts b/packages/nextjs/test/config/webpack/sentryWebpackPlugin.test.ts index 1ec6af7212a1..8f5c7a3923ad 100644 --- a/packages/nextjs/test/config/webpack/sentryWebpackPlugin.test.ts +++ b/packages/nextjs/test/config/webpack/sentryWebpackPlugin.test.ts @@ -141,8 +141,7 @@ describe('Sentry webpack plugin config', () => { ) as SentryWebpackPlugin; expect(sentryWebpackPluginInstance.options.include).toEqual([ - { paths: [`${serverBuildContextWebpack4.dir}/.next/server/pages/`], urlPrefix: '~/_next/server/pages' }, - { paths: [`${serverBuildContextWebpack4.dir}/.next/server/app/`], urlPrefix: '~/_next/server/app' }, + { paths: [`${serverBuildContextWebpack4.dir}/.next/server/`], urlPrefix: '~/_next/server' }, ]); }); @@ -159,9 +158,7 @@ describe('Sentry webpack plugin config', () => { ) as SentryWebpackPlugin; expect(sentryWebpackPluginInstance.options.include).toEqual([ - { paths: [`${serverBuildContext.dir}/.next/server/pages/`], urlPrefix: '~/_next/server/pages' }, - { paths: [`${serverBuildContext.dir}/.next/server/app/`], urlPrefix: '~/_next/server/app' }, - { paths: [`${serverBuildContext.dir}/.next/server/chunks/`], urlPrefix: '~/_next/server/chunks' }, + { paths: [`${serverBuildContext.dir}/.next/server/`], urlPrefix: '~/_next/server' }, ]); }); }); From 003607c0e8fb5434dc718ed72f2e494673c8436a Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 6 Nov 2023 13:58:26 +0000 Subject: [PATCH 3/3] hmm --- packages/nextjs/src/config/webpack.ts | 2 +- packages/nextjs/src/edge/index.ts | 11 +++++++++-- packages/nextjs/src/server/index.ts | 10 ++++++++-- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 61c6852936e0..d86493c4d3eb 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -958,7 +958,7 @@ function addValueInjectionLoader( ...isomorphicValues, // Make sure that if we have a windows path, the backslashes are interpreted as such (rather than as escape // characters) - __rewriteFramesDistDir__: path.resolve(userNextConfig.distDir?.replace(/\\/g, '\\\\') || '.next'), + __rewriteFramesDistDir__: userNextConfig.distDir?.replace(/\\/g, '\\\\') || '.next', }; const clientValues = { diff --git a/packages/nextjs/src/edge/index.ts b/packages/nextjs/src/edge/index.ts index 5edea797853d..07c6de85e92c 100644 --- a/packages/nextjs/src/edge/index.ts +++ b/packages/nextjs/src/edge/index.ts @@ -1,7 +1,7 @@ import { SDK_VERSION } from '@sentry/core'; import { RewriteFrames } from '@sentry/integrations'; import type { SdkMetadata } from '@sentry/types'; -import { addOrUpdateIntegration, GLOBAL_OBJ } from '@sentry/utils'; +import { addOrUpdateIntegration, escapeStringForRegex, GLOBAL_OBJ } from '@sentry/utils'; import type { VercelEdgeOptions } from '@sentry/vercel-edge'; import { init as vercelEdgeInit } from '@sentry/vercel-edge'; @@ -35,12 +35,19 @@ export function init(options: VercelEdgeOptions = {}): void { // is set there, we set it here as well, just in case something has gone wrong with the injection. const distDirName = globalWithInjectedValues.__rewriteFramesDistDir__; if (distDirName) { + const distDirAbsPath = distDirName.replace(/(\/|\\)$/, ''); // We strip trailing slashes because "app:///_next" also doesn't have one + + // Normally we would use `path.resolve` to obtain the absolute path we will strip from the stack frame to align with + // the uploaded artifacts, however we don't have access to that API in edge so we need to be a bit more lax. + const SOURCEMAP_FILENAME_REGEX = new RegExp(`.*${escapeStringForRegex(distDirAbsPath)}`); + const defaultRewriteFramesIntegration = new RewriteFrames({ iteratee: frame => { - frame.filename = frame.filename?.replace(distDirName, 'app:///_next'); + frame.filename = frame.filename?.replace(SOURCEMAP_FILENAME_REGEX, 'app:///_next'); return frame; }, }); + integrations = addOrUpdateIntegration(defaultRewriteFramesIntegration, integrations); } diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts index 705c57cbde1c..cfbe245ca936 100644 --- a/packages/nextjs/src/server/index.ts +++ b/packages/nextjs/src/server/index.ts @@ -3,7 +3,8 @@ import type { NodeOptions } from '@sentry/node'; import { configureScope, getCurrentHub, init as nodeInit, Integrations } from '@sentry/node'; import type { EventProcessor } from '@sentry/types'; import type { IntegrationWithExclusionOption } from '@sentry/utils'; -import { addOrUpdateIntegration, logger } from '@sentry/utils'; +import { addOrUpdateIntegration, escapeStringForRegex, logger } from '@sentry/utils'; +import * as path from 'path'; import { devErrorSymbolicationEventProcessor } from '../common/devErrorSymbolicationEventProcessor'; import { getVercelEnv } from '../common/getVercelEnv'; @@ -120,9 +121,14 @@ function addServerIntegrations(options: NodeOptions): void { // is set there, we set it here as well, just in case something has gone wrong with the injection. const distDirName = globalWithInjectedValues.__rewriteFramesDistDir__; if (distDirName) { + // nextjs always puts the build directory at the project root level, which is also where you run `next start` from, so + // we can read in the project directory from the currently running process + const distDirAbsPath = path.resolve(distDirName).replace(/(\/|\\)$/, ''); // We strip trailing slashes because "app:///_next" also doesn't have one + const SOURCEMAP_FILENAME_REGEX = new RegExp(escapeStringForRegex(distDirAbsPath)); + const defaultRewriteFramesIntegration = new RewriteFrames({ iteratee: frame => { - frame.filename = frame.filename?.replace(distDirName, 'app:///_next'); + frame.filename = frame.filename?.replace(SOURCEMAP_FILENAME_REGEX, 'app:///_next'); return frame; }, });