diff --git a/packages/core/src/hub.ts b/packages/core/src/hub.ts index 905fb4f3bcde..4cf1edb59752 100644 --- a/packages/core/src/hub.ts +++ b/packages/core/src/hub.ts @@ -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'; @@ -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[]; } /** @@ -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; @@ -551,11 +539,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); } @@ -611,34 +594,6 @@ export function runWithAsyncContext(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 diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts index 1eeafddb4251..b22a6c977a78 100644 --- a/packages/nextjs/src/server/index.ts +++ b/packages/nextjs/src/server/index.ts @@ -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'; @@ -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. @@ -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 => { @@ -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'); } diff --git a/packages/nextjs/src/server/utils/wrapperUtils.ts b/packages/nextjs/src/server/utils/wrapperUtils.ts index 9fa91fbbee5a..2448cfb7b8b6 100644 --- a/packages/nextjs/src/server/utils/wrapperUtils.ts +++ b/packages/nextjs/src/server/utils/wrapperUtils.ts @@ -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'; @@ -75,7 +74,7 @@ export function withTracedServerSideDataFetcher Pr }, ): (...params: Parameters) => Promise> { return async function (this: unknown, ...args: Parameters): Promise> { - return domain.create().bind(async () => { + return runWithAsyncContext(async hub => { let requestTransaction: Transaction | undefined = getTransactionFromRequest(req); let dataFetcherSpan; @@ -134,7 +133,7 @@ export function withTracedServerSideDataFetcher Pr }); } - const currentScope = getCurrentHub().getScope(); + const currentScope = hub.getScope(); if (currentScope) { currentScope.setSpan(dataFetcherSpan); currentScope.setSDKProcessingMetadata({ request: req }); @@ -154,7 +153,7 @@ export function withTracedServerSideDataFetcher Pr await flushQueue(); } } - })(); + }); }; } diff --git a/packages/nextjs/src/server/wrapApiHandlerWithSentry.ts b/packages/nextjs/src/server/wrapApiHandlerWithSentry.ts index 37ef93bf30cc..b1388352c039 100644 --- a/packages/nextjs/src/server/wrapApiHandlerWithSentry.ts +++ b/packages/nextjs/src/server/wrapApiHandlerWithSentry.ts @@ -1,5 +1,5 @@ -import { hasTracingEnabled } from '@sentry/core'; -import { captureException, getCurrentHub, startTransaction } from '@sentry/node'; +import { hasTracingEnabled, runWithAsyncContext } from '@sentry/core'; +import { captureException, startTransaction } from '@sentry/node'; import type { Transaction } from '@sentry/types'; import { addExceptionMechanism, @@ -10,7 +10,6 @@ import { objectify, stripUrlQueryAndFragment, } from '@sentry/utils'; -import * as domain from 'domain'; import type { AugmentedNextApiRequest, AugmentedNextApiResponse, NextApiHandler } from './types'; import { platformSupportsStreaming } from './utils/platformSupportsStreaming'; @@ -61,159 +60,154 @@ export function withSentry(apiHandler: NextApiHandler, parameterizedRoute?: stri } req.__withSentry_applied__ = true; - // use a domain in order to prevent scope bleed between requests - const local = domain.create(); - local.add(req); - local.add(res); - - // `local.bind` causes everything to run inside a domain, just like `local.run` does, but it also lets the callback - // return a value. In our case, all any of the codepaths return is a promise of `void`, but nextjs still counts on - // getting that before it will finish the response. - // eslint-disable-next-line complexity - const boundHandler = local.bind(async () => { - let transaction: Transaction | undefined; - const hub = getCurrentHub(); - const currentScope = hub.getScope(); - const options = hub.getClient()?.getOptions(); - - if (currentScope) { - currentScope.setSDKProcessingMetadata({ request: req }); - - if (hasTracingEnabled(options) && options?.instrumenter === 'sentry') { - // If there is a trace header set, extract the data from it (parentSpanId, traceId, and sampling decision) - let traceparentData; - if (req.headers && isString(req.headers['sentry-trace'])) { - traceparentData = extractTraceparentData(req.headers['sentry-trace']); - __DEBUG_BUILD__ && logger.log(`[Tracing] Continuing trace ${traceparentData?.traceId}.`); - } + // eslint-disable-next-line complexity, @typescript-eslint/no-explicit-any + const boundHandler = runWithAsyncContext( + // eslint-disable-next-line complexity + async hub => { + let transaction: Transaction | undefined; + const currentScope = hub.getScope(); + const options = hub.getClient()?.getOptions(); - const baggageHeader = req.headers && req.headers.baggage; - const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(baggageHeader); - - // prefer the parameterized route, if we have it (which we will if we've auto-wrapped the route handler) - let reqPath = parameterizedRoute; - - // If not, fake it by just replacing parameter values with their names, hoping that none of them match either - // each other or any hard-coded parts of the path - if (!reqPath) { - const url = `${req.url}`; - // pull off query string, if any - reqPath = stripUrlQueryAndFragment(url); - // Replace with placeholder - if (req.query) { - for (const [key, value] of Object.entries(req.query)) { - reqPath = reqPath.replace(`${value}`, `[${key}]`); - } + if (currentScope) { + currentScope.setSDKProcessingMetadata({ request: req }); + + if (hasTracingEnabled(options) && options?.instrumenter === 'sentry') { + // If there is a trace header set, extract the data from it (parentSpanId, traceId, and sampling decision) + let traceparentData; + if (req.headers && isString(req.headers['sentry-trace'])) { + traceparentData = extractTraceparentData(req.headers['sentry-trace']); + __DEBUG_BUILD__ && logger.log(`[Tracing] Continuing trace ${traceparentData?.traceId}.`); } - } - const reqMethod = `${(req.method || 'GET').toUpperCase()} `; - - transaction = startTransaction( - { - name: `${reqMethod}${reqPath}`, - op: 'http.server', - ...traceparentData, - metadata: { - dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, - source: 'route', - request: req, - }, - }, - // extra context passed to the `tracesSampler` - { request: req }, - ); - currentScope.setSpan(transaction); - if (platformSupportsStreaming() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) { - autoEndTransactionOnResponseEnd(transaction, res); - } else { - // If we're not on a platform that supports streaming, we're blocking res.end() until the queue is flushed. - // res.json() and res.send() will implicitly call res.end(), so it is enough to wrap res.end(). - - // eslint-disable-next-line @typescript-eslint/unbound-method - const origResEnd = res.end; - res.end = async function (this: unknown, ...args: unknown[]) { - if (transaction) { - await finishTransaction(transaction, res); - await flushQueue(); + const baggageHeader = req.headers && req.headers.baggage; + const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(baggageHeader); + + // prefer the parameterized route, if we have it (which we will if we've auto-wrapped the route handler) + let reqPath = parameterizedRoute; + + // If not, fake it by just replacing parameter values with their names, hoping that none of them match either + // each other or any hard-coded parts of the path + if (!reqPath) { + const url = `${req.url}`; + // pull off query string, if any + reqPath = stripUrlQueryAndFragment(url); + // Replace with placeholder + if (req.query) { + for (const [key, value] of Object.entries(req.query)) { + reqPath = reqPath.replace(`${value}`, `[${key}]`); + } } + } - origResEnd.apply(this, args); - }; + const reqMethod = `${(req.method || 'GET').toUpperCase()} `; + + transaction = startTransaction( + { + name: `${reqMethod}${reqPath}`, + op: 'http.server', + ...traceparentData, + metadata: { + dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, + source: 'route', + request: req, + }, + }, + // extra context passed to the `tracesSampler` + { request: req }, + ); + currentScope.setSpan(transaction); + if (platformSupportsStreaming() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) { + autoEndTransactionOnResponseEnd(transaction, res); + } else { + // If we're not on a platform that supports streaming, we're blocking res.end() until the queue is flushed. + // res.json() and res.send() will implicitly call res.end(), so it is enough to wrap res.end(). + + // eslint-disable-next-line @typescript-eslint/unbound-method + const origResEnd = res.end; + res.end = async function (this: unknown, ...args: unknown[]) { + if (transaction) { + await finishTransaction(transaction, res); + await flushQueue(); + } + + origResEnd.apply(this, args); + }; + } } } - } - - try { - const handlerResult = await wrappingTarget.apply(thisArg, args); - - if ( - process.env.NODE_ENV === 'development' && - !process.env.SENTRY_IGNORE_API_RESOLUTION_ERROR && - !res.finished - // This can only happen (not always) when the user is using `withSentry` manually, which we're deprecating. - // Warning suppression on Next.JS is only necessary in that case. - ) { - // eslint-disable-next-line no-console - console.warn( - `[sentry] If Next.js logs a warning "API resolved without sending a response", it's a false positive, which may happen when you use \`withSentry\` manually to wrap your routes. + + try { + const handlerResult = await wrappingTarget.apply(thisArg, args); + + if ( + process.env.NODE_ENV === 'development' && + !process.env.SENTRY_IGNORE_API_RESOLUTION_ERROR && + !res.finished + // This can only happen (not always) when the user is using `withSentry` manually, which we're deprecating. + // Warning suppression on Next.JS is only necessary in that case. + ) { + // eslint-disable-next-line no-console + console.warn( + `[sentry] If Next.js logs a warning "API resolved without sending a response", it's a false positive, which may happen when you use \`withSentry\` manually to wrap your routes. To suppress this warning, set \`SENTRY_IGNORE_API_RESOLUTION_ERROR\` to 1 in your env. To suppress the nextjs warning, use the \`externalResolver\` API route option (see https://nextjs.org/docs/api-routes/api-middlewares#custom-config for details).`, - ); - } - - return handlerResult; - } catch (e) { - // In case we have a primitive, wrap it in the equivalent wrapper class (string -> String, etc.) so that we can - // store a seen flag on it. (Because of the one-way-on-Vercel-one-way-off-of-Vercel approach we've been forced - // to take, it can happen that the same thrown object gets caught in two different ways, and flagging it is a - // way to prevent it from actually being reported twice.) - const objectifiedErr = objectify(e); + ); + } - if (currentScope) { - currentScope.addEventProcessor(event => { - addExceptionMechanism(event, { - type: 'instrument', - handled: true, - data: { - wrapped_handler: wrappingTarget.name, - function: 'withSentry', - }, + return handlerResult; + } catch (e) { + // In case we have a primitive, wrap it in the equivalent wrapper class (string -> String, etc.) so that we can + // store a seen flag on it. (Because of the one-way-on-Vercel-one-way-off-of-Vercel approach we've been forced + // to take, it can happen that the same thrown object gets caught in two different ways, and flagging it is a + // way to prevent it from actually being reported twice.) + const objectifiedErr = objectify(e); + + if (currentScope) { + currentScope.addEventProcessor(event => { + addExceptionMechanism(event, { + type: 'instrument', + handled: true, + data: { + wrapped_handler: wrappingTarget.name, + function: 'withSentry', + }, + }); + return event; }); - return event; - }); - captureException(objectifiedErr); - } + captureException(objectifiedErr); + } - // Because we're going to finish and send the transaction before passing the error onto nextjs, it won't yet - // have had a chance to set the status to 500, so unless we do it ourselves now, we'll incorrectly report that - // the transaction was error-free - res.statusCode = 500; - res.statusMessage = 'Internal Server Error'; - - // Make sure we have a chance to finish the transaction and flush events to Sentry before the handler errors - // out. (Apps which are deployed on Vercel run their API routes in lambdas, and those lambdas will shut down the - // moment they detect an error, so it's important to get this done before rethrowing the error. Apps not - // deployed serverlessly will run into this cleanup code again in `res.end(), but the transaction will already - // be finished and the queue will already be empty, so effectively it'll just no-op.) - if (platformSupportsStreaming() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) { - void finishTransaction(transaction, res); - } else { - await finishTransaction(transaction, res); - await flushQueue(); - } + // Because we're going to finish and send the transaction before passing the error onto nextjs, it won't yet + // have had a chance to set the status to 500, so unless we do it ourselves now, we'll incorrectly report that + // the transaction was error-free + res.statusCode = 500; + res.statusMessage = 'Internal Server Error'; + + // Make sure we have a chance to finish the transaction and flush events to Sentry before the handler errors + // out. (Apps which are deployed on Vercel run their API routes in lambdas, and those lambdas will shut down the + // moment they detect an error, so it's important to get this done before rethrowing the error. Apps not + // deployed serverlessly will run into this cleanup code again in `res.end(), but the transaction will already + // be finished and the queue will already be empty, so effectively it'll just no-op.) + if (platformSupportsStreaming() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) { + void finishTransaction(transaction, res); + } else { + await finishTransaction(transaction, res); + await flushQueue(); + } - // We rethrow here so that nextjs can do with the error whatever it would normally do. (Sometimes "whatever it - // would normally do" is to allow the error to bubble up to the global handlers - another reason we need to mark - // the error as already having been captured.) - throw objectifiedErr; - } - }); + // We rethrow here so that nextjs can do with the error whatever it would normally do. (Sometimes "whatever it + // would normally do" is to allow the error to bubble up to the global handlers - another reason we need to mark + // the error as already having been captured.) + throw objectifiedErr; + } + }, + { emitters: [req, res] }, + ); // Since API route handlers are all async, nextjs always awaits the return value (meaning it's fine for us to return // a promise here rather than a real result, and it saves us the overhead of an `await` call.) - return boundHandler(); + return boundHandler; }, }); } diff --git a/packages/nextjs/src/server/wrapServerComponentWithSentry.ts b/packages/nextjs/src/server/wrapServerComponentWithSentry.ts index 2c25e8811409..73e331424c5c 100644 --- a/packages/nextjs/src/server/wrapServerComponentWithSentry.ts +++ b/packages/nextjs/src/server/wrapServerComponentWithSentry.ts @@ -1,6 +1,5 @@ -import { addTracingExtensions, captureException, getCurrentHub, startTransaction } from '@sentry/core'; +import { addTracingExtensions, captureException, runWithAsyncContext, startTransaction } from '@sentry/core'; import { baggageHeaderToDynamicSamplingContext, extractTraceparentData } from '@sentry/utils'; -import * as domain from 'domain'; import { isNotFoundNavigationError, isRedirectNavigationError } from '../common/nextNavigationErrorUtils'; import type { ServerComponentContext } from '../common/types'; @@ -21,7 +20,7 @@ export function wrapServerComponentWithSentry any> // hook. 🤯 return new Proxy(appDirComponent, { apply: (originalFunction, thisArg, args) => { - return domain.create().bind(() => { + return runWithAsyncContext(hub => { let maybePromiseResult; const traceparentData = context.sentryTraceHeader @@ -41,7 +40,7 @@ export function wrapServerComponentWithSentry any> }, }); - const currentScope = getCurrentHub().getScope(); + const currentScope = hub.getScope(); if (currentScope) { currentScope.setSpan(transaction); } @@ -85,7 +84,7 @@ export function wrapServerComponentWithSentry any> transaction.finish(); return maybePromiseResult; } - })(); + }); }, }); } diff --git a/packages/nextjs/test/serverSdk.test.ts b/packages/nextjs/test/serverSdk.test.ts index bb55d9f6e184..4367c9ec1abc 100644 --- a/packages/nextjs/test/serverSdk.test.ts +++ b/packages/nextjs/test/serverSdk.test.ts @@ -1,8 +1,8 @@ +import { runWithAsyncContext } from '@sentry/core'; import * as SentryNode from '@sentry/node'; import { getCurrentHub, NodeClient } from '@sentry/node'; import type { Integration } from '@sentry/types'; import { GLOBAL_OBJ, logger } from '@sentry/utils'; -import * as domain from 'domain'; import { init } from '../src/server'; @@ -21,7 +21,7 @@ function findIntegrationByName(integrations: Integration[] = [], name: string): describe('Server init()', () => { afterEach(() => { jest.clearAllMocks(); - GLOBAL_OBJ.__SENTRY__.hub = undefined; + delete GLOBAL_OBJ.__SENTRY__; delete process.env.VERCEL; }); @@ -116,26 +116,27 @@ describe('Server init()', () => { 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); + runWithAsyncContext(globalHub2 => { + // If we call runWithAsyncContext before init, it executes the callback in the same context as there is no + // strategy yet + expect(globalHub2).toBe(globalHub); expect(globalHub.getClient()).toBeUndefined(); - expect(domainHub.getClient()).toBeUndefined(); - - // this tag should end up only in the domain hub - domainHub.setTag('dogs', 'areGreat'); + expect(globalHub2.getClient()).toBeUndefined(); 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' }); + runWithAsyncContext(domainHub => { + // this tag should end up only in the domain hub + domainHub.setTag('dogs', 'areGreat'); + + 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' }); + }); }); }); diff --git a/packages/node/package.json b/packages/node/package.json index 49e7a09eb998..483bf278610b 100644 --- a/packages/node/package.json +++ b/packages/node/package.json @@ -62,5 +62,6 @@ }, "volta": { "extends": "../../package.json" - } + }, + "sideEffects": false } diff --git a/packages/node/src/async/domain.ts b/packages/node/src/async/domain.ts index c34bf2e2124f..b41ae623d594 100644 --- a/packages/node/src/async/domain.ts +++ b/packages/node/src/async/domain.ts @@ -31,7 +31,7 @@ function runWithAsyncContext(callback: (hub: Hub) => T, options: RunWithAsync const activeDomain = getActiveDomain(); if (activeDomain) { - for (const emitter of options.args || []) { + for (const emitter of options.emitters || []) { if (emitter instanceof EventEmitter) { activeDomain.add(emitter); } @@ -44,7 +44,7 @@ function runWithAsyncContext(callback: (hub: Hub) => T, options: RunWithAsync const local = domain.create(); - for (const emitter of options.args || []) { + for (const emitter of options.emitters || []) { if (emitter instanceof EventEmitter) { local.add(emitter); } diff --git a/packages/node/src/handlers.ts b/packages/node/src/handlers.ts index 5dfcb7bf0a13..17800cadb03b 100644 --- a/packages/node/src/handlers.ts +++ b/packages/node/src/handlers.ts @@ -1,5 +1,12 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ -import { captureException, getCurrentHub, hasTracingEnabled, startTransaction, withScope } from '@sentry/core'; +import { + captureException, + getCurrentHub, + hasTracingEnabled, + runWithAsyncContext, + startTransaction, + withScope, +} from '@sentry/core'; import type { Span } from '@sentry/types'; import type { AddRequestDataToEventOptions } from '@sentry/utils'; import { @@ -12,7 +19,6 @@ import { logger, normalize, } from '@sentry/utils'; -import * as domain from 'domain'; import type * as http from 'http'; import type { NodeClient } from './client'; @@ -181,46 +187,43 @@ export function requestHandler( }); }; } - const local = domain.create(); - local.add(req); - local.add(res); - - local.run(() => { - const currentHub = getCurrentHub(); - - currentHub.configureScope(scope => { - scope.setSDKProcessingMetadata({ - request: req, - // TODO (v8): Stop passing this - requestDataOptionsFromExpressHandler: requestDataOptions, - }); + runWithAsyncContext( + currentHub => { + currentHub.configureScope(scope => { + scope.setSDKProcessingMetadata({ + request: req, + // TODO (v8): Stop passing this + requestDataOptionsFromExpressHandler: requestDataOptions, + }); - const client = currentHub.getClient(); - if (isAutoSessionTrackingEnabled(client)) { - const scope = currentHub.getScope(); - if (scope) { - // Set `status` of `RequestSession` to Ok, at the beginning of the request - scope.setRequestSession({ status: 'ok' }); + const client = currentHub.getClient(); + if (isAutoSessionTrackingEnabled(client)) { + const scope = currentHub.getScope(); + if (scope) { + // Set `status` of `RequestSession` to Ok, at the beginning of the request + scope.setRequestSession({ status: 'ok' }); + } } - } - }); + }); - res.once('finish', () => { - const client = currentHub.getClient(); - if (isAutoSessionTrackingEnabled(client)) { - setImmediate(() => { - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - if (client && (client as any)._captureRequestSession) { - // Calling _captureRequestSession to capture request session at the end of the request by incrementing - // the correct SessionAggregates bucket i.e. crashed, errored or exited + res.once('finish', () => { + const client = currentHub.getClient(); + if (isAutoSessionTrackingEnabled(client)) { + setImmediate(() => { // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - (client as any)._captureRequestSession(); - } - }); - } - }); - next(); - }); + if (client && (client as any)._captureRequestSession) { + // Calling _captureRequestSession to capture request session at the end of the request by incrementing + // the correct SessionAggregates bucket i.e. crashed, errored or exited + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + (client as any)._captureRequestSession(); + } + }); + } + }); + next(); + }, + { emitters: [req, res] }, + ); }; } diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index 7b4e3fe5d56b..6537b16aca70 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -37,6 +37,7 @@ export { getCurrentHub, Hub, makeMain, + runWithAsyncContext, Scope, startTransaction, SDK_VERSION, @@ -59,8 +60,7 @@ export { defaultIntegrations, init, defaultStackParser, lastEventId, flush, clos export { addRequestDataToEvent, DEFAULT_USER_INCLUDES, extractRequestData } from './requestdata'; export { deepReadDirSync } from './utils'; -import { getMainCarrier, Integrations as CoreIntegrations } from '@sentry/core'; -import * as domain from 'domain'; +import { Integrations as CoreIntegrations } from '@sentry/core'; import * as Handlers from './handlers'; import * as NodeIntegrations from './integrations'; @@ -73,11 +73,3 @@ const INTEGRATIONS = { }; export { INTEGRATIONS as Integrations, Handlers }; - -// We need to patch domain on the global __SENTRY__ object to make it work for node in cross-platform packages like -// @sentry/core. If we don't do this, browser bundlers will have troubles resolving `require('domain')`. -const carrier = getMainCarrier(); -if (carrier.__SENTRY__) { - carrier.__SENTRY__.extensions = carrier.__SENTRY__.extensions || {}; - carrier.__SENTRY__.extensions.domain = carrier.__SENTRY__.extensions.domain || domain; -} diff --git a/packages/node/src/sdk.ts b/packages/node/src/sdk.ts index 71ae258826c8..6e8ff2b16d1f 100644 --- a/packages/node/src/sdk.ts +++ b/packages/node/src/sdk.ts @@ -5,7 +5,6 @@ import { getMainCarrier, initAndBind, Integrations as CoreIntegrations, - setHubOnCarrier, } from '@sentry/core'; import type { SessionStatus, StackParser } from '@sentry/types'; import { @@ -15,8 +14,8 @@ import { nodeStackLineParser, stackParserFromStackParserOptions, } from '@sentry/utils'; -import * as domain from 'domain'; +import { setDomainAsyncContextStrategy } from './async/domain'; import { NodeClient } from './client'; import { Console, @@ -111,6 +110,9 @@ export const defaultIntegrations = [ */ export function init(options: NodeOptions = {}): void { const carrier = getMainCarrier(); + + setDomainAsyncContextStrategy(); + const autoloadedIntegrations = carrier.__SENTRY__?.integrations || []; options.defaultIntegrations = @@ -154,11 +156,6 @@ export function init(options: NodeOptions = {}): void { options.instrumenter = 'sentry'; } - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any - if ((domain as any).active) { - setHubOnCarrier(carrier, getCurrentHub()); - } - // TODO(v7): Refactor this to reduce the logic above const clientOptions: NodeClientOptions = { ...options, diff --git a/packages/node/test/domain.test.ts b/packages/node/test/domain.test.ts deleted file mode 100644 index fa6bb94db292..000000000000 --- a/packages/node/test/domain.test.ts +++ /dev/null @@ -1,72 +0,0 @@ -import { getCurrentHub, Hub } from '@sentry/core'; -import * as domain from 'domain'; - -// We need this import here to patch domain on the global object -import * as Sentry from '../src'; - -// TODO This is here because if we don't use the `Sentry` object, the 'concurrent domain hubs' test will fail. Is this a -// product of treeshaking? -Sentry.getCurrentHub(); - -describe('domains', () => { - test('without domain', () => { - // @ts-ignore property active does not exist on domain - expect(domain.active).toBeFalsy(); - const hub = getCurrentHub(); - expect(hub).toEqual(new Hub()); - }); - - test('domain hub scope inheritance', () => { - const globalHub = getCurrentHub(); - globalHub.configureScope(scope => { - scope.setExtra('a', 'b'); - scope.setTag('a', 'b'); - scope.addBreadcrumb({ message: 'a' }); - }); - const d = domain.create(); - d.run(() => { - const hub = getCurrentHub(); - expect(globalHub).toEqual(hub); - }); - }); - - test('domain hub single instance', () => { - const d = domain.create(); - d.run(() => { - expect(getCurrentHub()).toBe(getCurrentHub()); - }); - }); - - test('concurrent domain hubs', done => { - const d1 = domain.create(); - const d2 = domain.create(); - let d1done = false; - let d2done = false; - - d1.run(() => { - const hub = getCurrentHub(); - hub.getStack().push({ client: 'process' } as any); - expect(hub.getStack()[1]).toEqual({ client: 'process' }); - // Just in case so we don't have to worry which one finishes first - // (although it always should be d2) - setTimeout(() => { - d1done = true; - if (d2done) { - done(); - } - }); - }); - - d2.run(() => { - const hub = getCurrentHub(); - hub.getStack().push({ client: 'local' } as any); - expect(hub.getStack()[1]).toEqual({ client: 'local' }); - setTimeout(() => { - d2done = true; - if (d1done) { - done(); - } - }); - }); - }); -}); diff --git a/packages/node/test/handlers.test.ts b/packages/node/test/handlers.test.ts index 028cdff98af1..4f7060b655ff 100644 --- a/packages/node/test/handlers.test.ts +++ b/packages/node/test/handlers.test.ts @@ -4,11 +4,14 @@ import type { Event } from '@sentry/types'; import { SentryError } from '@sentry/utils'; import * as http from 'http'; +import { setDomainAsyncContextStrategy } from '../src/async/domain'; import { NodeClient } from '../src/client'; import { errorHandler, requestHandler, tracingHandler } from '../src/handlers'; import * as SDK from '../src/sdk'; import { getDefaultNodeClientOptions } from './helper/node-client-options'; +setDomainAsyncContextStrategy(); + describe('requestHandler', () => { const headers = { ears: 'furry', nose: 'wet', tongue: 'spotted', cookie: 'favorite=zukes' }; const method = 'wagging'; diff --git a/packages/node/test/index.test.ts b/packages/node/test/index.test.ts index 6ca834874d2e..8a4c971d4f3f 100644 --- a/packages/node/test/index.test.ts +++ b/packages/node/test/index.test.ts @@ -1,6 +1,5 @@ -import { getMainCarrier, initAndBind, SDK_VERSION } from '@sentry/core'; +import { getMainCarrier, initAndBind, runWithAsyncContext, SDK_VERSION } from '@sentry/core'; import type { EventHint, Integration } from '@sentry/types'; -import * as domain from 'domain'; import type { Event, Scope } from '../src'; import { @@ -13,6 +12,7 @@ import { init, NodeClient, } from '../src'; +import { setDomainAsyncContextStrategy } from '../src/async/domain'; import { ContextLines, LinkedErrors } from '../src/integrations'; import { defaultStackParser } from '../src/sdk'; import type { NodeClientOptions } from '../src/types'; @@ -278,8 +278,6 @@ describe('SentryNode', () => { }); test('capture an event in a domain', done => { - const d = domain.create(); - const options = getDefaultNodeClientOptions({ stackParser: defaultStackParser, beforeSend: (event: Event) => { @@ -290,12 +288,13 @@ describe('SentryNode', () => { }, dsn, }); + setDomainAsyncContextStrategy(); const client = new NodeClient(options); - d.run(() => { - getCurrentHub().bindClient(client); + runWithAsyncContext(hub => { + hub.bindClient(client); expect(getCurrentHub().getClient()).toBe(client); - getCurrentHub().captureEvent({ message: 'test domain' }); + hub.captureEvent({ message: 'test domain' }); }); }); diff --git a/packages/node/test/manual/webpack-domain/index.js b/packages/node/test/manual/webpack-domain/index.js index 4a9a5a0f902a..27f3fe0376ef 100644 --- a/packages/node/test/manual/webpack-domain/index.js +++ b/packages/node/test/manual/webpack-domain/index.js @@ -47,8 +47,7 @@ Sentry.configureScope(scope => { scope.setTag('a', 'b'); }); -const d = require('domain').create(); -d.run(() => { +Sentry.runWithAsyncContext(() => { Sentry.configureScope(scope => { scope.setTag('a', 'x'); scope.setTag('b', 'c'); diff --git a/packages/remix/src/utils/instrumentServer.ts b/packages/remix/src/utils/instrumentServer.ts index 8e916d75ffc4..96acde1db2aa 100644 --- a/packages/remix/src/utils/instrumentServer.ts +++ b/packages/remix/src/utils/instrumentServer.ts @@ -1,5 +1,5 @@ /* eslint-disable max-lines */ -import { getActiveTransaction, hasTracingEnabled } from '@sentry/core'; +import { getActiveTransaction, hasTracingEnabled, runWithAsyncContext } from '@sentry/core'; import type { Hub } from '@sentry/node'; import { captureException, getCurrentHub } from '@sentry/node'; import type { Transaction, TransactionSource, WrappedFunction } from '@sentry/types'; @@ -13,7 +13,6 @@ import { loadModule, logger, } from '@sentry/utils'; -import * as domain from 'domain'; import type { AppData, @@ -314,13 +313,7 @@ function wrapRequestHandler(origRequestHandler: RequestHandler, build: ServerBui const pkg = loadModule('react-router-dom'); return async function (this: unknown, request: RemixRequest, loadContext?: unknown): Promise { - const local = domain.create(); - - // Waiting for the next tick to make sure that the domain is active - // https://github.com/nodejs/node/issues/40999#issuecomment-1002719169 - await new Promise(resolve => setImmediate(resolve)); - - return local.bind(async () => { + return runWithAsyncContext(async () => { const hub = getCurrentHub(); const options = hub.getClient()?.getOptions(); const scope = hub.getScope(); @@ -365,7 +358,7 @@ function wrapRequestHandler(origRequestHandler: RequestHandler, build: ServerBui transaction.finish(); return res; - })(); + }); }; } diff --git a/packages/serverless/package.json b/packages/serverless/package.json index 00c1e8bf1964..5d3cb0ab65a6 100644 --- a/packages/serverless/package.json +++ b/packages/serverless/package.json @@ -16,6 +16,7 @@ "access": "public" }, "dependencies": { + "@sentry/core": "7.47.0", "@sentry/node": "7.47.0", "@sentry/types": "7.47.0", "@sentry/utils": "7.47.0", diff --git a/packages/serverless/src/gcpfunction/cloud_events.ts b/packages/serverless/src/gcpfunction/cloud_events.ts index 9783d5a530b3..6a8bc47f226f 100644 --- a/packages/serverless/src/gcpfunction/cloud_events.ts +++ b/packages/serverless/src/gcpfunction/cloud_events.ts @@ -1,7 +1,7 @@ import { captureException, flush, getCurrentHub } from '@sentry/node'; import { isThenable, logger } from '@sentry/utils'; -import { domainify, getActiveDomain, proxyFunction } from '../utils'; +import { domainify, proxyFunction } from '../utils'; import type { CloudEventFunction, CloudEventFunctionWithCallback, WrapperOptions } from './general'; export type CloudEventFunctionWrapperOptions = WrapperOptions; @@ -47,9 +47,7 @@ function _wrapCloudEventFunction( scope.setSpan(transaction); }); - const activeDomain = getActiveDomain()!; // eslint-disable-line @typescript-eslint/no-non-null-assertion - - const newCallback = activeDomain.bind((...args: unknown[]) => { + const newCallback = domainify((...args: unknown[]) => { if (args[0] !== null && args[0] !== undefined) { captureException(args[0]); } diff --git a/packages/serverless/src/gcpfunction/events.ts b/packages/serverless/src/gcpfunction/events.ts index 8f87bd478b0d..c00838ccaae6 100644 --- a/packages/serverless/src/gcpfunction/events.ts +++ b/packages/serverless/src/gcpfunction/events.ts @@ -1,7 +1,7 @@ import { captureException, flush, getCurrentHub } from '@sentry/node'; import { isThenable, logger } from '@sentry/utils'; -import { domainify, getActiveDomain, proxyFunction } from '../utils'; +import { domainify, proxyFunction } from '../utils'; import type { EventFunction, EventFunctionWithCallback, WrapperOptions } from './general'; export type EventFunctionWrapperOptions = WrapperOptions; @@ -49,9 +49,7 @@ function _wrapEventFunction scope.setSpan(transaction); }); - const activeDomain = getActiveDomain()!; // eslint-disable-line @typescript-eslint/no-non-null-assertion - - const newCallback = activeDomain.bind((...args: unknown[]) => { + const newCallback = domainify((...args: unknown[]) => { if (args[0] !== null && args[0] !== undefined) { captureException(args[0]); } diff --git a/packages/serverless/src/utils.ts b/packages/serverless/src/utils.ts index 38e85c6a1c4e..ae1f4b987ffb 100644 --- a/packages/serverless/src/utils.ts +++ b/packages/serverless/src/utils.ts @@ -1,6 +1,6 @@ +import { runWithAsyncContext } from '@sentry/core'; import type { Event } from '@sentry/node'; import { addExceptionMechanism } from '@sentry/utils'; -import * as domain from 'domain'; /** * Event processor that will override SDK details to point to the serverless SDK instead of Node, @@ -17,26 +17,12 @@ export function serverlessEventProcessor(event: Event): Event { return event; } -/** - * @returns Current active domain with a correct type. - */ -export function getActiveDomain(): domain.Domain | null { - // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access - return (domain as any).active as domain.Domain | null; -} - /** * @param fn function to run * @returns function which runs in the newly created domain or in the existing one */ export function domainify(fn: (...args: A) => R): (...args: A) => R | void { - return (...args) => { - if (getActiveDomain()) { - return fn(...args); - } - const dom = domain.create(); - return dom.run(() => fn(...args)); - }; + return (...args) => runWithAsyncContext(() => fn(...args), { reuseExisting: true }); } /** diff --git a/packages/sveltekit/src/server/handle.ts b/packages/sveltekit/src/server/handle.ts index cc291e63f579..419e749d1732 100644 --- a/packages/sveltekit/src/server/handle.ts +++ b/packages/sveltekit/src/server/handle.ts @@ -1,10 +1,9 @@ /* eslint-disable @sentry-internal/sdk/no-optional-chaining */ import type { Span } from '@sentry/core'; -import { getActiveTransaction, getCurrentHub, trace } from '@sentry/core'; +import { getActiveTransaction, getCurrentHub, runWithAsyncContext, trace } from '@sentry/core'; import { captureException } from '@sentry/node'; import { addExceptionMechanism, dynamicSamplingContextToSentryBaggageHeader, objectify } from '@sentry/utils'; import type { Handle, ResolveOptions } from '@sveltejs/kit'; -import * as domain from 'domain'; import { getTracePropagationData } from './utils'; @@ -67,9 +66,9 @@ export const sentryHandle: Handle = input => { if (getCurrentHub().getScope().getSpan()) { return instrumentHandle(input); } - return domain.create().bind(() => { + return runWithAsyncContext(() => { return instrumentHandle(input); - })(); + }); }; function instrumentHandle({ event, resolve }: Parameters[0]): ReturnType {