Skip to content

Commit 97cc190

Browse files
authored
fix(core): Fix scope capturing via captureContext function (#10735)
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
1 parent 1f75cde commit 97cc190

File tree

2 files changed

+158
-34
lines changed

2 files changed

+158
-34
lines changed

packages/core/src/scope.ts

Lines changed: 32 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -366,50 +366,48 @@ export class Scope implements ScopeInterface {
366366
return this;
367367
}
368368

369-
if (typeof captureContext === 'function') {
370-
const updatedScope = (captureContext as <T>(scope: T) => T)(this);
371-
return updatedScope instanceof Scope ? updatedScope : this;
372-
}
369+
const scopeToMerge = typeof captureContext === 'function' ? captureContext(this) : captureContext;
370+
371+
if (scopeToMerge instanceof Scope) {
372+
const scopeData = scopeToMerge.getScopeData();
373373

374-
if (captureContext instanceof Scope) {
375-
this._tags = { ...this._tags, ...captureContext._tags };
376-
this._extra = { ...this._extra, ...captureContext._extra };
377-
this._contexts = { ...this._contexts, ...captureContext._contexts };
378-
if (captureContext._user && Object.keys(captureContext._user).length) {
379-
this._user = captureContext._user;
374+
this._tags = { ...this._tags, ...scopeData.tags };
375+
this._extra = { ...this._extra, ...scopeData.extra };
376+
this._contexts = { ...this._contexts, ...scopeData.contexts };
377+
if (scopeData.user && Object.keys(scopeData.user).length) {
378+
this._user = scopeData.user;
380379
}
381-
if (captureContext._level) {
382-
this._level = captureContext._level;
380+
if (scopeData.level) {
381+
this._level = scopeData.level;
383382
}
384-
if (captureContext._fingerprint) {
385-
this._fingerprint = captureContext._fingerprint;
383+
if (scopeData.fingerprint) {
384+
this._fingerprint = scopeData.fingerprint;
386385
}
387-
if (captureContext._requestSession) {
388-
this._requestSession = captureContext._requestSession;
386+
if (scopeToMerge.getRequestSession()) {
387+
this._requestSession = scopeToMerge.getRequestSession();
389388
}
390-
if (captureContext._propagationContext) {
391-
this._propagationContext = captureContext._propagationContext;
389+
if (scopeData.propagationContext) {
390+
this._propagationContext = scopeData.propagationContext;
392391
}
393-
} else if (isPlainObject(captureContext)) {
394-
// eslint-disable-next-line no-param-reassign
395-
captureContext = captureContext as ScopeContext;
396-
this._tags = { ...this._tags, ...captureContext.tags };
397-
this._extra = { ...this._extra, ...captureContext.extra };
398-
this._contexts = { ...this._contexts, ...captureContext.contexts };
399-
if (captureContext.user) {
400-
this._user = captureContext.user;
392+
} else if (isPlainObject(scopeToMerge)) {
393+
const scopeContext = captureContext as ScopeContext;
394+
this._tags = { ...this._tags, ...scopeContext.tags };
395+
this._extra = { ...this._extra, ...scopeContext.extra };
396+
this._contexts = { ...this._contexts, ...scopeContext.contexts };
397+
if (scopeContext.user) {
398+
this._user = scopeContext.user;
401399
}
402-
if (captureContext.level) {
403-
this._level = captureContext.level;
400+
if (scopeContext.level) {
401+
this._level = scopeContext.level;
404402
}
405-
if (captureContext.fingerprint) {
406-
this._fingerprint = captureContext.fingerprint;
403+
if (scopeContext.fingerprint) {
404+
this._fingerprint = scopeContext.fingerprint;
407405
}
408-
if (captureContext.requestSession) {
409-
this._requestSession = captureContext.requestSession;
406+
if (scopeContext.requestSession) {
407+
this._requestSession = scopeContext.requestSession;
410408
}
411-
if (captureContext.propagationContext) {
412-
this._propagationContext = captureContext.propagationContext;
409+
if (scopeContext.propagationContext) {
410+
this._propagationContext = scopeContext.propagationContext;
413411
}
414412
}
415413

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

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,4 +379,130 @@ describe('prepareEvent', () => {
379379
},
380380
});
381381
});
382+
383+
describe('captureContext', () => {
384+
it('works with scope & captureContext=POJO', async () => {
385+
const scope = new Scope();
386+
scope.setTags({
387+
initial: 'aa',
388+
foo: 'foo',
389+
});
390+
391+
const event = { message: 'foo' };
392+
393+
const options = {} as ClientOptions;
394+
const client = {
395+
getEventProcessors() {
396+
return [] as EventProcessor[];
397+
},
398+
} as Client;
399+
400+
const processedEvent = await prepareEvent(
401+
options,
402+
event,
403+
{
404+
captureContext: { tags: { foo: 'bar' } },
405+
integrations: [],
406+
},
407+
scope,
408+
client,
409+
);
410+
411+
expect(processedEvent).toEqual({
412+
timestamp: expect.any(Number),
413+
event_id: expect.any(String),
414+
environment: 'production',
415+
message: 'foo',
416+
sdkProcessingMetadata: {},
417+
tags: { initial: 'aa', foo: 'bar' },
418+
});
419+
});
420+
421+
it('works with scope & captureContext=scope instance', async () => {
422+
const scope = new Scope();
423+
scope.setTags({
424+
initial: 'aa',
425+
foo: 'foo',
426+
});
427+
428+
const event = { message: 'foo' };
429+
430+
const options = {} as ClientOptions;
431+
const client = {
432+
getEventProcessors() {
433+
return [] as EventProcessor[];
434+
},
435+
} as Client;
436+
437+
const captureContext = new Scope();
438+
captureContext.setTags({ foo: 'bar' });
439+
440+
const processedEvent = await prepareEvent(
441+
options,
442+
event,
443+
{
444+
captureContext,
445+
integrations: [],
446+
},
447+
scope,
448+
client,
449+
);
450+
451+
expect(processedEvent).toEqual({
452+
timestamp: expect.any(Number),
453+
event_id: expect.any(String),
454+
environment: 'production',
455+
message: 'foo',
456+
sdkProcessingMetadata: {},
457+
tags: { initial: 'aa', foo: 'bar' },
458+
});
459+
});
460+
461+
it('works with scope & captureContext=function', async () => {
462+
const scope = new Scope();
463+
scope.setTags({
464+
initial: 'aa',
465+
foo: 'foo',
466+
});
467+
468+
const event = { message: 'foo' };
469+
470+
const options = {} as ClientOptions;
471+
const client = {
472+
getEventProcessors() {
473+
return [] as EventProcessor[];
474+
},
475+
} as Client;
476+
477+
const captureContextScope = new Scope();
478+
captureContextScope.setTags({ foo: 'bar' });
479+
480+
const captureContext = jest.fn(passedScope => {
481+
expect(passedScope).toEqual(scope);
482+
return captureContextScope;
483+
});
484+
485+
const processedEvent = await prepareEvent(
486+
options,
487+
event,
488+
{
489+
captureContext,
490+
integrations: [],
491+
},
492+
scope,
493+
client,
494+
);
495+
496+
expect(captureContext).toHaveBeenCalledTimes(1);
497+
498+
expect(processedEvent).toEqual({
499+
timestamp: expect.any(Number),
500+
event_id: expect.any(String),
501+
environment: 'production',
502+
message: 'foo',
503+
sdkProcessingMetadata: {},
504+
tags: { initial: 'aa', foo: 'bar' },
505+
});
506+
});
507+
});
382508
});

0 commit comments

Comments
 (0)