Skip to content

Commit

Permalink
feat(node): Migrate to domains used through AsyncContextStrategy (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
timfish authored Apr 11, 2023
1 parent 7987221 commit 5062ce1
Show file tree
Hide file tree
Showing 21 changed files with 236 additions and 419 deletions.
49 changes: 2 additions & 47 deletions packages/core/src/hub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,7 @@ import type {
TransactionContext,
User,
} from '@sentry/types';
import {
consoleSandbox,
dateTimestampInSeconds,
getGlobalSingleton,
GLOBAL_OBJ,
isNodeEnv,
logger,
uuid4,
} from '@sentry/utils';
import { consoleSandbox, dateTimestampInSeconds, getGlobalSingleton, GLOBAL_OBJ, logger, uuid4 } from '@sentry/utils';

import { DEFAULT_ENVIRONMENT } from './constants';
import { Scope } from './scope';
Expand All @@ -54,7 +46,7 @@ export interface RunWithAsyncContextOptions {
/** Whether to reuse an existing async context if one exists. Defaults to false. */
reuseExisting?: boolean;
/** Instances that should be referenced and retained in the new context */
args?: unknown[];
emitters?: unknown[];
}

/**
Expand Down Expand Up @@ -95,10 +87,6 @@ export interface Carrier {
*/
integrations?: Integration[];
extensions?: {
/** Hack to prevent bundlers from breaking our usage of the domain package in the cross-platform Hub package */
// eslint-disable-next-line @typescript-eslint/no-explicit-any
domain?: { [key: string]: any };
} & {
/** Extension methods for the hub, which are bound to the current Hub instance */
// eslint-disable-next-line @typescript-eslint/ban-types
[key: string]: Function;
Expand Down Expand Up @@ -561,11 +549,6 @@ export function getCurrentHub(): Hub {
}
}

// Prefer domains over global if they are there (applicable only to Node environment)
if (isNodeEnv()) {
return getHubFromActiveDomain(registry);
}

// Return hub that lives on a global object
return getGlobalHub(registry);
}
Expand Down Expand Up @@ -621,34 +604,6 @@ export function runWithAsyncContext<T>(callback: (hub: Hub) => T, options: RunWi
return callback(getCurrentHub());
}

/**
* Try to read the hub from an active domain, and fallback to the registry if one doesn't exist
* @returns discovered hub
*/
function getHubFromActiveDomain(registry: Carrier): Hub {
try {
const sentry = getMainCarrier().__SENTRY__;
const activeDomain = sentry && sentry.extensions && sentry.extensions.domain && sentry.extensions.domain.active;

// If there's no active domain, just return global hub
if (!activeDomain) {
return getHubFromCarrier(registry);
}

// If there's no hub on current domain, or it's an old API, assign a new one
if (!hasHubOnCarrier(activeDomain) || getHubFromCarrier(activeDomain).isOlderThan(API_VERSION)) {
const registryHubTopStack = getHubFromCarrier(registry).getStackTop();
setHubOnCarrier(activeDomain, new Hub(registryHubTopStack.client, Scope.clone(registryHubTopStack.scope)));
}

// Return hub that lives on a domain
return getHubFromCarrier(activeDomain);
} catch (_Oo) {
// Return hub that lives on a global object
return getHubFromCarrier(registry);
}
}

/**
* This will tell whether a carrier has a hub on it or not
* @param carrier object
Expand Down
30 changes: 1 addition & 29 deletions packages/nextjs/src/server/index.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import type { Carrier } from '@sentry/core';
import { getHubFromCarrier, getMainCarrier, hasTracingEnabled } from '@sentry/core';
import { hasTracingEnabled } from '@sentry/core';
import { RewriteFrames } from '@sentry/integrations';
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 domainModule from 'domain';
import * as path from 'path';

import { devErrorSymbolicationEventProcessor } from '../common/devErrorSymbolicationEventProcessor';
Expand Down Expand Up @@ -55,8 +53,6 @@ const globalWithInjectedValues = global as typeof global & {
__rewriteFramesDistDir__: string;
};

const domain = domainModule as typeof domainModule & { active: (domainModule.Domain & Carrier) | null };

// TODO (v8): Remove this
/**
* @deprecated This constant will be removed in the next major update.
Expand Down Expand Up @@ -87,16 +83,6 @@ export function init(options: NodeOptions): 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);

const filterTransactions: EventProcessor = event => {
Expand All @@ -118,20 +104,6 @@ export function init(options: NodeOptions): void {
}
});

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());
// `scope.update()` doesn’t copy over event processors, so we have to add it manually
domainHub.getScope()?.addEventProcessor(filterTransactions);

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

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

Expand Down
9 changes: 4 additions & 5 deletions packages/nextjs/src/server/utils/wrapperUtils.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { captureException, getActiveTransaction, getCurrentHub, startTransaction } from '@sentry/core';
import { captureException, getActiveTransaction, runWithAsyncContext, startTransaction } from '@sentry/core';
import type { Transaction } from '@sentry/types';
import { baggageHeaderToDynamicSamplingContext, extractTraceparentData } from '@sentry/utils';
import * as domain from 'domain';
import type { IncomingMessage, ServerResponse } from 'http';

import { platformSupportsStreaming } from './platformSupportsStreaming';
Expand Down Expand Up @@ -75,7 +74,7 @@ export function withTracedServerSideDataFetcher<F extends (...args: any[]) => Pr
},
): (...params: Parameters<F>) => Promise<ReturnType<F>> {
return async function (this: unknown, ...args: Parameters<F>): Promise<ReturnType<F>> {
return domain.create().bind(async () => {
return runWithAsyncContext(async hub => {
let requestTransaction: Transaction | undefined = getTransactionFromRequest(req);
let dataFetcherSpan;

Expand Down Expand Up @@ -134,7 +133,7 @@ export function withTracedServerSideDataFetcher<F extends (...args: any[]) => Pr
});
}

const currentScope = getCurrentHub().getScope();
const currentScope = hub.getScope();
if (currentScope) {
currentScope.setSpan(dataFetcherSpan);
currentScope.setSDKProcessingMetadata({ request: req });
Expand All @@ -154,7 +153,7 @@ export function withTracedServerSideDataFetcher<F extends (...args: any[]) => Pr
await flushQueue();
}
}
})();
});
};
}

Expand Down
Loading

0 comments on commit 5062ce1

Please sign in to comment.