feat(core): Deprecate pushScope & popScope (#9890)

This deprecates using `pushScope` / `popScope` on the hub.

Author: mydea

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:

packages/browser/test/unit/index.test.ts

@@ -37,43 +37,39 @@ 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,
       transport: makeSimpleTransport,
     });
   });
 
-  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',
       });
     });

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<T>(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();
     }
   }

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://[email protected]/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',
       });
     });

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);
 

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 });
 

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;