Skip to content

fix(nextjs): Always initialize SDK with global hub #4086

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/nextjs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
},
"dependencies": {
"@sentry/core": "6.13.3",
"@sentry/hub": "6.13.3",
"@sentry/integrations": "6.13.3",
"@sentry/node": "6.13.3",
"@sentry/react": "6.13.3",
Expand Down
26 changes: 26 additions & 0 deletions packages/nextjs/src/index.server.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { Carrier, getHubFromCarrier, getMainCarrier } from '@sentry/hub';
import { RewriteFrames } from '@sentry/integrations';
import { configureScope, getCurrentHub, init as nodeInit, Integrations } from '@sentry/node';
import { escapeStringForRegex, logger } from '@sentry/utils';
import * as domainModule from 'domain';
import * as path from 'path';

import { instrumentServer } from './utils/instrumentServer';
Expand All @@ -15,6 +17,7 @@ export * from '@sentry/node';
export { ErrorBoundary, withErrorBoundary } from '@sentry/react';

type GlobalWithDistDir = typeof global & { __rewriteFramesDistDir__: string };
const domain = domainModule as typeof domainModule & { active: (domainModule.Domain & Carrier) | null };

/** Inits the Sentry NextJS SDK on node. */
export function init(options: NextjsOptions): void {
Expand All @@ -36,11 +39,34 @@ export function init(options: NextjsOptions): void {
// Right now we only capture frontend sessions for Next.js
options.autoSessionTracking = false;

// In an ideal world, this init function would be called before any requests are handled. That way, every domain we
// use to wrap a request would inherit its scope and client from the global hub. In practice, however, handling the
// first request is what causes us to initialize the SDK, as the init code is injected into `_app` and all API route
// handlers, and those are only accessed in the course of handling a request. As a result, we're already in a domain
// when `init` is called. In order to compensate for this and mimic the ideal world scenario, we stash the active
// domain, run `init` as normal, and then restore the domain afterwards, copying over data from the main hub as if we
// really were inheriting.
const activeDomain = domain.active;
domain.active = null;

nodeInit(options);

configureScope(scope => {
scope.setTag('runtime', 'node');
});

if (activeDomain) {
const globalHub = getHubFromCarrier(getMainCarrier());
const domainHub = getHubFromCarrier(activeDomain);

// apply the changes made by `nodeInit` to the domain's hub also
domainHub.bindClient(globalHub.getClient());
domainHub.getScope()?.update(globalHub.getScope());

// restore the domain hub as the current one
domain.active = activeDomain;
}

logger.log('SDK successfully initialized');
}

Expand Down
42 changes: 35 additions & 7 deletions packages/nextjs/test/index.server.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { RewriteFrames } from '@sentry/integrations';
import * as SentryNode from '@sentry/node';
import { getCurrentHub, NodeClient } from '@sentry/node';
import { Integration } from '@sentry/types';
import { getGlobalObject } from '@sentry/utils';
import * as domain from 'domain';

import { init, Scope } from '../src/index.server';
import { init } from '../src/index.server';
import { NextjsOptions } from '../src/utils/nextjsOptions';

const { Integrations } = SentryNode;
Expand All @@ -13,14 +15,11 @@ const global = getGlobalObject();
// normally this is set as part of the build process, so mock it here
(global as typeof global & { __rewriteFramesDistDir__: string }).__rewriteFramesDistDir__ = '.next';

let configureScopeCallback: (scope: Scope) => void = () => undefined;
jest.spyOn(SentryNode, 'configureScope').mockImplementation(callback => (configureScopeCallback = callback));
const nodeInit = jest.spyOn(SentryNode, 'init');

describe('Server init()', () => {
afterEach(() => {
nodeInit.mockClear();
configureScopeCallback = () => undefined;
global.__SENTRY__.hub = undefined;
});

Expand Down Expand Up @@ -53,11 +52,40 @@ describe('Server init()', () => {
});

it('sets runtime on scope', () => {
const mockScope = new Scope();
const currentScope = getCurrentHub().getScope();

// @ts-ignore need access to protected _tags attribute
expect(currentScope._tags).toEqual({});

init({});
configureScopeCallback(mockScope);

// @ts-ignore need access to protected _tags attribute
expect(mockScope._tags).toEqual({ runtime: 'node' });
expect(currentScope._tags).toEqual({ runtime: 'node' });
});

it("initializes both global hub and domain hub when there's an active domain", () => {
const globalHub = getCurrentHub();
const local = domain.create();
local.run(() => {
const domainHub = getCurrentHub();

// they are in fact two different hubs, and neither one yet has a client
expect(domainHub).not.toBe(globalHub);
expect(globalHub.getClient()).toBeUndefined();
expect(domainHub.getClient()).toBeUndefined();

// this tag should end up only in the domain hub
domainHub.setTag('dogs', 'areGreat');

init({});

expect(globalHub.getClient()).toEqual(expect.any(NodeClient));
expect(domainHub.getClient()).toBe(globalHub.getClient());
// @ts-ignore need access to protected _tags attribute
expect(globalHub.getScope()._tags).toEqual({ runtime: 'node' });
// @ts-ignore need access to protected _tags attribute
expect(domainHub.getScope()._tags).toEqual({ runtime: 'node', dogs: 'areGreat' });
});
});

describe('integrations', () => {
Expand Down