feat(core): Write data from setUser, setTags, setExtras, setTag, setExtra, and setContext to isolation scope

packages/core/src/hub.ts

@@ -387,6 +387,14 @@ export class Hub implements HubInterface {
       client.emit('beforeAddBreadcrumb', finalBreadcrumb, hint);
     }
 
+    // TODO(v8): I know this comment doesn't make much sense because the hub will be deprecated but I still wanted to
+    // write it down. In theory, we would have to add the breadcrumbs to the isolation scope here, however, that would
+    // duplicate all of the breadcrumbs. There was the possibility of adding breadcrumbs to both, the isolation scope
+    // and the normal scope, and deduplicating it down the line in the event processing pipeline. However, that would
+    // have been very fragile, because the breadcrumb objects would have needed to keep their identity all throughout
+    // the event processing pipeline.
+    // In the new implementation, the top level `Sentry.addBreadcrumb()` should ONLY write to the isolation scope.
+
     scope.addBreadcrumb(finalBreadcrumb, maxBreadcrumbs);
   }
 
@@ -395,44 +403,59 @@ 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);
   }
 
   /**
    * @inheritDoc
    * @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);
   }
 
   /**
    * @inheritDoc
    * @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);
   }
 
   /**
    * @inheritDoc
    * @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);
   }
 
   /**
    * @inheritDoc
    * @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);
   }
 
   /**
@@ -441,8 +464,11 @@ 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);
   }
 
   /**

packages/core/src/utils/applyScopeDataToEvent.ts

@@ -44,11 +44,11 @@ export function mergeScopeData(data: ScopeData, mergeData: ScopeData): void {
     span,
   } = mergeData;
 
-  mergePropOverwrite(data, 'extra', extra);
-  mergePropOverwrite(data, 'tags', tags);
-  mergePropOverwrite(data, 'user', user);
-  mergePropOverwrite(data, 'contexts', contexts);
-  mergePropOverwrite(data, 'sdkProcessingMetadata', sdkProcessingMetadata);
+  mergeAndOverwriteScopeData(data, 'extra', extra);
+  mergeAndOverwriteScopeData(data, 'tags', tags);
+  mergeAndOverwriteScopeData(data, 'user', user);
+  mergeAndOverwriteScopeData(data, 'contexts', contexts);
+  mergeAndOverwriteScopeData(data, 'sdkProcessingMetadata', sdkProcessingMetadata);
 
   if (level) {
     data.level = level;
@@ -83,28 +83,17 @@ export function mergeScopeData(data: ScopeData, mergeData: ScopeData): void {
 }
 
 /**
- * Merge properties, overwriting existing keys.
+ * Merges certain scope data. Undefined values will overwrite any existing values.
  * Exported only for tests.
  */
-export function mergePropOverwrite<
+export function mergeAndOverwriteScopeData<
   Prop extends 'extra' | 'tags' | 'user' | 'contexts' | 'sdkProcessingMetadata',
