Skip to content

Commit

Permalink
fix(core): Fix scope capturing via captureContext function (#10735) (
Browse files Browse the repository at this point in the history
…#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
  • Loading branch information
mydea committed Feb 20, 2024
1 parent 0cc946d commit b327157
Show file tree
Hide file tree
Showing 3 changed files with 551 additions and 35 deletions.
66 changes: 32 additions & 34 deletions packages/core/src/scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
126 changes: 126 additions & 0 deletions packages/core/test/lib/prepareEvent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' },
});
});
});
});

0 comments on commit b327157

Please sign in to comment.