From 43fb0131cdcca27c1ada5ed4abae99bb64a5784f Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Mon, 6 Dec 2021 20:31:07 -0800 Subject: [PATCH 1/5] only call instrumentServer at runtime and off of vercel --- packages/nextjs/src/index.server.ts | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/packages/nextjs/src/index.server.ts b/packages/nextjs/src/index.server.ts index 6fd4e1fce468..9266c5767092 100644 --- a/packages/nextjs/src/index.server.ts +++ b/packages/nextjs/src/index.server.ts @@ -6,7 +6,6 @@ import { escapeStringForRegex, logger } from '@sentry/utils'; import * as domainModule from 'domain'; import * as path from 'path'; -import { instrumentServer } from './utils/instrumentServer'; import { MetadataBuilder } from './utils/metadataBuilder'; import { NextjsOptions } from './utils/nextjsOptions'; import { addIntegration } from './utils/userIntegrations'; @@ -20,6 +19,17 @@ export { ErrorBoundary, withErrorBoundary } from '@sentry/react'; type GlobalWithDistDir = typeof global & { __rewriteFramesDistDir__: string }; const domain = domainModule as typeof domainModule & { active: (domainModule.Domain & Carrier) | null }; +// During build, the main process is invoked by +// `node next build` +// and child processes are invoked as +// `node /node_modules/jest-worker/build/workers/processChild.js`, +// whereas at runtime the process is invoked as +// `node next start` +// or +// `node /var/runtime/index.js`. +const isBuild = new RegExp('build').test(process.argv.toString()); +const isVercel = !!process.env.VERCEL; + /** Inits the Sentry NextJS SDK on node. */ export function init(options: NextjsOptions): void { if (options.debug) { @@ -54,7 +64,7 @@ export function init(options: NextjsOptions): void { configureScope(scope => { scope.setTag('runtime', 'node'); - if (process.env.VERCEL) { + if (isVercel) { scope.setTag('vercel', true); } @@ -119,5 +129,13 @@ function filterTransactions(event: Event): Event | null { export { withSentryConfig } from './config'; export { withSentry } from './utils/withSentry'; -// wrap various server methods to enable error monitoring and tracing -instrumentServer(); +// Wrap various server methods to enable error monitoring and tracing. (Note: This only happens for non-vercel +// deployments, because the current method of doing the wrapping a) crashes next 12 apps deployed to vercel doesn't and +// b) doesn't work on those apps anyway. We also don't do it during build, because there's no server running in that +// phase.) +if (!isVercel && !isBuild) { + // we have to dynamically require the file because even importing from it causes next 12 to crash on vercel + // eslint-disable-next-line @typescript-eslint/no-var-requires + const { instrumentServer } = require('./utils/instrumentServer.js'); + instrumentServer(); +} From 586f120208c0b7b672bfcc485a65403a11ae2621 Mon Sep 17 00:00:00 2001 From: iker barriocanal <32816711+iker-barriocanal@users.noreply.github.com> Date: Fri, 10 Dec 2021 12:47:03 +0100 Subject: [PATCH 2/5] Remove test to check `vercel` tag when running on vercel The condition to check whether the `vercel` tag is added to vercel is resolved in `index.server.ts` at the module level. The `VERCEL` env var should be set (or not) when the module is imported, instead of importing it and then initing the app. --- packages/nextjs/test/index.server.test.ts | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/packages/nextjs/test/index.server.test.ts b/packages/nextjs/test/index.server.test.ts index d3fc53a194ed..3280118c52e7 100644 --- a/packages/nextjs/test/index.server.test.ts +++ b/packages/nextjs/test/index.server.test.ts @@ -66,18 +66,9 @@ describe('Server init()', () => { expect(currentScope._tags).toEqual({ runtime: 'node' }); }); - it('applies `vercel` tag when running on vercel', () => { - const currentScope = getCurrentHub().getScope(); - - process.env.VERCEL = '1'; - - init({}); - - // @ts-ignore need access to protected _tags attribute - expect(currentScope._tags.vercel).toEqual(true); - - delete process.env.VERCEL; - }); + // TODO: test `vercel` tag when running on Vercel + // Can't just add the test and set env variables, since the value in `index.server.ts` + // is resolved when importing. it('does not apply `vercel` tag when not running on vercel', () => { const currentScope = getCurrentHub().getScope(); From 2aa7519db86da756c30f4674d0767f81585d21b8 Mon Sep 17 00:00:00 2001 From: iker barriocanal <32816711+iker-barriocanal@users.noreply.github.com> Date: Fri, 10 Dec 2021 12:50:24 +0100 Subject: [PATCH 3/5] Delete `VERCEL` env var after each unit test --- packages/nextjs/test/index.server.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/nextjs/test/index.server.test.ts b/packages/nextjs/test/index.server.test.ts index 3280118c52e7..41d721c0daaf 100644 --- a/packages/nextjs/test/index.server.test.ts +++ b/packages/nextjs/test/index.server.test.ts @@ -24,6 +24,7 @@ describe('Server init()', () => { afterEach(() => { jest.clearAllMocks(); global.__SENTRY__.hub = undefined; + delete process.env.VERCEL; }); it('inits the Node SDK', () => { From 9fa8b1b301edb6f5406a4ee36d785b3f50d1a90e Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Fri, 10 Dec 2021 13:13:02 +0100 Subject: [PATCH 4/5] Clean up comments --- packages/nextjs/src/index.server.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/nextjs/src/index.server.ts b/packages/nextjs/src/index.server.ts index 9266c5767092..5e6123b70d39 100644 --- a/packages/nextjs/src/index.server.ts +++ b/packages/nextjs/src/index.server.ts @@ -129,12 +129,12 @@ function filterTransactions(event: Event): Event | null { export { withSentryConfig } from './config'; export { withSentry } from './utils/withSentry'; -// Wrap various server methods to enable error monitoring and tracing. (Note: This only happens for non-vercel -// deployments, because the current method of doing the wrapping a) crashes next 12 apps deployed to vercel doesn't and +// Wrap various server methods to enable error monitoring and tracing. (Note: This only happens for non-Vercel +// deployments, because the current method of doing the wrapping a) crashes Next 12 apps deployed to Vercel and // b) doesn't work on those apps anyway. We also don't do it during build, because there's no server running in that // phase.) if (!isVercel && !isBuild) { - // we have to dynamically require the file because even importing from it causes next 12 to crash on vercel + // we have to dynamically require the file because even importing from it causes Next 12 to crash on Vercel // eslint-disable-next-line @typescript-eslint/no-var-requires const { instrumentServer } = require('./utils/instrumentServer.js'); instrumentServer(); From ededc3f7e6d5a733ff9b42fa706809a0f77d1255 Mon Sep 17 00:00:00 2001 From: iker barriocanal <32816711+iker-barriocanal@users.noreply.github.com> Date: Fri, 10 Dec 2021 13:14:42 +0100 Subject: [PATCH 5/5] Import TS file if JS file doesn't exist, on server instrumentation --- packages/nextjs/src/index.server.ts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/packages/nextjs/src/index.server.ts b/packages/nextjs/src/index.server.ts index 9266c5767092..2d1587cd7ad4 100644 --- a/packages/nextjs/src/index.server.ts +++ b/packages/nextjs/src/index.server.ts @@ -134,8 +134,19 @@ export { withSentry } from './utils/withSentry'; // b) doesn't work on those apps anyway. We also don't do it during build, because there's no server running in that // phase.) if (!isVercel && !isBuild) { - // we have to dynamically require the file because even importing from it causes next 12 to crash on vercel - // eslint-disable-next-line @typescript-eslint/no-var-requires - const { instrumentServer } = require('./utils/instrumentServer.js'); - instrumentServer(); + // Dynamically require the file because even importing from it causes Next 12 to crash on Vercel. + // In environments where the JS file doesn't exist, such as testing, import the TS file. + try { + // eslint-disable-next-line @typescript-eslint/no-var-requires + const { instrumentServer } = require('./utils/instrumentServer.js'); + instrumentServer(); + } catch { + try { + // eslint-disable-next-line @typescript-eslint/no-var-requires + const { instrumentServer } = require('./utils/instrumentServer.ts'); + instrumentServer(); + } catch { + // Server not instrumented. Not adding logs to avoid noise. + } + } }