fix(core): Fix scope capturing via captureContext function (#10735) (#10737)

Backport to v7.

In #9801, we
introduced a regression that if you pass a function as `captureContext`
to capture methods, the returned scope is not used.

The cause for this was a confusion on my end, based on the slightly
weird way this works in `scope.update(fn)` - we don't actually merge
this or update the scope based on the return value of `fn`, but `fn`
receives the `scope` as argument, does nothing with the return type of
`fn` and just returns it - which we didn't use, because I assumed that
`scope.update` would actually return the scope (also, the return type of
it is `this` which is not correct there).

This PR changes this so that the returned scope of `fn` is actually
merged with the scope, same as if you'd pass a `scope` directly - so
this is fundamentally the same now:

```js
const otherScope = new Scope();
scope.update(otherScope);
scope.update(() => otherScope);
```

(which before would have had vastly different outcomes!)

I added a bunch of tests to verify how this works/should work.

Fixes #10686

Author: mydea

packages/core/src/scope.ts

@@ -378,50 +378,48 @@ export class Scope implements ScopeInterface {
       return this;
     }
 
-    if (typeof captureContext === 'function') {
-      const updatedScope = (captureContext as <T>(scope: T) => T)(this);
-      return updatedScope instanceof Scope ? updatedScope : this;
-    }
+    const scopeToMerge = typeof captureContext === 'function' ? captureContext(this) : captureContext;
+
+    if (scopeToMerge instanceof Scope) {
+      const scopeData = scopeToMerge.getScopeData();
 
-    if (captureContext instanceof Scope) {
-      this._tags = { ...this._tags, ...captureContext._tags };
-      this._extra = { ...this._extra, ...captureContext._extra };
-      this._contexts = { ...this._contexts, ...captureContext._contexts };
-      if (captureContext._user && Object.keys(captureContext._user).length) {
-        this._user = captureContext._user;
+      this._tags = { ...this._tags, ...scopeData.tags };
+      this._extra = { ...this._extra, ...scopeData.extra };
+      this._contexts = { ...this._contexts, ...scopeData.contexts };
+      if (scopeData.user && Object.keys(scopeData.user).length) {
+        this._user = scopeData.user;
       }
-      if (captureContext._level) {
-        this._level = captureContext._level;
+      if (scopeData.level) {
+        this._level = scopeData.level;
       }
-      if (captureContext._fingerprint) {
-        this._fingerprint = captureContext._fingerprint;
+      if (scopeData.fingerprint.length) {
+        this._fingerprint = scopeData.fingerprint;
       }
-      if (captureContext._requestSession) {
-        this._requestSession = captureContext._requestSession;
+      if (scopeToMerge.getRequestSession()) {
+        this._requestSession = scopeToMerge.getRequestSession();
       }
-      if (captureContext._propagationContext) {
-        this._propagationContext = captureContext._propagationContext;
+      if (scopeData.propagationContext) {
+        this._propagationContext = scopeData.propagationContext;
       }
-    } else if (isPlainObject(captureContext)) {
-      // eslint-disable-next-line no-param-reassign
-      captureContext = captureContext as ScopeContext;
-      this._tags = { ...this._tags, ...captureContext.tags };
-      this._extra = { ...this._extra, ...captureContext.extra };
-      this._contexts = { ...this._contexts, ...captureContext.contexts };
-      if (captureContext.user) {
-        this._user = captureContext.user;
+    } else if (isPlainObject(scopeToMerge)) {
+      const scopeContext = captureContext as ScopeContext;
+      this._tags = { ...this._tags, ...scopeContext.tags };
+      this._extra = { ...this._extra, ...scopeContext.extra };
+      this._contexts = { ...this._contexts, ...scopeContext.contexts };
+      if (scopeContext.user) {
+        this._user = scopeContext.user;
       }
-      if (captureContext.level) {
-        this._level = captureContext.level;
+      if (scopeContext.level) {
+        this._level = scopeContext.level;
       }
-      if (captureContext.fingerprint) {
-        this._fingerprint = captureContext.fingerprint;
+      if (scopeContext.fingerprint) {
+        this._fingerprint = scopeContext.fingerprint;
       }
-      if (captureContext.requestSession) {
-        this._requestSession = captureContext.requestSession;
+      if (scopeContext.requestSession) {
+        this._requestSession = scopeContext.requestSession;
       }
-      if (captureContext.propagationContext) {
-        this._propagationContext = captureContext.propagationContext;
+      if (scopeContext.propagationContext) {
+        this._propagationContext = scopeContext.propagationContext;
       }
     }
 

packages/core/test/lib/prepareEvent.test.ts

@@ -379,4 +379,130 @@ describe('prepareEvent', () => {
       },
     });
   });
+
+  describe('captureContext', () => {
+    it('works with scope & captureContext=POJO', async () => {
+      const scope = new Scope();
+      scope.setTags({
+        initial: 'aa',
+        foo: 'foo',
+      });
+
+      const event = { message: 'foo' };
+
+      const options = {} as ClientOptions;
+      const client = {
+        getEventProcessors() {
+          return [] as EventProcessor[];
+        },
+      } as Client;
+
+      const processedEvent = await prepareEvent(
+        options,
+        event,
+        {
+          captureContext: { tags: { foo: 'bar' } },
+          integrations: [],
+        },
+        scope,
+        client,
+      );
+
+      expect(processedEvent).toEqual({
+        timestamp: expect.any(Number),
+        event_id: expect.any(String),
+        environment: 'production',
+        message: 'foo',
+        sdkProcessingMetadata: {},
+        tags: { initial: 'aa', foo: 'bar' },
+      });
+    });
+
+    it('works with scope & captureContext=scope instance', async () => {
+      const scope = new Scope();
+      scope.setTags({
+        initial: 'aa',
+        foo: 'foo',
+      });
+
+      const event = { message: 'foo' };
+
+      const options = {} as ClientOptions;
+      const client = {
+        getEventProcessors() {
+          return [] as EventProcessor[];
+        },
+      } as Client;
+
+      const captureContext = new Scope();
+      captureContext.setTags({ foo: 'bar' });
+
+      const processedEvent = await prepareEvent(
+        options,
+        event,
+        {
+          captureContext,
+          integrations: [],
+        },
+        scope,
+        client,
+      );
+
+      expect(processedEvent).toEqual({
+        timestamp: expect.any(Number),
+        event_id: expect.any(String),
+        environment: 'production',
+        message: 'foo',
+        sdkProcessingMetadata: {},
+        tags: { initial: 'aa', foo: 'bar' },
+      });
+    });
+
+    it('works with scope & captureContext=function', async () => {
+      const scope = new Scope();
+      scope.setTags({
+        initial: 'aa',
+        foo: 'foo',
+      });
+
+      const event = { message: 'foo' };
+
+      const options = {} as ClientOptions;
+      const client = {
+        getEventProcessors() {
+          return [] as EventProcessor[];
+        },
+      } as Client;
+
+      const captureContextScope = new Scope();
+      captureContextScope.setTags({ foo: 'bar' });
+
+      const captureContext = jest.fn(passedScope => {
+        expect(passedScope).toEqual(scope);
+        return captureContextScope;
+      });
+
+      const processedEvent = await prepareEvent(
+        options,
+        event,
+        {
+          captureContext,
+          integrations: [],
+        },
+        scope,
+        client,
+      );
+
+      expect(captureContext).toHaveBeenCalledTimes(1);
+
+      expect(processedEvent).toEqual({
+        timestamp: expect.any(Number),
+        event_id: expect.any(String),
+        environment: 'production',
+        message: 'foo',
+        sdkProcessingMetadata: {},
+        tags: { initial: 'aa', foo: 'bar' },
+      });
+    });
+  });
 });