Skip to content

Commit f644fd6

Browse files
mydeaLuca Forstner
mydea
and
Luca Forstner
authored
feat(core): Allow to pass scope to withIsolationScope() (#11478)
Also adds tests for this for core, vercel-edge and opentelemetry. NOTE: In core, this does not do anything, as we cannot actually update the isolation scope in browser. Co-authored-by: Luca Forstner <[email protected]>
1 parent d7c6fe3 commit f644fd6

File tree

5 files changed

+436
-32
lines changed

5 files changed

+436
-32
lines changed

‎packages/core/src/currentScopes.ts

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,15 +74,44 @@ export function withScope<T>(
7474
*
7575
* This function is intended for Sentry SDK and SDK integration development. It is not recommended to be used in "normal"
7676
* applications directly because it comes with pitfalls. Use at your own risk!
77+
*/
78+
export function withIsolationScope<T>(callback: (isolationScope: Scope) => T): T;
79+
/**
80+
* Set the provided isolation scope as active in the given callback. If no
81+
* async context strategy is set, the isolation scope and the current scope will not be forked (this is currently the
82+
* case, for example, in the browser).
83+
*
84+
* Usage of this function in environments without async context strategy is discouraged and may lead to unexpected behaviour.
7785
*
78-
* @param callback The callback in which the passed isolation scope is active. (Note: In environments without async
79-
* context strategy, the currently active isolation scope may change within execution of the callback.)
80-
* @returns The same value that `callback` returns.
86+
* This function is intended for Sentry SDK and SDK integration development. It is not recommended to be used in "normal"
87+
* applications directly because it comes with pitfalls. Use at your own risk!
88+
*
89+
* If you pass in `undefined` as a scope, it will fork a new isolation scope, the same as if no scope is passed.
90+
*/
91+
export function withIsolationScope<T>(isolationScope: Scope | undefined, callback: (isolationScope: Scope) => T): T;
92+
/**
93+
* Either creates a new active isolation scope, or sets the given isolation scope as active scope in the given callback.
8194
*/
82-
export function withIsolationScope<T>(callback: (isolationScope: Scope) => T): T {
95+
export function withIsolationScope<T>(
96+
...rest:
97+
| [callback: (isolationScope: Scope) => T]
98+
| [isolationScope: Scope | undefined, callback: (isolationScope: Scope) => T]
99+
): T {
83100
const carrier = getMainCarrier();
84101
const acs = getAsyncContextStrategy(carrier);
85-
return acs.withIsolationScope(callback);
102+
103+
// If a scope is defined, we want to make this the active scope instead of the default one
104+
if (rest.length === 2) {
105+
const [isolationScope, callback] = rest;
106+
107+
if (!isolationScope) {
108+
return acs.withIsolationScope(callback);
109+
}
110+
111+
return acs.withSetIsolationScope(isolationScope, callback);
112+
}
113+
114+
return acs.withIsolationScope(rest[0]);
86115
}
87116

88117
/**

‎packages/core/test/lib/scope.test.ts

Lines changed: 120 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
getGlobalScope,
66
getIsolationScope,
77
withIsolationScope,
8+
withScope,
89
} from '../../src';
910

1011
import { Scope } from '../../src/scope';
@@ -853,40 +854,135 @@ describe('Scope', () => {
853854
});
854855
});
855856

856-
describe('isolation scope', () => {
857-
describe('withIsolationScope()', () => {
858-
it('will pass an isolation scope without Sentry.init()', done => {
859-
expect.assertions(1);
860-
withIsolationScope(scope => {
861-
expect(scope).toBeDefined();
862-
done();
863-
});
857+
describe('withScope()', () => {
858+
beforeEach(() => {
859+
getIsolationScope().clear();
860+
getCurrentScope().clear();
861+
getGlobalScope().clear();
862+
});
863+
864+
it('will make the passed scope the active scope within the callback', done => {
865+
withScope(scope => {
866+
expect(getCurrentScope()).toBe(scope);
867+
done();
868+
});
869+
});
870+
871+
it('will pass a scope that is different from the current active isolation scope', done => {
872+
withScope(scope => {
873+
expect(getIsolationScope()).not.toBe(scope);
874+
done();
864875
});
876+
});
865877

866-
it('will make the passed isolation scope the active isolation scope within the callback', done => {
867-
expect.assertions(1);
868-
withIsolationScope(scope => {
869-
expect(getIsolationScope()).toBe(scope);
878+
it('will always make the inner most passed scope the current isolation scope when nesting calls', done => {
879+
withIsolationScope(_scope1 => {
880+
withIsolationScope(scope2 => {
881+
expect(getIsolationScope()).toBe(scope2);
870882
done();
871883
});
872884
});
885+
});
886+
887+
it('forks the scope when not passing any scope', done => {
888+
const initialScope = getCurrentScope();
889+
initialScope.setTag('aa', 'aa');
873890

874-
it('will pass an isolation scope that is different from the current active scope', done => {
875-
expect.assertions(1);
876-
withIsolationScope(scope => {
877-
expect(getCurrentScope()).not.toBe(scope);
891+
withScope(scope => {
892+
expect(getCurrentScope()).toBe(scope);
893+
scope.setTag('bb', 'bb');
894+
expect(scope).not.toBe(initialScope);
895+
expect(scope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' });
896+
done();
897+
});
898+
});
899+
900+
it('forks the scope when passing undefined', done => {
901+
const initialScope = getCurrentScope();
902+
initialScope.setTag('aa', 'aa');
903+
904+
withScope(undefined, scope => {
905+
expect(getCurrentScope()).toBe(scope);
906+
scope.setTag('bb', 'bb');
907+
expect(scope).not.toBe(initialScope);
908+
expect(scope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' });
909+
done();
910+
});
911+
});
912+
913+
it('sets the passed in scope as active scope', done => {
914+
const initialScope = getCurrentScope();
915+
initialScope.setTag('aa', 'aa');
916+
917+
const customScope = new Scope();
918+
919+
withScope(customScope, scope => {
920+
expect(getCurrentScope()).toBe(customScope);
921+
expect(scope).toBe(customScope);
922+
done();
923+
});
924+
});
925+
});
926+
927+
describe('withIsolationScope()', () => {
928+
beforeEach(() => {
929+
getIsolationScope().clear();
930+
getCurrentScope().clear();
931+
getGlobalScope().clear();
932+
});
933+
934+
it('will make the passed isolation scope the active isolation scope within the callback', done => {
935+
withIsolationScope(scope => {
936+
expect(getIsolationScope()).toBe(scope);
937+
done();
938+
});
939+
});
940+
941+
it('will pass an isolation scope that is different from the current active scope', done => {
942+
withIsolationScope(scope => {
943+
expect(getCurrentScope()).not.toBe(scope);
944+
done();
945+
});
946+
});
947+
948+
it('will always make the inner most passed scope the current scope when nesting calls', done => {
949+
withIsolationScope(_scope1 => {
950+
withIsolationScope(scope2 => {
951+
expect(getIsolationScope()).toBe(scope2);
878952
done();
879953
});
880954
});
955+
});
956+
957+
// Note: This is expected! In browser, we do not actually fork this
958+
it('does not fork isolation scope when not passing any isolation scope', done => {
959+
const isolationScope = getIsolationScope();
960+
961+
withIsolationScope(scope => {
962+
expect(getIsolationScope()).toBe(scope);
963+
expect(scope).toBe(isolationScope);
964+
done();
965+
});
966+
});
881967

882-
it('will always make the inner most passed scope the current scope when nesting calls', done => {
883-
expect.assertions(1);
884-
withIsolationScope(_scope1 => {
885-
withIsolationScope(scope2 => {
886-
expect(getIsolationScope()).toBe(scope2);
887-
done();
888-
});
889-
});
968+
it('does not fork isolation scope when passing undefined', done => {
969+
const isolationScope = getIsolationScope();
970+
971+
withIsolationScope(undefined, scope => {
972+
expect(getIsolationScope()).toBe(scope);
973+
expect(scope).toBe(isolationScope);
974+
done();
975+
});
976+
});
977+
978+
it('ignores passed in isolation scope', done => {
979+
const isolationScope = getIsolationScope();
980+
const customIsolationScope = new Scope();
981+
982+
withIsolationScope(customIsolationScope, scope => {
983+
expect(getIsolationScope()).toBe(isolationScope);
984+
expect(scope).toBe(isolationScope);
985+
done();
890986
});
891987
});
892988
});

‎packages/opentelemetry/test/asyncContextStrategy.test.ts

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { BasicTracerProvider } from '@opentelemetry/sdk-trace-base';
22
import {
3+
Scope as ScopeClass,
34
getCurrentScope,
45
getIsolationScope,
56
setAsyncContextStrategy,
@@ -298,4 +299,132 @@ describe('asyncContextStrategy', () => {
298299
});
299300
});
300301
});
302+
303+
describe('withScope()', () => {
304+
it('will make the passed scope the active scope within the callback', done => {
305+
withScope(scope => {
306+
expect(getCurrentScope()).toBe(scope);
307+
done();
308+
});
309+
});
310+
311+
it('will pass a scope that is different from the current active isolation scope', done => {
312+
withScope(scope => {
313+
expect(getIsolationScope()).not.toBe(scope);
314+
done();
315+
});
316+
});
317+
318+
it('will always make the inner most passed scope the current scope when nesting calls', done => {
319+
withIsolationScope(_scope1 => {
320+
withIsolationScope(scope2 => {
321+
expect(getIsolationScope()).toBe(scope2);
322+
done();
323+
});
324+
});
325+
});
326+
327+
it('forks the scope when not passing any scope', done => {
328+
const initialScope = getCurrentScope();
329+
initialScope.setTag('aa', 'aa');
330+
331+
withScope(scope => {
332+
expect(getCurrentScope()).toBe(scope);
333+
scope.setTag('bb', 'bb');
334+
expect(scope).not.toBe(initialScope);
335+
expect(scope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' });
336+
done();
337+
});
338+
});
339+
340+
it('forks the scope when passing undefined', done => {
341+
const initialScope = getCurrentScope();
342+
initialScope.setTag('aa', 'aa');
343+
344+
withScope(undefined, scope => {
345+
expect(getCurrentScope()).toBe(scope);
346+
scope.setTag('bb', 'bb');
347+
expect(scope).not.toBe(initialScope);
348+
expect(scope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' });
349+
done();
350+
});
351+
});
352+
353+
it('sets the passed in scope as active scope', done => {
354+
const initialScope = getCurrentScope();
355+
initialScope.setTag('aa', 'aa');
356+
357+
const customScope = new ScopeClass();
358+
359+
withScope(customScope, scope => {
360+
expect(getCurrentScope()).toBe(customScope);
361+
expect(scope).toBe(customScope);
362+
done();
363+
});
364+
});
365+
});
366+
367+
describe('withIsolationScope()', () => {
368+
it('will make the passed isolation scope the active isolation scope within the callback', done => {
369+
withIsolationScope(scope => {
370+
expect(getIsolationScope()).toBe(scope);
371+
done();
372+
});
373+
});
374+
375+
it('will pass an isolation scope that is different from the current active scope', done => {
376+
withIsolationScope(scope => {
377+
expect(getCurrentScope()).not.toBe(scope);
378+
done();
379+
});
380+
});
381+
382+
it('will always make the inner most passed scope the current scope when nesting calls', done => {
383+
withIsolationScope(_scope1 => {
384+
withIsolationScope(scope2 => {
385+
expect(getIsolationScope()).toBe(scope2);
386+
done();
387+
});
388+
});
389+
});
390+
391+
it('forks the isolation scope when not passing any isolation scope', done => {
392+
const initialScope = getIsolationScope();
393+
initialScope.setTag('aa', 'aa');
394+
395+
withIsolationScope(scope => {
396+
expect(getIsolationScope()).toBe(scope);
397+
scope.setTag('bb', 'bb');
398+
expect(scope).not.toBe(initialScope);
399+
expect(scope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' });
400+
done();
401+
});
402+
});
403+
404+
it('forks the isolation scope when passing undefined', done => {
405+
const initialScope = getIsolationScope();
406+
initialScope.setTag('aa', 'aa');
407+
408+
withIsolationScope(undefined, scope => {
409+
expect(getIsolationScope()).toBe(scope);
410+
scope.setTag('bb', 'bb');
411+
expect(scope).not.toBe(initialScope);
412+
expect(scope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' });
413+
done();
414+
});
415+
});
416+
417+
it('sets the passed in isolation scope as active isolation scope', done => {
418+
const initialScope = getIsolationScope();
419+
initialScope.setTag('aa', 'aa');
420+
421+
const customScope = new ScopeClass();
422+
423+
withIsolationScope(customScope, scope => {
424+
expect(getIsolationScope()).toBe(customScope);
425+
expect(scope).toBe(customScope);
426+
done();
427+
});
428+
});
429+
});
301430
});

‎packages/vercel-edge/src/async.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,15 @@ interface AsyncLocalStorage<T> {
1111
run<R, TArgs extends any[]>(store: T, callback: (...args: TArgs) => R, ...args: TArgs): R;
1212
}
1313

14-
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any
15-
const MaybeGlobalAsyncLocalStorage = (GLOBAL_OBJ as any).AsyncLocalStorage;
16-
1714
let asyncStorage: AsyncLocalStorage<Hub>;
1815

1916
/**
2017
* Sets the async context strategy to use AsyncLocalStorage which should be available in the edge runtime.
2118
*/
2219
export function setAsyncLocalStorageAsyncContextStrategy(): void {
20+
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any
21+
const MaybeGlobalAsyncLocalStorage = (GLOBAL_OBJ as any).AsyncLocalStorage;
22+
2323
if (!MaybeGlobalAsyncLocalStorage) {
2424
DEBUG_BUILD &&
2525
logger.warn(

0 commit comments

Comments
 (0)