Skip to content

feat(core): Deprecate pushScope & popScope #9890

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
18 changes: 7 additions & 11 deletions packages/browser/test/unit/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
});
});
Expand Down
6 changes: 6 additions & 0 deletions packages/core/src/hub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -152,6 +154,8 @@ export class Hub implements HubInterface {

/**
* @inheritDoc
*
* @deprecated Use `withScope` instead.
*/
public popScope(): boolean {
if (this.getStack().length <= 1) return false;
Expand All @@ -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();
}
}
Expand Down
15 changes: 6 additions & 9 deletions packages/node/test/index.test.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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',
});
});
Expand Down
1 change: 1 addition & 0 deletions packages/opentelemetry/test/custom/hub.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
17 changes: 6 additions & 11 deletions packages/replay/test/integration/eventProcessors.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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();
});

Expand All @@ -31,7 +26,7 @@ describe('Integration | eventProcessors', () => {
},
});

const client = hub.getClient()!;
const client = getClient()!;

jest.runAllTimers();
const mockTransportSend = jest.spyOn(client.getTransport()!, 'send');
Expand All @@ -47,7 +42,7 @@ describe('Integration | eventProcessors', () => {
return null;
});

scope.addEventProcessor(handler1);
getCurrentScope().addEventProcessor(handler1);

const TEST_EVENT = getTestEventIncremental({ timestamp: BASE_TIMESTAMP });

Expand All @@ -58,7 +53,7 @@ describe('Integration | eventProcessors', () => {

expect(mockTransportSend).toHaveBeenCalledTimes(1);

scope.addEventProcessor(handler2);
getCurrentScope().addEventProcessor(handler2);

const TEST_EVENT2 = getTestEventIncremental({ timestamp: BASE_TIMESTAMP });

Expand Down
4 changes: 4 additions & 0 deletions packages/types/src/hub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;

Expand Down