From 0e2f8d68b4ebecc8060824b59b4e2b1b6f903266 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 8 Feb 2024 13:01:53 +0000 Subject: [PATCH 1/5] ref: Make scope setters on hub only write to isolation scope --- packages/core/src/hub.ts | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/packages/core/src/hub.ts b/packages/core/src/hub.ts index 79359657cc8b..f5b5b617d1fe 100644 --- a/packages/core/src/hub.ts +++ b/packages/core/src/hub.ts @@ -413,9 +413,6 @@ export class Hub implements HubInterface { * @deprecated Use `Sentry.setUser()` instead. */ public setUser(user: User | null): void { - // TODO(v8): The top level `Sentry.setUser()` function should write ONLY to the isolation scope. - // eslint-disable-next-line deprecation/deprecation - this.getScope().setUser(user); // eslint-disable-next-line deprecation/deprecation this.getIsolationScope().setUser(user); } @@ -425,9 +422,6 @@ export class Hub implements HubInterface { * @deprecated Use `Sentry.setTags()` instead. */ public setTags(tags: { [key: string]: Primitive }): void { - // TODO(v8): The top level `Sentry.setTags()` function should write ONLY to the isolation scope. - // eslint-disable-next-line deprecation/deprecation - this.getScope().setTags(tags); // eslint-disable-next-line deprecation/deprecation this.getIsolationScope().setTags(tags); } @@ -437,9 +431,6 @@ export class Hub implements HubInterface { * @deprecated Use `Sentry.setExtras()` instead. */ public setExtras(extras: Extras): void { - // TODO(v8): The top level `Sentry.setExtras()` function should write ONLY to the isolation scope. - // eslint-disable-next-line deprecation/deprecation - this.getScope().setExtras(extras); // eslint-disable-next-line deprecation/deprecation this.getIsolationScope().setExtras(extras); } @@ -449,9 +440,6 @@ export class Hub implements HubInterface { * @deprecated Use `Sentry.setTag()` instead. */ public setTag(key: string, value: Primitive): void { - // TODO(v8): The top level `Sentry.setTag()` function should write ONLY to the isolation scope. - // eslint-disable-next-line deprecation/deprecation - this.getScope().setTag(key, value); // eslint-disable-next-line deprecation/deprecation this.getIsolationScope().setTag(key, value); } @@ -461,9 +449,6 @@ export class Hub implements HubInterface { * @deprecated Use `Sentry.setExtra()` instead. */ public setExtra(key: string, extra: Extra): void { - // TODO(v8): The top level `Sentry.setExtra()` function should write ONLY to the isolation scope. - // eslint-disable-next-line deprecation/deprecation - this.getScope().setExtra(key, extra); // eslint-disable-next-line deprecation/deprecation this.getIsolationScope().setExtra(key, extra); } @@ -474,9 +459,6 @@ export class Hub implements HubInterface { */ // eslint-disable-next-line @typescript-eslint/no-explicit-any public setContext(name: string, context: { [key: string]: any } | null): void { - // TODO(v8): The top level `Sentry.setContext()` function should write ONLY to the isolation scope. - // eslint-disable-next-line deprecation/deprecation - this.getScope().setContext(name, context); // eslint-disable-next-line deprecation/deprecation this.getIsolationScope().setContext(name, context); } From 38ee02d55d615196587fa747283a7cc963a75d65 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 8 Feb 2024 15:36:35 +0000 Subject: [PATCH 2/5] . --- packages/astro/test/client/sdk.test.ts | 9 +++++---- packages/astro/test/server/sdk.test.ts | 8 ++++---- packages/node/test/integrations/http.test.ts | 3 ++- packages/sveltekit/test/client/sdk.test.ts | 10 +++++----- packages/sveltekit/test/server/sdk.test.ts | 8 ++++---- 5 files changed, 20 insertions(+), 18 deletions(-) diff --git a/packages/astro/test/client/sdk.test.ts b/packages/astro/test/client/sdk.test.ts index d6f22dc9ed7a..e916a75471bb 100644 --- a/packages/astro/test/client/sdk.test.ts +++ b/packages/astro/test/client/sdk.test.ts @@ -5,6 +5,7 @@ import * as SentryBrowser from '@sentry/browser'; import { BrowserTracing, SDK_VERSION, WINDOW, getClient } from '@sentry/browser'; import { vi } from 'vitest'; +import { getIsolationScope } from '@sentry/core'; import { init } from '../../../astro/src/client/sdk'; const browserInit = vi.spyOn(SentryBrowser, 'init'); @@ -38,16 +39,16 @@ describe('Sentry client SDK', () => { ); }); - it('sets the runtime tag on the scope', () => { - const currentScope = getCurrentScope(); + it('sets the runtime tag on the isolation scope', () => { + const isolationScope = getIsolationScope(); // @ts-expect-error need access to protected _tags attribute - expect(currentScope._tags).toEqual({}); + expect(isolationScope._tags).toEqual({}); init({ dsn: 'https://public@dsn.ingest.sentry.io/1337' }); // @ts-expect-error need access to protected _tags attribute - expect(currentScope._tags).toEqual({ runtime: 'browser' }); + expect(isolationScope._tags).toEqual({ runtime: 'browser' }); }); describe('automatically adds integrations', () => { diff --git a/packages/astro/test/server/sdk.test.ts b/packages/astro/test/server/sdk.test.ts index b132c32a03c9..b4f0536a2a76 100644 --- a/packages/astro/test/server/sdk.test.ts +++ b/packages/astro/test/server/sdk.test.ts @@ -36,16 +36,16 @@ describe('Sentry server SDK', () => { ); }); - it('sets the runtime tag on the scope', () => { - const currentScope = SentryNode.getCurrentScope(); + it('sets the runtime tag on the isolation scope', () => { + const isolationScope = SentryNode.getIsolationScope(); // @ts-expect-error need access to protected _tags attribute - expect(currentScope._tags).toEqual({}); + expect(isolationScope._tags).toEqual({}); init({ dsn: 'https://public@dsn.ingest.sentry.io/1337' }); // @ts-expect-error need access to protected _tags attribute - expect(currentScope._tags).toEqual({ runtime: 'node' }); + expect(isolationScope._tags).toEqual({ runtime: 'node' }); }); }); }); diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts index 527ef48c3f0f..62c355219811 100644 --- a/packages/node/test/integrations/http.test.ts +++ b/packages/node/test/integrations/http.test.ts @@ -1,6 +1,6 @@ import * as http from 'http'; import * as https from 'https'; -import type { Span } from '@sentry/core'; +import { Span, withIsolationScope } from '@sentry/core'; import { Transaction } from '@sentry/core'; import { getCurrentScope, makeMain, setUser, spanToJSON, startInactiveSpan } from '@sentry/core'; import { Hub, addTracingExtensions } from '@sentry/core'; @@ -10,6 +10,7 @@ import * as nock from 'nock'; import { HttpsProxyAgent } from '../../src/proxy'; import type { Breadcrumb } from '../../src'; +import { setNodeAsyncContextStrategy } from '../../src/async'; import { NodeClient } from '../../src/client'; import { Http as HttpIntegration, diff --git a/packages/sveltekit/test/client/sdk.test.ts b/packages/sveltekit/test/client/sdk.test.ts index bc863af99897..fd549370c289 100644 --- a/packages/sveltekit/test/client/sdk.test.ts +++ b/packages/sveltekit/test/client/sdk.test.ts @@ -1,4 +1,4 @@ -import { getClient, getCurrentScope } from '@sentry/core'; +import { getClient, getCurrentScope, getIsolationScope } from '@sentry/core'; import type { BrowserClient } from '@sentry/svelte'; import * as SentrySvelte from '@sentry/svelte'; import { SDK_VERSION, WINDOW, browserTracingIntegration } from '@sentry/svelte'; @@ -38,16 +38,16 @@ describe('Sentry client SDK', () => { ); }); - it('sets the runtime tag on the scope', () => { - const currentScope = getCurrentScope(); + it('sets the runtime tag on the isolation scope', () => { + const isolationScope = getIsolationScope(); // @ts-expect-error need access to protected _tags attribute - expect(currentScope._tags).toEqual({}); + expect(isolationScope._tags).toEqual({}); init({ dsn: 'https://public@dsn.ingest.sentry.io/1337' }); // @ts-expect-error need access to protected _tags attribute - expect(currentScope._tags).toEqual({ runtime: 'browser' }); + expect(isolationScope._tags).toEqual({ runtime: 'browser' }); }); describe('automatically added integrations', () => { diff --git a/packages/sveltekit/test/server/sdk.test.ts b/packages/sveltekit/test/server/sdk.test.ts index 99dcb37516e1..53f3ee8e33e8 100644 --- a/packages/sveltekit/test/server/sdk.test.ts +++ b/packages/sveltekit/test/server/sdk.test.ts @@ -36,16 +36,16 @@ describe('Sentry server SDK', () => { ); }); - it('sets the runtime tag on the scope', () => { - const currentScope = SentryNode.getCurrentScope(); + it('sets the runtime tag on the isolation scope', () => { + const isolationScope = SentryNode.getIsolationScope(); // @ts-expect-error need access to protected _tags attribute - expect(currentScope._tags).toEqual({}); + expect(isolationScope._tags).toEqual({}); init({ dsn: 'https://public@dsn.ingest.sentry.io/1337' }); // @ts-expect-error need access to protected _tags attribute - expect(currentScope._tags).toEqual({ runtime: 'node' }); + expect(isolationScope._tags).toEqual({ runtime: 'node' }); }); it('adds rewriteFramesIntegration by default', () => { From b9b329bb7c342354760bb49312511555cb6af844 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 9 Feb 2024 09:04:00 +0000 Subject: [PATCH 3/5] lint --- packages/astro/test/client/sdk.test.ts | 2 +- packages/node/test/integrations/http.test.ts | 3 +-- packages/sveltekit/test/client/sdk.test.ts | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/astro/test/client/sdk.test.ts b/packages/astro/test/client/sdk.test.ts index e916a75471bb..9136f5650135 100644 --- a/packages/astro/test/client/sdk.test.ts +++ b/packages/astro/test/client/sdk.test.ts @@ -1,6 +1,6 @@ import type { BrowserClient } from '@sentry/browser'; import { getActiveSpan } from '@sentry/browser'; -import { browserTracingIntegration, getCurrentScope } from '@sentry/browser'; +import { browserTracingIntegration } from '@sentry/browser'; import * as SentryBrowser from '@sentry/browser'; import { BrowserTracing, SDK_VERSION, WINDOW, getClient } from '@sentry/browser'; import { vi } from 'vitest'; diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts index 771bac73eeba..dc2fbc69a37f 100644 --- a/packages/node/test/integrations/http.test.ts +++ b/packages/node/test/integrations/http.test.ts @@ -1,6 +1,6 @@ import * as http from 'http'; import * as https from 'https'; -import { Span, withIsolationScope } from '@sentry/core'; +import type { Span } from '@sentry/core'; import { Transaction } from '@sentry/core'; import { getCurrentScope, makeMain, setUser, spanToJSON, startInactiveSpan } from '@sentry/core'; import { Hub, addTracingExtensions } from '@sentry/core'; @@ -10,7 +10,6 @@ import * as nock from 'nock'; import { HttpsProxyAgent } from '../../src/proxy'; import type { Breadcrumb } from '../../src'; -import { setNodeAsyncContextStrategy } from '../../src/async'; import { NodeClient } from '../../src/client'; import { Http as HttpIntegration, diff --git a/packages/sveltekit/test/client/sdk.test.ts b/packages/sveltekit/test/client/sdk.test.ts index fd549370c289..0cf28d5f0514 100644 --- a/packages/sveltekit/test/client/sdk.test.ts +++ b/packages/sveltekit/test/client/sdk.test.ts @@ -1,4 +1,4 @@ -import { getClient, getCurrentScope, getIsolationScope } from '@sentry/core'; +import { getClient, getIsolationScope } from '@sentry/core'; import type { BrowserClient } from '@sentry/svelte'; import * as SentrySvelte from '@sentry/svelte'; import { SDK_VERSION, WINDOW, browserTracingIntegration } from '@sentry/svelte'; From 5dec8a5885495cd76eeaeaad5d428c2b6f971568 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 9 Feb 2024 09:26:52 +0000 Subject: [PATCH 4/5] Remove unnecessary test --- packages/nextjs/test/serverSdk.test.ts | 41 +------------------------- 1 file changed, 1 insertion(+), 40 deletions(-) diff --git a/packages/nextjs/test/serverSdk.test.ts b/packages/nextjs/test/serverSdk.test.ts index 1c7c9e384657..868164ddda8a 100644 --- a/packages/nextjs/test/serverSdk.test.ts +++ b/packages/nextjs/test/serverSdk.test.ts @@ -1,6 +1,5 @@ -import { runWithAsyncContext } from '@sentry/core'; import * as SentryNode from '@sentry/node'; -import { NodeClient, getClient, getCurrentHub, getCurrentScope } from '@sentry/node'; +import { getClient, getCurrentScope } from '@sentry/node'; import type { Integration } from '@sentry/types'; import { GLOBAL_OBJ, logger } from '@sentry/utils'; @@ -113,44 +112,6 @@ describe('Server init()', () => { expect(loggerLogSpy).toHaveBeenCalledWith('An event processor returned `null`, will not send event.'); }); - it("initializes both global hub and domain hub when there's an active domain", () => { - // eslint-disable-next-line deprecation/deprecation - const globalHub = getCurrentHub(); - - runWithAsyncContext(() => { - // eslint-disable-next-line deprecation/deprecation - const globalHub2 = getCurrentHub(); - // If we call runWithAsyncContext before init, it executes the callback in the same context as there is no - // strategy yet - expect(globalHub2).toBe(globalHub); - // eslint-disable-next-line deprecation/deprecation - expect(globalHub.getClient()).toBeUndefined(); - // eslint-disable-next-line deprecation/deprecation - expect(globalHub2.getClient()).toBeUndefined(); - - init({}); - - runWithAsyncContext(() => { - // eslint-disable-next-line deprecation/deprecation - const domainHub = getCurrentHub(); - // this tag should end up only in the domain hub - // eslint-disable-next-line deprecation/deprecation - domainHub.setTag('dogs', 'areGreat'); - - // eslint-disable-next-line deprecation/deprecation - expect(globalHub.getClient()).toEqual(expect.any(NodeClient)); - // eslint-disable-next-line deprecation/deprecation - expect(domainHub.getClient()).toBe(globalHub.getClient()); - // @ts-expect-error need access to protected _tags attribute - // eslint-disable-next-line deprecation/deprecation - expect(globalHub.getScope()._tags).toEqual({ runtime: 'node' }); - // @ts-expect-error need access to protected _tags attribute - // eslint-disable-next-line deprecation/deprecation - expect(domainHub.getScope()._tags).toEqual({ runtime: 'node', dogs: 'areGreat' }); - }); - }); - }); - describe('integrations', () => { // Options passed by `@sentry/nextjs`'s `init` to `@sentry/node`'s `init` after modifying them type ModifiedInitOptions = { integrations: Integration[]; defaultIntegrations: Integration[] }; From 3d146f4e4c8aaf576eae07e7b84766943d03650f Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 9 Feb 2024 10:45:12 +0000 Subject: [PATCH 5/5] Fix astro and sveltekit tests --- packages/sveltekit/src/client/sdk.ts | 6 +++--- packages/sveltekit/src/server/sdk.ts | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/sveltekit/src/client/sdk.ts b/packages/sveltekit/src/client/sdk.ts index b0dc7ee6af2d..a3c996457c46 100644 --- a/packages/sveltekit/src/client/sdk.ts +++ b/packages/sveltekit/src/client/sdk.ts @@ -1,7 +1,7 @@ -import { applySdkMetadata, hasTracingEnabled } from '@sentry/core'; +import { applySdkMetadata, hasTracingEnabled, setTag } from '@sentry/core'; import type { BrowserOptions, browserTracingIntegration } from '@sentry/svelte'; import { getDefaultIntegrations as getDefaultSvelteIntegrations } from '@sentry/svelte'; -import { WINDOW, getCurrentScope, init as initSvelteSdk } from '@sentry/svelte'; +import { WINDOW, init as initSvelteSdk } from '@sentry/svelte'; import type { Integration } from '@sentry/types'; import { @@ -42,7 +42,7 @@ export function init(options: BrowserOptions): void { restoreFetch(actualFetch); } - getCurrentScope().setTag('runtime', 'browser'); + setTag('runtime', 'browser'); } // TODO v8: Remove this again diff --git a/packages/sveltekit/src/server/sdk.ts b/packages/sveltekit/src/server/sdk.ts index c6b3bd26dffe..f16220775d3a 100644 --- a/packages/sveltekit/src/server/sdk.ts +++ b/packages/sveltekit/src/server/sdk.ts @@ -1,4 +1,4 @@ -import { applySdkMetadata, getCurrentScope } from '@sentry/core'; +import { applySdkMetadata, setTag } from '@sentry/core'; import type { NodeOptions } from '@sentry/node'; import { getDefaultIntegrations as getDefaultNodeIntegrations } from '@sentry/node'; import { init as initNodeSdk } from '@sentry/node'; @@ -19,5 +19,5 @@ export function init(options: NodeOptions): void { initNodeSdk(opts); - getCurrentScope().setTag('runtime', 'node'); + setTag('runtime', 'node'); }