diff --git a/packages/core/src/hub.ts b/packages/core/src/hub.ts index 46fd2d0f7139..bc3e1ef4fd38 100644 --- a/packages/core/src/hub.ts +++ b/packages/core/src/hub.ts @@ -568,10 +568,10 @@ function getGlobalHub(registry: Carrier = getMainCarrier()): Hub { * * If the carrier does not contain a hub, a new hub is created with the global hub client and scope. */ -export function ensureHubOnCarrier(carrier: Carrier): void { +export function ensureHubOnCarrier(carrier: Carrier, parent: Hub = getGlobalHub()): void { // If there's no hub on current domain, or it's an old API, assign a new one if (!hasHubOnCarrier(carrier) || getHubFromCarrier(carrier).isOlderThan(API_VERSION)) { - const globalHubTopStack = getGlobalHub().getStackTop(); + const globalHubTopStack = parent.getStackTop(); setHubOnCarrier(carrier, new Hub(globalHubTopStack.client, Scope.clone(globalHubTopStack.scope))); } } diff --git a/packages/node/src/async/domain.ts b/packages/node/src/async/domain.ts index b41ae623d594..371b2451e1b8 100644 --- a/packages/node/src/async/domain.ts +++ b/packages/node/src/async/domain.ts @@ -1,10 +1,5 @@ import type { Carrier, Hub, RunWithAsyncContextOptions } from '@sentry/core'; -import { - ensureHubOnCarrier, - getCurrentHub as getCurrentHubCore, - getHubFromCarrier, - setAsyncContextStrategy, -} from '@sentry/core'; +import { ensureHubOnCarrier, getHubFromCarrier, setAsyncContextStrategy, setHubOnCarrier } from '@sentry/core'; import * as domain from 'domain'; import { EventEmitter } from 'events'; @@ -26,23 +21,27 @@ function getCurrentHub(): Hub | undefined { return getHubFromCarrier(activeDomain); } +function createNewHub(parent: Hub | undefined): Hub { + const carrier: Carrier = {}; + ensureHubOnCarrier(carrier, parent); + return getHubFromCarrier(carrier); +} + function runWithAsyncContext(callback: (hub: Hub) => T, options: RunWithAsyncContextOptions): T { - if (options?.reuseExisting) { - const activeDomain = getActiveDomain(); + const activeDomain = getActiveDomain(); - if (activeDomain) { - for (const emitter of options.emitters || []) { - if (emitter instanceof EventEmitter) { - activeDomain.add(emitter); - } + if (activeDomain && options?.reuseExisting) { + for (const emitter of options.emitters || []) { + if (emitter instanceof EventEmitter) { + activeDomain.add(emitter); } - - // We're already in a domain, so we don't need to create a new one, just call the callback with the current hub - return callback(getHubFromCarrier(activeDomain)); } + + // We're already in a domain, so we don't need to create a new one, just call the callback with the current hub + return callback(getHubFromCarrier(activeDomain)); } - const local = domain.create(); + const local = domain.create() as domain.Domain & Carrier; for (const emitter of options.emitters || []) { if (emitter instanceof EventEmitter) { @@ -50,9 +49,12 @@ function runWithAsyncContext(callback: (hub: Hub) => T, options: RunWithAsync } } + const parentHub = activeDomain ? getHubFromCarrier(activeDomain) : undefined; + const newHub = createNewHub(parentHub); + setHubOnCarrier(local, newHub); + return local.bind(() => { - const hub = getCurrentHubCore(); - return callback(hub); + return callback(newHub); })(); } diff --git a/packages/node/test/async/domain.test.ts b/packages/node/test/async/domain.test.ts index 9a1f39ee5b23..445718de227d 100644 --- a/packages/node/test/async/domain.test.ts +++ b/packages/node/test/async/domain.test.ts @@ -16,19 +16,29 @@ describe('domains', () => { expect(hub).toEqual(new Hub()); }); - test('domain hub scope inheritance', () => { + test('hub scope inheritance', () => { + setDomainAsyncContextStrategy(); + const globalHub = getCurrentHub(); - globalHub.configureScope(scope => { - scope.setExtra('a', 'b'); - scope.setTag('a', 'b'); - scope.addBreadcrumb({ message: 'a' }); - }); - runWithAsyncContext(hub => { - expect(globalHub).toEqual(hub); + globalHub.setExtra('a', 'b'); + + runWithAsyncContext(hub1 => { + expect(hub1).toEqual(globalHub); + + hub1.setExtra('c', 'd'); + expect(hub1).not.toEqual(globalHub); + + runWithAsyncContext(hub2 => { + expect(hub2).toEqual(hub1); + expect(hub2).not.toEqual(globalHub); + + hub2.setExtra('e', 'f'); + expect(hub2).not.toEqual(hub1); + }); }); }); - test('domain hub single instance', () => { + test('hub single instance', () => { setDomainAsyncContextStrategy(); runWithAsyncContext(hub => { @@ -36,7 +46,7 @@ describe('domains', () => { }); }); - test('domain within a domain not reused', () => { + test('within a domain not reused', () => { setDomainAsyncContextStrategy(); runWithAsyncContext(hub1 => { @@ -46,7 +56,7 @@ describe('domains', () => { }); }); - test('domain within a domain reused when requested', () => { + test('within a domain reused when requested', () => { setDomainAsyncContextStrategy(); runWithAsyncContext(hub1 => { @@ -59,7 +69,7 @@ describe('domains', () => { }); }); - test('concurrent domain hubs', done => { + test('concurrent hub contexts', done => { setDomainAsyncContextStrategy(); let d1done = false; diff --git a/packages/node/test/handlers.test.ts b/packages/node/test/handlers.test.ts index 4f7060b655ff..3cf5127ce848 100644 --- a/packages/node/test/handlers.test.ts +++ b/packages/node/test/handlers.test.ts @@ -1,16 +1,26 @@ +import type { Hub } from '@sentry/core'; import * as sentryCore from '@sentry/core'; -import { Transaction } from '@sentry/core'; +import { setAsyncContextStrategy, Transaction } from '@sentry/core'; 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(); +function mockAsyncContextStrategy(getHub: () => Hub): void { + function getCurrentHub(): Hub | undefined { + return getHub(); + } + + function runWithAsyncContext(fn: (hub: Hub) => T): T { + return fn(getHub()); + } + + setAsyncContextStrategy({ getCurrentHub, runWithAsyncContext }); +} describe('requestHandler', () => { const headers = { ears: 'furry', nose: 'wet', tongue: 'spotted', cookie: 'favorite=zukes' }; @@ -52,6 +62,7 @@ describe('requestHandler', () => { const hub = new sentryCore.Hub(client); jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub); + mockAsyncContextStrategy(() => hub); sentryRequestMiddleware(req, res, next); @@ -65,6 +76,7 @@ describe('requestHandler', () => { const hub = new sentryCore.Hub(client); jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub); + mockAsyncContextStrategy(() => hub); sentryRequestMiddleware(req, res, next); @@ -78,6 +90,7 @@ describe('requestHandler', () => { const hub = new sentryCore.Hub(client); jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub); + mockAsyncContextStrategy(() => hub); const captureRequestSession = jest.spyOn(client, '_captureRequestSession'); @@ -97,7 +110,9 @@ describe('requestHandler', () => { const options = getDefaultNodeClientOptions({ autoSessionTracking: false, release: '1.2' }); client = new NodeClient(options); const hub = new sentryCore.Hub(client); + jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub); + mockAsyncContextStrategy(() => hub); const captureRequestSession = jest.spyOn(client, '_captureRequestSession'); @@ -142,6 +157,7 @@ describe('requestHandler', () => { it('stores request and request data options in `sdkProcessingMetadata`', () => { const hub = new sentryCore.Hub(new NodeClient(getDefaultNodeClientOptions())); jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub); + mockAsyncContextStrategy(() => hub); const requestHandlerOptions = { include: { ip: false } }; const sentryRequestMiddleware = requestHandler(requestHandlerOptions); @@ -177,6 +193,7 @@ describe('tracingHandler', () => { beforeEach(() => { hub = new sentryCore.Hub(new NodeClient(getDefaultNodeClientOptions({ tracesSampleRate: 1.0 }))); jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub); + mockAsyncContextStrategy(() => hub); req = { headers, method, @@ -274,6 +291,8 @@ describe('tracingHandler', () => { const tracesSampler = jest.fn(); const options = getDefaultNodeClientOptions({ tracesSampler }); const hub = new sentryCore.Hub(new NodeClient(options)); + mockAsyncContextStrategy(() => hub); + hub.run(() => { sentryTracingMiddleware(req, res, next); @@ -296,6 +315,7 @@ describe('tracingHandler', () => { const hub = new sentryCore.Hub(new NodeClient(options)); jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub); + mockAsyncContextStrategy(() => hub); sentryTracingMiddleware(req, res, next); @@ -502,14 +522,17 @@ describe('errorHandler()', () => { client.initSessionFlusher(); const scope = new sentryCore.Scope(); const hub = new sentryCore.Hub(client, scope); + mockAsyncContextStrategy(() => hub); jest.spyOn(client, '_captureRequestSession'); hub.run(() => { scope?.setRequestSession({ status: 'ok' }); - sentryErrorMiddleware({ name: 'error', message: 'this is an error' }, req, res, next); - const requestSession = scope?.getRequestSession(); - expect(requestSession).toEqual({ status: 'crashed' }); + sentryErrorMiddleware({ name: 'error', message: 'this is an error' }, req, res, () => { + const scope = sentryCore.getCurrentHub().getScope(); + const requestSession = scope?.getRequestSession(); + expect(requestSession).toEqual({ status: 'crashed' }); + }); }); }); @@ -535,6 +558,7 @@ describe('errorHandler()', () => { client = new NodeClient(options); const hub = new sentryCore.Hub(client); + mockAsyncContextStrategy(() => hub); sentryCore.makeMain(hub); // `sentryErrorMiddleware` uses `withScope`, and we need access to the temporary scope it creates, so monkeypatch