-  Data extends ScopeData | Event,
+  Data extends ScopeData,
 >(data: Data, prop: Prop, mergeVal: Data[Prop]): void {
-  if (mergeVal && Object.keys(mergeVal).length) {
-    data[prop] = { ...data[prop], ...mergeVal };
-  }
-}
-
-/**
- * Merge properties, keeping existing keys.
- * Exported only for tests.
- */
-export function mergePropKeep<
-  Prop extends 'extra' | 'tags' | 'user' | 'contexts' | 'sdkProcessingMetadata',
-  Data extends ScopeData | Event,
->(data: Data, prop: Prop, mergeVal: Data[Prop]): void {
-  if (mergeVal && Object.keys(mergeVal).length) {
-    data[prop] = { ...mergeVal, ...data[prop] };
+  for (const key in mergeVal) {
+    if (Object.prototype.hasOwnProperty.call(mergeVal, key)) {
+      data[prop][key] = mergeVal[key];
+    }
   }
 }
 

packages/core/test/lib/utils/applyScopeDataToEvent.test.ts

@@ -1,10 +1,5 @@
 import type { Attachment, Breadcrumb, EventProcessor, ScopeData } from '@sentry/types';
-import {
-  mergeArray,
-  mergePropKeep,
-  mergePropOverwrite,
-  mergeScopeData,
-} from '../../../src/utils/applyScopeDataToEvent';
+import { mergeAndOverwriteScopeData, mergeArray, mergeScopeData } from '../../../src/utils/applyScopeDataToEvent';
 
 describe('mergeArray', () => {
   it.each([
@@ -28,55 +23,19 @@ describe('mergeArray', () => {
   });
 });
 
-describe('mergePropKeep', () => {
-  it.each([
-    [{}, {}, {}],
-    [{ a: 'aa' }, {}, { a: 'aa' }],
-    [{ a: 'aa' }, { b: 'bb' }, { a: 'aa', b: 'bb' }],
-    // Does not overwrite existing keys
-    [{ a: 'aa' }, { b: 'bb', a: 'cc' }, { a: 'aa', b: 'bb' }],
-  ])('works with %s and %s', (a, b, expected) => {
-    const data = { tags: a } as unknown as ScopeData;
-    mergePropKeep(data, 'tags', b);
-    expect(data.tags).toEqual(expected);
-  });
-
-  it('does not deep merge', () => {
-    const data = {
-      contexts: {
-        app: { app_version: 'v1' },
-        culture: { display_name: 'name1' },
-      },
-    } as unknown as ScopeData;
-    mergePropKeep(data, 'contexts', {
-      os: { name: 'os1' },
-      app: { app_name: 'name1' },
-    });
-    expect(data.contexts).toEqual({
-      os: { name: 'os1' },
-      culture: { display_name: 'name1' },
-      app: { app_version: 'v1' },
-    });
-  });
-
-  it('does not mutate the original object if no changes are made', () => {
-    const tags = { a: 'aa' };
-    const data = { tags } as unknown as ScopeData;
-    mergePropKeep(data, 'tags', {});
-    expect(data.tags).toBe(tags);
-  });
-});
-
 describe('mergePropOverwrite', () => {
   it.each([
     [{}, {}, {}],
     [{ a: 'aa' }, {}, { a: 'aa' }],
     [{ a: 'aa' }, { b: 'bb' }, { a: 'aa', b: 'bb' }],
     // overwrites existing keys
     [{ a: 'aa' }, { b: 'bb', a: 'cc' }, { a: 'cc', b: 'bb' }],
-  ])('works with %s and %s', (a, b, expected) => {
-    const data = { tags: a } as unknown as ScopeData;
-    mergePropOverwrite(data, 'tags', b);
+    // undefined values overwrite existing values
+    [{ a: 'defined' }, { a: undefined, b: 'defined' }, { a: undefined, b: 'defined' }],
+    [{ a: 'defined' }, { a: null, b: 'defined' }, { a: null, b: 'defined' }],
+  ])('works with %s and %s', (oldData, newData, expected) => {
+    const data = { tags: oldData } as unknown as ScopeData;
+    mergeAndOverwriteScopeData(data, 'tags', newData);
     expect(data.tags).toEqual(expected);
   });
 
@@ -87,7 +46,7 @@ describe('mergePropOverwrite', () => {
         culture: { display_name: 'name1' },
       },
     } as unknown as ScopeData;
-    mergePropOverwrite(data, 'contexts', {
+    mergeAndOverwriteScopeData(data, 'contexts', {
       os: { name: 'os1' },
       app: { app_name: 'name1' },
     });
@@ -101,7 +60,7 @@ describe('mergePropOverwrite', () => {
   it('does not mutate the original object if no changes are made', () => {
     const tags = { a: 'aa' };
     const data = { tags } as unknown as ScopeData;
-    mergePropOverwrite(data, 'tags', {});
+    mergeAndOverwriteScopeData(data, 'tags', {});
     expect(data.tags).toBe(tags);
   });
 });