ref(nextjs): Stop reinitializing the server SDK unnecessarily (#3860)

In the nextjs SDK, we bundle the user's `Sentry.init()` code with every API route and with the `_app` page, which is the basis of all user-provided pages. This is necessary because, when deployed on Vercel, each API route (and optionally each individual page) is turned into its own serverless function, so each one has to independently be able to start up the SDK.

That said, a) not all nextjs apps are deployed to Vercel, and b) even those that are sometimes have multiple serverless API routes combined into a single serverless function. As a result, `Sentry.init()` can end up getting called over and over again, as different routes are hit on the same running server process. While we manage hubs and clients and scopes in such a way that this doesn't break anything, it's also a total waste of time - the startup code that we bundle with each route comes from a single source (`sentry.server.config.js`) and therefore is the same for every route; once the SDK is started with the given options, it's started. No reason to do it multiple times. 

This thus introduces a check when calling the server-side `Sentry.init()`: if there is a client already defined on the current hub, it means the startup process has already run, and so it bails. 

There are also changes to the tests. TL;DR, because I needed the real `init()` from the node SDK to run (in order to create an already-initialized client to check), I switched the mocking of `@sentry/node` from a direct mock to a pair of spies. I further did two renamings - `s/mockInit/nodeInit`, to make it clear which `init()` function we're talking about, and `s/reactInitOptions/nodeInitOptions`, since this is the backend initialization we're testing rather than the front end.

Author: lobsterkatie

packages/nextjs/src/index.server.ts

@@ -1,5 +1,6 @@
 import { RewriteFrames } from '@sentry/integrations';
-import { configureScope, init as nodeInit, Integrations } from '@sentry/node';
+import { configureScope, getCurrentHub, init as nodeInit, Integrations } from '@sentry/node';
+import { logger } from '@sentry/utils';
 
 import { instrumentServer } from './utils/instrumentServer';
 import { MetadataBuilder } from './utils/metadataBuilder';
