From 58fc4c91c184b6d4f65160e3ce84d9c2da05df84 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 4 Apr 2023 22:24:25 +0200 Subject: [PATCH] Replace usages if `domain.run` and `domain.bind` with `runWithHub` --- .../nextjs/src/server/utils/wrapperUtils.ts | 6 +- .../src/server/wrapApiHandlerWithSentry.ts | 12 +--- .../server/wrapServerComponentWithSentry.ts | 6 +- packages/node/src/handlers.ts | 70 +++++++++---------- packages/node/src/index.ts | 2 +- packages/node/src/utils.ts | 20 ++++++ packages/remix/src/utils/instrumentServer.ts | 9 +-- packages/sveltekit/src/server/handle.ts | 7 +- 8 files changed, 70 insertions(+), 62 deletions(-) diff --git a/packages/nextjs/src/server/utils/wrapperUtils.ts b/packages/nextjs/src/server/utils/wrapperUtils.ts index 9fa91fbbee5a..827f53359a76 100644 --- a/packages/nextjs/src/server/utils/wrapperUtils.ts +++ b/packages/nextjs/src/server/utils/wrapperUtils.ts @@ -1,7 +1,7 @@ import { captureException, getActiveTransaction, getCurrentHub, startTransaction } from '@sentry/core'; +import { runWithHub } from '@sentry/node'; 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 +75,7 @@ export function withTracedServerSideDataFetcher Pr }, ): (...params: Parameters) => Promise> { return async function (this: unknown, ...args: Parameters): Promise> { - return domain.create().bind(async () => { + return runWithHub(async () => { let requestTransaction: Transaction | undefined = getTransactionFromRequest(req); let dataFetcherSpan; @@ -154,7 +154,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..87d5c4c55201 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 { captureException, getCurrentHub, runWithHub, 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,16 +60,11 @@ 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 () => { + const boundHandler = runWithHub(async () => { let transaction: Transaction | undefined; const hub = getCurrentHub(); const currentScope = hub.getScope(); @@ -213,7 +207,7 @@ export function withSentry(apiHandler: NextApiHandler, parameterizedRoute?: stri // 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..23052d133769 100644 --- a/packages/nextjs/src/server/wrapServerComponentWithSentry.ts +++ b/packages/nextjs/src/server/wrapServerComponentWithSentry.ts @@ -1,6 +1,6 @@ import { addTracingExtensions, captureException, getCurrentHub, startTransaction } from '@sentry/core'; +import { runWithHub } from '@sentry/node'; 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 +21,7 @@ export function wrapServerComponentWithSentry any> // hook. 🤯 return new Proxy(appDirComponent, { apply: (originalFunction, thisArg, args) => { - return domain.create().bind(() => { + return runWithHub(() => { let maybePromiseResult; const traceparentData = context.sentryTraceHeader @@ -85,7 +85,7 @@ export function wrapServerComponentWithSentry any> transaction.finish(); return maybePromiseResult; } - })(); + }); }, }); } diff --git a/packages/node/src/handlers.ts b/packages/node/src/handlers.ts index 5dfcb7bf0a13..cb2e9350cc8c 100644 --- a/packages/node/src/handlers.ts +++ b/packages/node/src/handlers.ts @@ -12,7 +12,6 @@ import { logger, normalize, } from '@sentry/utils'; -import * as domain from 'domain'; import type * as http from 'http'; import type { NodeClient } from './client'; @@ -20,6 +19,7 @@ import { extractRequestData } from './requestdata'; // TODO (v8 / XXX) Remove this import import type { ParseRequestOptions } from './requestDataDeprecated'; import { flush, isAutoSessionTrackingEnabled } from './sdk'; +import { runWithHub } from './utils'; /** * Express-compatible tracing handler. @@ -181,46 +181,44 @@ 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, - }); + runWithHub( + 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(); + }, + [req, res], + ); }; } diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index 7b4e3fe5d56b..bf962c59e1cd 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -57,7 +57,7 @@ export { NodeClient } from './client'; export { makeNodeTransport } from './transports'; export { defaultIntegrations, init, defaultStackParser, lastEventId, flush, close, getSentryRelease } from './sdk'; export { addRequestDataToEvent, DEFAULT_USER_INCLUDES, extractRequestData } from './requestdata'; -export { deepReadDirSync } from './utils'; +export { deepReadDirSync, runWithHub } from './utils'; import { getMainCarrier, Integrations as CoreIntegrations } from '@sentry/core'; import * as domain from 'domain'; diff --git a/packages/node/src/utils.ts b/packages/node/src/utils.ts index 87fb8f6157e3..af81e0e31a5c 100644 --- a/packages/node/src/utils.ts +++ b/packages/node/src/utils.ts @@ -1,3 +1,7 @@ +import type { Hub } from '@sentry/core'; +import { getCurrentHub } from '@sentry/core'; +import * as domain from 'domain'; +import type { EventEmitter } from 'events'; import * as fs from 'fs'; import * as path from 'path'; @@ -36,3 +40,19 @@ export function deepReadDirSync(targetDir: string): string[] { return deepReadCurrentDir(targetDirAbsPath).map(absPath => path.relative(targetDirAbsPath, absPath)); } + +/** + * Runs a callback in it's own domain and passes it the hub. + */ +export function runWithHub(callback: (hub: Hub) => T, emitters: EventEmitter[] = []): T { + const local = domain.create(); + + for (const emitter of emitters) { + local.add(emitter); + } + + return local.bind(() => { + const hub = getCurrentHub(); + return callback(hub); + })(); +} diff --git a/packages/remix/src/utils/instrumentServer.ts b/packages/remix/src/utils/instrumentServer.ts index 8e916d75ffc4..99ccbeb704d4 100644 --- a/packages/remix/src/utils/instrumentServer.ts +++ b/packages/remix/src/utils/instrumentServer.ts @@ -1,7 +1,7 @@ /* eslint-disable max-lines */ import { getActiveTransaction, hasTracingEnabled } from '@sentry/core'; import type { Hub } from '@sentry/node'; -import { captureException, getCurrentHub } from '@sentry/node'; +import { captureException, getCurrentHub, runWithHub } from '@sentry/node'; import type { Transaction, TransactionSource, WrappedFunction } from '@sentry/types'; import { addExceptionMechanism, @@ -13,7 +13,6 @@ import { loadModule, logger, } from '@sentry/utils'; -import * as domain from 'domain'; import type { AppData, @@ -314,13 +313,11 @@ 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 runWithHub(async () => { const hub = getCurrentHub(); const options = hub.getClient()?.getOptions(); const scope = hub.getScope(); @@ -365,7 +362,7 @@ function wrapRequestHandler(origRequestHandler: RequestHandler, build: ServerBui transaction.finish(); return res; - })(); + }); }; } diff --git a/packages/sveltekit/src/server/handle.ts b/packages/sveltekit/src/server/handle.ts index cc291e63f579..1f1aafb880b8 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 { captureException } from '@sentry/node'; +import { captureException, runWithHub } 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 runWithHub(() => { return instrumentHandle(input); - })(); + }); }; function instrumentHandle({ event, resolve }: Parameters[0]): ReturnType {