Skip to content

Commit eed0dc9

Browse files
author
Luca Forstner
authored
ref(core): Don't pass hub to runWithAsyncContext callback (#7850)
1 parent 970e108 commit eed0dc9

File tree

11 files changed

+88
-44
lines changed

11 files changed

+88
-44
lines changed

‎packages/core/src/hub.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ export interface AsyncContextStrategy {
6060
/**
6161
* Runs the supplied callback in its own async context.
6262
*/
63-
runWithAsyncContext<T>(callback: (hub: Hub) => T, options: RunWithAsyncContextOptions): T;
63+
runWithAsyncContext<T>(callback: () => T, options: RunWithAsyncContextOptions): T;
6464
}
6565

6666
/**
@@ -593,15 +593,15 @@ export function setAsyncContextStrategy(strategy: AsyncContextStrategy | undefin
593593
* @param options Options to pass to the async context strategy
594594
* @returns The result of the callback
595595
*/
596-
export function runWithAsyncContext<T>(callback: (hub: Hub) => T, options: RunWithAsyncContextOptions = {}): T {
596+
export function runWithAsyncContext<T>(callback: () => T, options: RunWithAsyncContextOptions = {}): T {
597597
const registry = getMainCarrier();
598598

599599
if (registry.__SENTRY__ && registry.__SENTRY__.acs) {
600600
return registry.__SENTRY__.acs.runWithAsyncContext(callback, options);
601601
}
602602

603603
// if there was no strategy, fallback to just calling the callback
604-
return callback(getCurrentHub());
604+
return callback();
605605
}
606606

607607
/**

‎packages/nextjs/src/server/utils/wrapperUtils.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,10 @@
1-
import { captureException, getActiveTransaction, runWithAsyncContext, startTransaction } from '@sentry/core';
1+
import {
2+
captureException,
3+
getActiveTransaction,
4+
getCurrentHub,
5+
runWithAsyncContext,
6+
startTransaction,
7+
} from '@sentry/core';
28
import type { Transaction } from '@sentry/types';
39
import { baggageHeaderToDynamicSamplingContext, extractTraceparentData } from '@sentry/utils';
410
import type { IncomingMessage, ServerResponse } from 'http';
@@ -74,7 +80,8 @@ export function withTracedServerSideDataFetcher<F extends (...args: any[]) => Pr
7480
},
7581
): (...params: Parameters<F>) => Promise<ReturnType<F>> {
7682
return async function (this: unknown, ...args: Parameters<F>): Promise<ReturnType<F>> {
77-
return runWithAsyncContext(async hub => {
83+
return runWithAsyncContext(async () => {
84+
const hub = getCurrentHub();
7885
let requestTransaction: Transaction | undefined = getTransactionFromRequest(req);
7986
let dataFetcherSpan;
8087

‎packages/nextjs/src/server/wrapApiHandlerWithSentry.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { hasTracingEnabled, runWithAsyncContext } from '@sentry/core';
1+
import { getCurrentHub, hasTracingEnabled, runWithAsyncContext } from '@sentry/core';
22
import { captureException, startTransaction } from '@sentry/node';
33
import type { Transaction } from '@sentry/types';
44
import {
@@ -63,7 +63,8 @@ export function withSentry(apiHandler: NextApiHandler, parameterizedRoute?: stri
6363
// eslint-disable-next-line complexity, @typescript-eslint/no-explicit-any
6464
const boundHandler = runWithAsyncContext(
6565
// eslint-disable-next-line complexity
66-
async hub => {
66+
async () => {
67+
const hub = getCurrentHub();
6768
let transaction: Transaction | undefined;
6869
const currentScope = hub.getScope();
6970
const options = hub.getClient()?.getOptions();

‎packages/nextjs/src/server/wrapServerComponentWithSentry.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,10 @@
1-
import { addTracingExtensions, captureException, runWithAsyncContext, startTransaction } from '@sentry/core';
1+
import {
2+
addTracingExtensions,
3+
captureException,
4+
getCurrentHub,
5+
runWithAsyncContext,
6+
startTransaction,
7+
} from '@sentry/core';
28
import { baggageHeaderToDynamicSamplingContext, extractTraceparentData } from '@sentry/utils';
39

410
import { isNotFoundNavigationError, isRedirectNavigationError } from '../common/nextNavigationErrorUtils';
@@ -20,7 +26,9 @@ export function wrapServerComponentWithSentry<F extends (...args: any[]) => any>
2026
// hook. 🤯
2127
return new Proxy(appDirComponent, {
2228
apply: (originalFunction, thisArg, args) => {
23-
return runWithAsyncContext(hub => {
29+
return runWithAsyncContext(() => {
30+
const hub = getCurrentHub();
31+
2432
let maybePromiseResult;
2533

2634
const traceparentData = context.sentryTraceHeader

‎packages/nextjs/test/serverSdk.test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,8 @@ describe('Server init()', () => {
117117
it("initializes both global hub and domain hub when there's an active domain", () => {
118118
const globalHub = getCurrentHub();
119119

120-
runWithAsyncContext(globalHub2 => {
120+
runWithAsyncContext(() => {
121+
const globalHub2 = getCurrentHub();
121122
// If we call runWithAsyncContext before init, it executes the callback in the same context as there is no
122123
// strategy yet
123124
expect(globalHub2).toBe(globalHub);
@@ -126,7 +127,8 @@ describe('Server init()', () => {
126127

127128
init({});
128129

129-
runWithAsyncContext(domainHub => {
130+
runWithAsyncContext(() => {
131+
const domainHub = getCurrentHub();
130132
// this tag should end up only in the domain hub
131133
domainHub.setTag('dogs', 'areGreat');
132134

‎packages/node/src/async/domain.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,12 @@ function createNewHub(parent: Hub | undefined): Hub {
2626
return getHubFromCarrier(carrier);
2727
}
2828

29-
function runWithAsyncContext<T>(callback: (hub: Hub) => T, options: RunWithAsyncContextOptions): T {
29+
function runWithAsyncContext<T>(callback: () => T, options: RunWithAsyncContextOptions): T {
3030
const activeDomain = getActiveDomain<domain.Domain & Carrier>();
3131

3232
if (activeDomain && options?.reuseExisting) {
3333
// We're already in a domain, so we don't need to create a new one, just call the callback with the current hub
34-
return callback(getHubFromCarrier(activeDomain));
34+
return callback();
3535
}
3636

3737
const local = domain.create() as domain.Domain & Carrier;
@@ -41,7 +41,7 @@ function runWithAsyncContext<T>(callback: (hub: Hub) => T, options: RunWithAsync
4141
setHubOnCarrier(local, newHub);
4242

4343
return local.bind(() => {
44-
return callback(newHub);
44+
return callback();
4545
})();
4646
}
4747

‎packages/node/src/async/hooks.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,19 +28,19 @@ export function setHooksAsyncContextStrategy(): void {
2828
return getHubFromCarrier(carrier);
2929
}
3030

31-
function runWithAsyncContext<T>(callback: (hub: Hub) => T, options: RunWithAsyncContextOptions): T {
31+
function runWithAsyncContext<T>(callback: () => T, options: RunWithAsyncContextOptions): T {
3232
const existingHub = getCurrentHub();
3333

3434
if (existingHub && options?.reuseExisting) {
3535
// We're already in an async context, so we don't need to create a new one
3636
// just call the callback with the current hub
37-
return callback(existingHub);
37+
return callback();
3838
}
3939

4040
const newHub = createNewHub(existingHub);
4141

4242
return asyncStorage.run(newHub, () => {
43-
return callback(newHub);
43+
return callback();
4444
});
4545
}
4646

‎packages/node/src/handlers.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,8 @@ export function requestHandler(
187187
});
188188
};
189189
}
190-
runWithAsyncContext(currentHub => {
190+
runWithAsyncContext(() => {
191+
const currentHub = getCurrentHub();
191192
currentHub.configureScope(scope => {
192193
scope.setSDKProcessingMetadata({
193194
request: req,

‎packages/node/test/async/domain.test.ts

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,15 @@ describe('domains', () => {
2222
const globalHub = getCurrentHub();
2323
globalHub.setExtra('a', 'b');
2424

25-
runWithAsyncContext(hub1 => {
25+
runWithAsyncContext(() => {
26+
const hub1 = getCurrentHub();
2627
expect(hub1).toEqual(globalHub);
2728

2829
hub1.setExtra('c', 'd');
2930
expect(hub1).not.toEqual(globalHub);
3031

31-
runWithAsyncContext(hub2 => {
32+
runWithAsyncContext(() => {
33+
const hub2 = getCurrentHub();
3234
expect(hub2).toEqual(hub1);
3335
expect(hub2).not.toEqual(globalHub);
3436

@@ -53,13 +55,15 @@ describe('domains', () => {
5355
const globalHub = getCurrentHub();
5456
await addRandomExtra(globalHub, 'a');
5557

56-
await runWithAsyncContext(async hub1 => {
58+
await runWithAsyncContext(async () => {
59+
const hub1 = getCurrentHub();
5760
expect(hub1).toEqual(globalHub);
5861

5962
await addRandomExtra(hub1, 'b');
6063
expect(hub1).not.toEqual(globalHub);
6164

62-
await runWithAsyncContext(async hub2 => {
65+
await runWithAsyncContext(async () => {
66+
const hub2 = getCurrentHub();
6367
expect(hub2).toEqual(hub1);
6468
expect(hub2).not.toEqual(globalHub);
6569

@@ -72,16 +76,19 @@ describe('domains', () => {
7276
test('hub single instance', () => {
7377
setDomainAsyncContextStrategy();
7478

75-
runWithAsyncContext(hub => {
79+
runWithAsyncContext(() => {
80+
const hub = getCurrentHub();
7681
expect(hub).toBe(getCurrentHub());
7782
});
7883
});
7984

8085
test('within a domain not reused', () => {
8186
setDomainAsyncContextStrategy();
8287

83-
runWithAsyncContext(hub1 => {
84-
runWithAsyncContext(hub2 => {
88+
runWithAsyncContext(() => {
89+
const hub1 = getCurrentHub();
90+
runWithAsyncContext(() => {
91+
const hub2 = getCurrentHub();
8592
expect(hub1).not.toBe(hub2);
8693
});
8794
});
@@ -90,9 +97,11 @@ describe('domains', () => {
9097
test('within a domain reused when requested', () => {
9198
setDomainAsyncContextStrategy();
9299

93-
runWithAsyncContext(hub1 => {
100+
runWithAsyncContext(() => {
101+
const hub1 = getCurrentHub();
94102
runWithAsyncContext(
95-
hub2 => {
103+
() => {
104+
const hub2 = getCurrentHub();
96105
expect(hub1).toBe(hub2);
97106
},
98107
{ reuseExisting: true },
@@ -106,7 +115,8 @@ describe('domains', () => {
106115
let d1done = false;
107116
let d2done = false;
108117

109-
runWithAsyncContext(hub => {
118+
runWithAsyncContext(() => {
119+
const hub = getCurrentHub();
110120
hub.getStack().push({ client: 'process' } as any);
111121
expect(hub.getStack()[1]).toEqual({ client: 'process' });
112122
// Just in case so we don't have to worry which one finishes first
@@ -119,7 +129,8 @@ describe('domains', () => {
119129
});
120130
});
121131

122-
runWithAsyncContext(hub => {
132+
runWithAsyncContext(() => {
133+
const hub = getCurrentHub();
123134
hub.getStack().push({ client: 'local' } as any);
124135
expect(hub.getStack()[1]).toEqual({ client: 'local' });
125136
setTimeout(() => {

‎packages/node/test/async/hooks.test.ts

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,10 @@ conditionalTest({ min: 12 })('async_hooks', () => {
1515
});
1616

1717
test('without strategy hubs should be equal', () => {
18-
runWithAsyncContext(hub1 => {
19-
runWithAsyncContext(hub2 => {
18+
runWithAsyncContext(() => {
19+
const hub1 = getCurrentHub();
20+
runWithAsyncContext(() => {
21+
const hub2 = getCurrentHub();
2022
expect(hub1).toBe(hub2);
2123
});
2224
});
@@ -28,13 +30,15 @@ conditionalTest({ min: 12 })('async_hooks', () => {
2830
const globalHub = getCurrentHub();
2931
globalHub.setExtra('a', 'b');
3032

31-
runWithAsyncContext(hub1 => {
33+
runWithAsyncContext(() => {
34+
const hub1 = getCurrentHub();
3235
expect(hub1).toEqual(globalHub);
3336

3437
hub1.setExtra('c', 'd');
3538
expect(hub1).not.toEqual(globalHub);
3639

37-
runWithAsyncContext(hub2 => {
40+
runWithAsyncContext(() => {
41+
const hub2 = getCurrentHub();
3842
expect(hub2).toEqual(hub1);
3943
expect(hub2).not.toEqual(globalHub);
4044

@@ -59,13 +63,15 @@ conditionalTest({ min: 12 })('async_hooks', () => {
5963
const globalHub = getCurrentHub();
6064
await addRandomExtra(globalHub, 'a');
6165

62-
await runWithAsyncContext(async hub1 => {
66+
await runWithAsyncContext(async () => {
67+
const hub1 = getCurrentHub();
6368
expect(hub1).toEqual(globalHub);
6469

6570
await addRandomExtra(hub1, 'b');
6671
expect(hub1).not.toEqual(globalHub);
6772

68-
await runWithAsyncContext(async hub2 => {
73+
await runWithAsyncContext(async () => {
74+
const hub2 = getCurrentHub();
6975
expect(hub2).toEqual(hub1);
7076
expect(hub2).not.toEqual(globalHub);
7177

@@ -78,16 +84,19 @@ conditionalTest({ min: 12 })('async_hooks', () => {
7884
test('context single instance', () => {
7985
setHooksAsyncContextStrategy();
8086

81-
runWithAsyncContext(hub => {
82-
expect(hub).toBe(getCurrentHub());
87+
const globalHub = getCurrentHub();
88+
runWithAsyncContext(() => {
89+
expect(globalHub).not.toBe(getCurrentHub());
8390
});
8491
});
8592

8693
test('context within a context not reused', () => {
8794
setHooksAsyncContextStrategy();
8895

89-
runWithAsyncContext(hub1 => {
90-
runWithAsyncContext(hub2 => {
96+
runWithAsyncContext(() => {
97+
const hub1 = getCurrentHub();
98+
runWithAsyncContext(() => {
99+
const hub2 = getCurrentHub();
91100
expect(hub1).not.toBe(hub2);
92101
});
93102
});
@@ -96,9 +105,11 @@ conditionalTest({ min: 12 })('async_hooks', () => {
96105
test('context within a context reused when requested', () => {
97106
setHooksAsyncContextStrategy();
98107

99-
runWithAsyncContext(hub1 => {
108+
runWithAsyncContext(() => {
109+
const hub1 = getCurrentHub();
100110
runWithAsyncContext(
101-
hub2 => {
111+
() => {
112+
const hub2 = getCurrentHub();
102113
expect(hub1).toBe(hub2);
103114
},
104115
{ reuseExisting: true },
@@ -112,7 +123,8 @@ conditionalTest({ min: 12 })('async_hooks', () => {
112123
let d1done = false;
113124
let d2done = false;
114125

115-
runWithAsyncContext(hub => {
126+
runWithAsyncContext(() => {
127+
const hub = getCurrentHub();
116128
hub.getStack().push({ client: 'process' } as any);
117129
expect(hub.getStack()[1]).toEqual({ client: 'process' });
118130
// Just in case so we don't have to worry which one finishes first
@@ -125,7 +137,8 @@ conditionalTest({ min: 12 })('async_hooks', () => {
125137
});
126138
});
127139

128-
runWithAsyncContext(hub => {
140+
runWithAsyncContext(() => {
141+
const hub = getCurrentHub();
129142
hub.getStack().push({ client: 'local' } as any);
130143
expect(hub.getStack()[1]).toEqual({ client: 'local' });
131144
setTimeout(() => {

‎packages/node/test/index.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,8 @@ describe('SentryNode', () => {
291291
setNodeAsyncContextStrategy();
292292
const client = new NodeClient(options);
293293

294-
runWithAsyncContext(hub => {
294+
runWithAsyncContext(() => {
295+
const hub = getCurrentHub();
295296
hub.bindClient(client);
296297
expect(getCurrentHub().getClient()).toBe(client);
297298
hub.captureEvent({ message: 'test domain' });

0 commit comments

Comments
 (0)