@@ -14,6 +15,17 @@ export { ErrorBoundary, withErrorBoundary } from '@sentry/react';
 
 /** Inits the Sentry NextJS SDK on node. */
 export function init(options: NextjsOptions): void {
+  if (options.debug) {
+    logger.enable();
+  }
+
+  logger.log('Initializing SDK...');
+
+  if (sdkAlreadyInitialized()) {
+    logger.log('SDK already initialized');
+    return;
+  }
+
   const metadataBuilder = new MetadataBuilder(options, ['nextjs', 'node']);
   metadataBuilder.addSdkMetadata();
   options.environment = options.environment || process.env.NODE_ENV;
@@ -26,6 +38,13 @@ export function init(options: NextjsOptions): void {
   configureScope(scope => {
     scope.setTag('runtime', 'node');
   });
+
+  logger.log('SDK successfully initialized');
+}
+
+function sdkAlreadyInitialized(): boolean {
+  const hub = getCurrentHub();
+  return !!hub.getClient();
 }
 
 const SOURCEMAP_FILENAME_REGEX = /^.*\/\.next\//;

packages/nextjs/test/index.server.test.ts

@@ -1,48 +1,52 @@
 import { RewriteFrames } from '@sentry/integrations';
-import { Integrations } from '@sentry/node';
+import * as SentryNode from '@sentry/node';
 import { Integration } from '@sentry/types';
+import { getGlobalObject } from '@sentry/utils';
 
 import { init, Scope } from '../src/index.server';
 import { NextjsOptions } from '../src/utils/nextjsOptions';
 
-const mockInit = jest.fn();
-let configureScopeCallback: (scope: Scope) => void = () => undefined;
+const { Integrations } = SentryNode;
 
-jest.mock('@sentry/node', () => {
-  const actual = jest.requireActual('@sentry/node');
-  return {
-    ...actual,
-    init: (options: NextjsOptions) => {
-      mockInit(options);
-    },
-    configureScope: (callback: (scope: Scope) => void) => {
-      configureScopeCallback = callback;
-    },
-  };
-});
+const global = getGlobalObject();
+
+let configureScopeCallback: (scope: Scope) => void = () => undefined;
+jest.spyOn(SentryNode, 'configureScope').mockImplementation(callback => (configureScopeCallback = callback));
+const nodeInit = jest.spyOn(SentryNode, 'init');
 
 describe('Server init()', () => {
   afterEach(() => {
-    mockInit.mockClear();
+    nodeInit.mockClear();
     configureScopeCallback = () => undefined;
+    global.__SENTRY__.hub = undefined;
   });
 
   it('inits the Node SDK', () => {
-    expect(mockInit).toHaveBeenCalledTimes(0);
+    expect(nodeInit).toHaveBeenCalledTimes(0);
     init({});
-    expect(mockInit).toHaveBeenCalledTimes(1);
-    expect(mockInit).toHaveBeenLastCalledWith({
-      _metadata: {
-        sdk: {
-          name: 'sentry.javascript.nextjs',
-          version: expect.any(String),
-          packages: expect.any(Array),
+    expect(nodeInit).toHaveBeenCalledTimes(1);
+    expect(nodeInit).toHaveBeenLastCalledWith(
+      expect.objectContaining({
+        _metadata: {
+          sdk: {
+            name: 'sentry.javascript.nextjs',
+            version: expect.any(String),
+            packages: expect.any(Array),
+          },
         },
-      },
-      autoSessionTracking: false,
-      environment: 'test',
-      integrations: [expect.any(RewriteFrames)],
-    });
+        autoSessionTracking: false,
+        environment: 'test',
+        integrations: [expect.any(RewriteFrames)],
+      }),
+    );
+  });
+
+  it("doesn't reinitialize the node SDK if already initialized", () => {
+    expect(nodeInit).toHaveBeenCalledTimes(0);
+    init({});
+    expect(nodeInit).toHaveBeenCalledTimes(1);
+    init({});
+    expect(nodeInit).toHaveBeenCalledTimes(1);
   });
 
   it('sets runtime on scope', () => {
@@ -57,45 +61,45 @@ describe('Server init()', () => {
     it('adds RewriteFrames integration by default', () => {
       init({});
 
-      const reactInitOptions: NextjsOptions = mockInit.mock.calls[0][0];
-      expect(reactInitOptions.integrations).toHaveLength(1);
-      const integrations = reactInitOptions.integrations as Integration[];
+      const nodeInitOptions: NextjsOptions = nodeInit.mock.calls[0][0]!;
+      expect(nodeInitOptions.integrations).toHaveLength(1);
+      const integrations = nodeInitOptions.integrations as Integration[];
       expect(integrations[0]).toEqual(expect.any(RewriteFrames));
     });
 
     it('adds Http integration by default if tracesSampleRate is set', () => {
       init({ tracesSampleRate: 1.0 });
 
-      const reactInitOptions: NextjsOptions = mockInit.mock.calls[0][0];
-      expect(reactInitOptions.integrations).toHaveLength(2);
-      const integrations = reactInitOptions.integrations as Integration[];
+      const nodeInitOptions: NextjsOptions = nodeInit.mock.calls[0][0]!;
+      expect(nodeInitOptions.integrations).toHaveLength(2);
+      const integrations = nodeInitOptions.integrations as Integration[];
       expect(integrations[1]).toEqual(expect.any(Integrations.Http));
     });
 
     it('adds Http integration by default if tracesSampler is set', () => {
       init({ tracesSampler: () => true });
 
-      const reactInitOptions: NextjsOptions = mockInit.mock.calls[0][0];
-      expect(reactInitOptions.integrations).toHaveLength(2);
-      const integrations = reactInitOptions.integrations as Integration[];
+      const nodeInitOptions: NextjsOptions = nodeInit.mock.calls[0][0]!;
+      expect(nodeInitOptions.integrations).toHaveLength(2);
+      const integrations = nodeInitOptions.integrations as Integration[];
       expect(integrations[1]).toEqual(expect.any(Integrations.Http));
     });
 
     it('adds Http integration with tracing true', () => {
       init({ tracesSampleRate: 1.0 });
-      const reactInitOptions: NextjsOptions = mockInit.mock.calls[0][0];
-      expect(reactInitOptions.integrations).toHaveLength(2);
+      const nodeInitOptions: NextjsOptions = nodeInit.mock.calls[0][0]!;
+      expect(nodeInitOptions.integrations).toHaveLength(2);
 
-      const integrations = reactInitOptions.integrations as Integration[];
+      const integrations = nodeInitOptions.integrations as Integration[];
       expect((integrations[1] as any)._tracing).toBe(true);
     });
 
     it('supports passing integration through options', () => {
       init({ tracesSampleRate: 1.0, integrations: [new Integrations.Console()] });
-      const reactInitOptions: NextjsOptions = mockInit.mock.calls[0][0];
-      expect(reactInitOptions.integrations).toHaveLength(3);
+      const nodeInitOptions: NextjsOptions = nodeInit.mock.calls[0][0]!;
+      expect(nodeInitOptions.integrations).toHaveLength(3);
 
-      const integrations = reactInitOptions.integrations as Integration[];
+      const integrations = nodeInitOptions.integrations as Integration[];
       expect(integrations).toEqual([
         expect.any(Integrations.Console),
         expect.any(RewriteFrames),
@@ -110,9 +114,9 @@ describe('Server init()', () => {
           integrations: [new Integrations.Http({ tracing: false })],
         });
 
-        const reactInitOptions: NextjsOptions = mockInit.mock.calls[0][0];
-        expect(reactInitOptions.integrations).toHaveLength(2);
-        const integrations = reactInitOptions.integrations as Integration[];
+        const nodeInitOptions: NextjsOptions = nodeInit.mock.calls[0][0]!;
+        expect(nodeInitOptions.integrations).toHaveLength(2);
+        const integrations = nodeInitOptions.integrations as Integration[];
         expect(integrations[0] as InstanceType<typeof Integrations.Http>).toEqual(
           expect.objectContaining({ _breadcrumbs: true, _tracing: true, name: 'Http' }),
         );
@@ -124,9 +128,9 @@ describe('Server init()', () => {
           integrations: [new Integrations.Http({ tracing: false })],
         });
 
-        const reactInitOptions: NextjsOptions = mockInit.mock.calls[0][0];
-        expect(reactInitOptions.integrations).toHaveLength(2);
-        const integrations = reactInitOptions.integrations as Integration[];
+        const nodeInitOptions: NextjsOptions = nodeInit.mock.calls[0][0]!;
+        expect(nodeInitOptions.integrations).toHaveLength(2);
+        const integrations = nodeInitOptions.integrations as Integration[];
         expect(integrations[0] as InstanceType<typeof Integrations.Http>).toEqual(
           expect.objectContaining({ _breadcrumbs: true, _tracing: true, name: 'Http' }),
         );