From 8d9a653a64a63b9c8f391886d7cd14ceafc9a9ac Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 18 Dec 2023 14:38:50 +0100 Subject: [PATCH] feat(core): Deprecate `pushScope` & `popScope` --- MIGRATION.md | 4 ++++ packages/browser/test/unit/index.test.ts | 18 +++++++----------- packages/core/src/hub.ts | 6 ++++++ packages/node/test/index.test.ts | 15 ++++++--------- packages/opentelemetry/test/custom/hub.test.ts | 1 + .../test/integration/eventProcessors.test.ts | 17 ++++++----------- packages/types/src/hub.ts | 4 ++++ 7 files changed, 34 insertions(+), 31 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index 40771832279a..101f9de4469d 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -8,6 +8,10 @@ npx @sentry/migr8@latest This will let you select which updates to run, and automatically update your code. Make sure to still review all code changes! +## Deprecate `pushScope` & `popScope` in favor of `withScope` + +Instead of manually pushing/popping a scope, you should use `Sentry.withScope(callback: (scope: Scope))` instead. + ## Deprecate `configureScope` in favor of using `getCurrentScope()` Instead of updating the scope in a callback via `configureScope()`, you should access it via `getCurrentScope()` and configure it directly: diff --git a/packages/browser/test/unit/index.test.ts b/packages/browser/test/unit/index.test.ts index 62bf08d0ee25..5968349adc1a 100644 --- a/packages/browser/test/unit/index.test.ts +++ b/packages/browser/test/unit/index.test.ts @@ -37,9 +37,10 @@ jest.mock('@sentry/core', () => { }); describe('SentryBrowser', () => { - const beforeSend = jest.fn(); + const beforeSend = jest.fn(event => event); - beforeAll(() => { + beforeEach(() => { + WINDOW.__SENTRY__ = { hub: undefined, logger: undefined, globalEventProcessors: [] }; init({ beforeSend, dsn, @@ -47,33 +48,28 @@ describe('SentryBrowser', () => { }); }); - beforeEach(() => { - getCurrentHub().pushScope(); - }); - afterEach(() => { - getCurrentHub().popScope(); - beforeSend.mockReset(); + beforeSend.mockClear(); }); describe('getContext() / setContext()', () => { it('should store/load extra', () => { getCurrentScope().setExtra('abc', { def: [1] }); - expect(global.__SENTRY__.hub._stack[1].scope._extra).toEqual({ + expect(global.__SENTRY__.hub._stack[0].scope._extra).toEqual({ abc: { def: [1] }, }); }); it('should store/load tags', () => { getCurrentScope().setTag('abc', 'def'); - expect(global.__SENTRY__.hub._stack[1].scope._tags).toEqual({ + expect(global.__SENTRY__.hub._stack[0].scope._tags).toEqual({ abc: 'def', }); }); it('should store/load user', () => { getCurrentScope().setUser({ id: 'def' }); - expect(global.__SENTRY__.hub._stack[1].scope._user).toEqual({ + expect(global.__SENTRY__.hub._stack[0].scope._user).toEqual({ id: 'def', }); }); diff --git a/packages/core/src/hub.ts b/packages/core/src/hub.ts index 1d681b555816..75960550081a 100644 --- a/packages/core/src/hub.ts +++ b/packages/core/src/hub.ts @@ -139,6 +139,8 @@ export class Hub implements HubInterface { /** * @inheritDoc + * + * @deprecated Use `withScope` instead. */ public pushScope(): Scope { // We want to clone the content of prev scope @@ -152,6 +154,8 @@ export class Hub implements HubInterface { /** * @inheritDoc + * + * @deprecated Use `withScope` instead. */ public popScope(): boolean { if (this.getStack().length <= 1) return false; @@ -162,10 +166,12 @@ export class Hub implements HubInterface { * @inheritDoc */ public withScope(callback: (scope: Scope) => T): T { + // eslint-disable-next-line deprecation/deprecation const scope = this.pushScope(); try { return callback(scope); } finally { + // eslint-disable-next-line deprecation/deprecation this.popScope(); } } diff --git a/packages/node/test/index.test.ts b/packages/node/test/index.test.ts index 7f9abbdc1072..30658128d2b4 100644 --- a/packages/node/test/index.test.ts +++ b/packages/node/test/index.test.ts @@ -1,5 +1,6 @@ import { LinkedErrors, SDK_VERSION, getMainCarrier, initAndBind, runWithAsyncContext } from '@sentry/core'; import type { EventHint, Integration } from '@sentry/types'; +import { GLOBAL_OBJ } from '@sentry/utils'; import type { Event } from '../src'; import { @@ -33,37 +34,33 @@ const dsn = 'https://53039209a22b4ec1bcc296a3c9fdecd6@sentry.io/4291'; declare var global: any; describe('SentryNode', () => { - beforeAll(() => { + beforeEach(() => { + GLOBAL_OBJ.__SENTRY__ = { hub: undefined, logger: undefined, globalEventProcessors: [] }; init({ dsn }); }); beforeEach(() => { jest.clearAllMocks(); - getCurrentHub().pushScope(); - }); - - afterEach(() => { - getCurrentHub().popScope(); }); describe('getContext() / setContext()', () => { test('store/load extra', async () => { getCurrentScope().setExtra('abc', { def: [1] }); - expect(global.__SENTRY__.hub._stack[1].scope._extra).toEqual({ + expect(global.__SENTRY__.hub._stack[0].scope._extra).toEqual({ abc: { def: [1] }, }); }); test('store/load tags', async () => { getCurrentScope().setTag('abc', 'def'); - expect(global.__SENTRY__.hub._stack[1].scope._tags).toEqual({ + expect(global.__SENTRY__.hub._stack[0].scope._tags).toEqual({ abc: 'def', }); }); test('store/load user', async () => { getCurrentScope().setUser({ id: 'def' }); - expect(global.__SENTRY__.hub._stack[1].scope._user).toEqual({ + expect(global.__SENTRY__.hub._stack[0].scope._user).toEqual({ id: 'def', }); }); diff --git a/packages/opentelemetry/test/custom/hub.test.ts b/packages/opentelemetry/test/custom/hub.test.ts index 3fb707dca18a..e3f2eea70e6b 100644 --- a/packages/opentelemetry/test/custom/hub.test.ts +++ b/packages/opentelemetry/test/custom/hub.test.ts @@ -26,6 +26,7 @@ describe('OpenTelemetryHub', () => { it('pushScope() creates correct scope', () => { const hub = new OpenTelemetryHub(); + // eslint-disable-next-line deprecation/deprecation const scope = hub.pushScope(); expect(scope).toBeInstanceOf(OpenTelemetryScope); diff --git a/packages/replay/test/integration/eventProcessors.test.ts b/packages/replay/test/integration/eventProcessors.test.ts index b9c86a5c5966..41219c75ae6c 100644 --- a/packages/replay/test/integration/eventProcessors.test.ts +++ b/packages/replay/test/integration/eventProcessors.test.ts @@ -1,5 +1,5 @@ -import { getCurrentHub } from '@sentry/core'; -import type { Event, Hub, Scope } from '@sentry/types'; +import { getClient, getCurrentScope } from '@sentry/core'; +import type { Event } from '@sentry/types'; import { BASE_TIMESTAMP } from '..'; import { resetSdkMock } from '../mocks/resetSdkMock'; @@ -9,16 +9,11 @@ import { useFakeTimers } from '../utils/use-fake-timers'; useFakeTimers(); describe('Integration | eventProcessors', () => { - let hub: Hub; - let scope: Scope; - beforeEach(() => { - hub = getCurrentHub(); - scope = hub.pushScope(); + getCurrentScope().clear(); }); afterEach(() => { - hub.popScope(); jest.resetAllMocks(); }); @@ -31,7 +26,7 @@ describe('Integration | eventProcessors', () => { }, }); - const client = hub.getClient()!; + const client = getClient()!; jest.runAllTimers(); const mockTransportSend = jest.spyOn(client.getTransport()!, 'send'); @@ -47,7 +42,7 @@ describe('Integration | eventProcessors', () => { return null; }); - scope.addEventProcessor(handler1); + getCurrentScope().addEventProcessor(handler1); const TEST_EVENT = getTestEventIncremental({ timestamp: BASE_TIMESTAMP }); @@ -58,7 +53,7 @@ describe('Integration | eventProcessors', () => { expect(mockTransportSend).toHaveBeenCalledTimes(1); - scope.addEventProcessor(handler2); + getCurrentScope().addEventProcessor(handler2); const TEST_EVENT2 = getTestEventIncremental({ timestamp: BASE_TIMESTAMP }); diff --git a/packages/types/src/hub.ts b/packages/types/src/hub.ts index 6edcb799b0e8..8d4d47885d40 100644 --- a/packages/types/src/hub.ts +++ b/packages/types/src/hub.ts @@ -40,6 +40,8 @@ export interface Hub { * when the operation finishes or throws. * * @returns Scope, the new cloned scope + * + * @deprecated Use `withScope` instead. */ pushScope(): Scope; @@ -49,6 +51,8 @@ export interface Hub { * This restores the state before the scope was pushed. All breadcrumbs and * context information added since the last call to {@link this.pushScope} are * discarded. + * + * @deprecated Use `withScope` instead. */ popScope(): boolean;