Skip to content

Commit

Permalink
fix(nextjs): Disable server instrumentation on Vercel (#4255)
Browse files Browse the repository at this point in the history
On Vercel, the server instrumentation has no effect. Errors and transactions are captured through the [custom `_error.js` page](https://docs.sentry.io/platforms/javascript/guides/nextjs/manual-setup/#create-a-custom-_error-page) and the [`withSentry` handler](https://docs.sentry.io/platforms/javascript/guides/nextjs/#configure). Not instrumenting the server (to not import any modules from `next`) when an app is deployed on Vercel should fix the issue.
  • Loading branch information
iker-barriocanal authored Dec 10, 2021
1 parent 24b600f commit 7e7e5f2
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 16 deletions.
37 changes: 33 additions & 4 deletions packages/nextjs/src/index.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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 <path>/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) {
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -119,5 +129,24 @@ 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 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) {
// 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.
}
}
}
16 changes: 4 additions & 12 deletions packages/nextjs/test/index.server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ describe('Server init()', () => {
afterEach(() => {
jest.clearAllMocks();
global.__SENTRY__.hub = undefined;
delete process.env.VERCEL;
});

it('inits the Node SDK', () => {
Expand Down Expand Up @@ -66,18 +67,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();
Expand Down

0 comments on commit 7e7e5f2

Please sign in to comment.