Skip to content

Commit e287bb9

Browse files
kamilogorekkamilogorek
committed
Integrations setup rework and tests
1 parent 1b564cd commit e287bb9

File tree

8 files changed

+254
-74
lines changed

8 files changed

+254
-74
lines changed

‎packages/core/src/baseclient.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,6 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
145145
if (options.dsn) {
146146
this.dsn = new Dsn(options.dsn);
147147
}
148-
149-
this.integrations = setupIntegrations(options);
150148
}
151149

152150
/**
@@ -157,6 +155,8 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
157155
return (this.installed = false);
158156
}
159157

158+
this.integrations = setupIntegrations(this.options);
159+
160160
const backend = this.getBackend();
161161
if (!this.installed && backend.install) {
162162
backend.install();

‎packages/core/src/integrations/index.ts‎

Lines changed: 67 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,74 @@ export { InboundFilters } from './inboundfilters';
1010
export { Debug } from './pluggable/debug';
1111
export { RewriteFrames } from './pluggable/rewriteframes';
1212

13-
const installedIntegrations: string[] = [];
13+
export const installedIntegrations: string[] = [];
1414

15+
/** Map of integrations assigned to a client */
1516
export interface IntegrationIndex {
1617
[key: string]: Integration;
1718
}
1819

20+
/** Gets integration to install */
21+
export function getIntegrationsToSetup(options: Options): Integration[] {
22+
const defaultIntegrations = (options.defaultIntegrations && [...options.defaultIntegrations]) || [];
23+
const userIntegrations = options.integrations;
24+
let integrations: Integration[] = [];
25+
26+
if (Array.isArray(userIntegrations)) {
27+
const userIntegrationsNames = userIntegrations.map(i => i.name);
28+
const pickedIntegrationsNames = [];
29+
30+
// Leave only unique default integrations, that were not overridden with provided user integrations
31+
for (const defaultIntegration of defaultIntegrations) {
32+
if (
33+
userIntegrationsNames.indexOf(defaultIntegration.name) === -1 &&
34+
pickedIntegrationsNames.indexOf(defaultIntegration.name) === -1
35+
) {
36+
integrations.push(defaultIntegration);
37+
pickedIntegrationsNames.push(defaultIntegration.name);
38+
}
39+
}
40+
41+
// Don't add same user integration twice
42+
for (const userIntegration of userIntegrations) {
43+
if (pickedIntegrationsNames.indexOf(userIntegration.name) === -1) {
44+
integrations.push(userIntegration);
45+
pickedIntegrationsNames.push(userIntegration.name);
46+
}
47+
}
48+
} else if (typeof userIntegrations === 'function') {
49+
integrations = userIntegrations(defaultIntegrations);
50+
integrations = Array.isArray(integrations) ? integrations : [integrations];
51+
} else {
52+
return [...defaultIntegrations];
53+
}
54+
55+
return integrations;
56+
}
57+
58+
/** Setup given integration */
59+
export function setupIntegration(integration: Integration, options: Options): void {
60+
if (installedIntegrations.indexOf(integration.name) !== -1) {
61+
return;
62+
}
63+
64+
try {
65+
if (integration.setupOnce) {
66+
integration.setupOnce(options);
67+
}
68+
} catch (_Oo) {
69+
// TODO: Remove in v5
70+
logger.warn(`Integration ${integration.name}: The install method is deprecated. Use "setupOnce".`);
71+
72+
if (integration.install) {
73+
integration.install(options);
74+
}
75+
}
76+
77+
installedIntegrations.push(integration.name);
78+
logger.log(`Integration installed: ${integration.name}`);
79+
}
80+
1981
/**
2082
* Given a list of integration instances this installs them all. When `withDefaults` is set to `true` then all default
2183
* integrations are added unless they were already provided before.
@@ -24,40 +86,9 @@ export interface IntegrationIndex {
2486
*/
2587
export function setupIntegrations<O extends Options>(options: O): IntegrationIndex {
2688
const integrations: IntegrationIndex = {};
27-
let integrationsToInstall = (options.defaultIntegrations && [...options.defaultIntegrations]) || [];
28-
if (Array.isArray(options.integrations)) {
29-
const providedIntegrationsNames = options.integrations.map(i => i.name);
30-
integrationsToInstall = [
31-
// Leave only unique integrations, that were not overridden with provided integrations with the same name
32-
...integrationsToInstall.filter(integration => providedIntegrationsNames.indexOf(integration.name) === -1),
33-
...options.integrations,
34-
];
35-
} else if (typeof options.integrations === 'function') {
36-
integrationsToInstall = options.integrations(integrationsToInstall);
37-
}
38-
39-
// Just in case someone will return non-array from a `itegrations` callback
40-
if (Array.isArray(integrationsToInstall)) {
41-
integrationsToInstall.forEach(integration => {
42-
integrations[name] = integration;
43-
if (installedIntegrations.indexOf(integration.name) !== -1) {
44-
return;
45-
}
46-
try {
47-
if (integration.setupOnce) {
48-
// TODO remove
49-
integration.setupOnce(options);
50-
}
51-
} catch (_Oo) {
52-
logger.warn(`Integration ${integration.name}: The install method is deprecated. Use "setupOnce".`);
53-
if (integration.install) {
54-
// TODO remove if
55-
integration.install(options);
56-
}
57-
}
58-
installedIntegrations.push(integration.name);
59-
logger.log(`Integration installed: ${integration.name}`);
60-
});
61-
}
89+
getIntegrationsToSetup(options).forEach(integration => {
90+
integrations[integration.name] = integration;
91+
setupIntegration(integration, options);
92+
});
6293
return integrations;
6394
}

‎packages/core/src/sdk.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ export function initAndBind<F extends Client, O extends Options>(clientClass: Cl
2525
}
2626

2727
const client = new clientClass(options);
28-
client.install();
29-
3028
getCurrentHub().bindClient(client);
29+
client.install();
3130
}

‎packages/core/test/lib/base.test.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { Breadcrumb, Status } from '@sentry/types';
33
import { SentryError } from '../../src/error';
44
import { TestBackend } from '../mocks/backend';
55
import { TestClient } from '../mocks/client';
6+
import { TestIntegration } from '../mocks/integration';
67

78
const PUBLIC_DSN = 'https://username@domain/path';
89

@@ -405,4 +406,16 @@ describe('BaseClient', () => {
405406
// TODO: Test rate limiting queues here
406407
});
407408
});
409+
410+
describe('integrations', () => {
411+
test('setup each one of them on install', () => {
412+
const client = new TestClient({
413+
dsn: PUBLIC_DSN,
414+
integrations: [new TestIntegration()],
415+
});
416+
expect(client.getIntegration('TestIntegration')).toBeFalsy();
417+
client.install();
418+
expect(client.getIntegration('TestIntegration')).toBeTruthy();
419+
});
420+
});
408421
});

‎packages/core/test/lib/integrations/inboundfilters.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,28 +65,28 @@ describe('InboundFilters', () => {
6565
};
6666

6767
it('string filter with partial match', () => {
68-
inboundFilters.install({
68+
inboundFilters.setupOnce({
6969
ignoreErrors: ['capture'],
7070
});
7171
expect(inboundFilters.isIgnoredError(messageEvent)).toBe(true);
7272
});
7373

7474
it('string filter with exact match', () => {
75-
inboundFilters.install({
75+
inboundFilters.setupOnce({
7676
ignoreErrors: ['captureMessage'],
7777
});
7878
expect(inboundFilters.isIgnoredError(messageEvent)).toBe(true);
7979
});
8080

8181
it('regexp filter with partial match', () => {
82-
inboundFilters.install({
82+
inboundFilters.setupOnce({
8383
ignoreErrors: [/capture/],
8484
});
8585
expect(inboundFilters.isIgnoredError(messageEvent)).toBe(true);
8686
});
8787

8888
it('regexp filter with exact match', () => {
89-
inboundFilters.install({
89+
inboundFilters.setupOnce({
9090
ignoreErrors: [/^captureMessage$/],
9191
});
9292
expect(inboundFilters.isIgnoredError(messageEvent)).toBe(true);

‎packages/core/test/lib/integrations/index.test.ts‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
import { Integration } from '@sentry/types';
2+
import { getIntegrationsToSetup } from '../../../src/integrations';
3+
4+
/** JSDoc */
5+
class MockIntegration implements Integration {
6+
public constructor(name: string) {
7+
this.name = name;
8+
}
9+
public name: string;
10+
}
11+
12+
describe('getIntegrationsToSetup', () => {
13+
it('works with empty array', () => {
14+
const integrations = getIntegrationsToSetup({
15+
integrations: [],
16+
});
17+
18+
expect(integrations.map(i => i.name)).toEqual([]);
19+
});
20+
21+
it('works with single item', () => {
22+
const integrations = getIntegrationsToSetup({
23+
integrations: [new MockIntegration('foo')],
24+
});
25+
26+
expect(integrations.map(i => i.name)).toEqual(['foo']);
27+
});
28+
29+
it('works with multiple items', () => {
30+
const integrations = getIntegrationsToSetup({
31+
integrations: [new MockIntegration('foo'), new MockIntegration('bar')],
32+
});
33+
34+
expect(integrations.map(i => i.name)).toEqual(['foo', 'bar']);
35+
});
36+
37+
it('filter duplicated items', () => {
38+
const integrations = getIntegrationsToSetup({
39+
integrations: [new MockIntegration('foo'), new MockIntegration('foo'), new MockIntegration('bar')],
40+
});
41+
42+
expect(integrations.map(i => i.name)).toEqual(['foo', 'bar']);
43+
});
44+
45+
it('filter duplicated items and always let first win', () => {
46+
const first = new MockIntegration('foo');
47+
(first as any).order = 'first';
48+
const second = new MockIntegration('foo');
49+
(second as any).order = 'second';
50+
51+
const integrations = getIntegrationsToSetup({
52+
integrations: [first, second, new MockIntegration('bar')],
53+
});
54+
55+
expect(integrations.map(i => i.name)).toEqual(['foo', 'bar']);
56+
expect((integrations[0] as any).order).toEqual('first');
57+
});
58+
59+
it('work with empty defaults', () => {
60+
const integrations = getIntegrationsToSetup({
61+
defaultIntegrations: [],
62+
});
63+
64+
expect(integrations.map(i => i.name)).toEqual([]);
65+
});
66+
67+
it('work with single defaults', () => {
68+
const integrations = getIntegrationsToSetup({
69+
defaultIntegrations: [new MockIntegration('foo')],
70+
});
71+
72+
expect(integrations.map(i => i.name)).toEqual(['foo']);
73+
});
74+
75+
it('work with multiple defaults', () => {
76+
const integrations = getIntegrationsToSetup({
77+
defaultIntegrations: [new MockIntegration('foo'), new MockIntegration('bar')],
78+
});
79+
80+
expect(integrations.map(i => i.name)).toEqual(['foo', 'bar']);
81+
});
82+
83+
it('work with user integrations and defaults and pick defaults first', () => {
84+
const integrations = getIntegrationsToSetup({
85+
defaultIntegrations: [new MockIntegration('foo')],
86+
integrations: [new MockIntegration('bar')],
87+
});
88+
89+
expect(integrations.map(i => i.name)).toEqual(['foo', 'bar']);
90+
});
91+
92+
it('work with user integrations and defaults and filter duplicates', () => {
93+
const integrations = getIntegrationsToSetup({
94+
defaultIntegrations: [new MockIntegration('foo'), new MockIntegration('foo')],
95+
integrations: [new MockIntegration('bar'), new MockIntegration('bar')],
96+
});
97+
98+
expect(integrations.map(i => i.name)).toEqual(['foo', 'bar']);
99+
});
100+
101+
it('user integrations override defaults', () => {
102+
const firstDefault = new MockIntegration('foo');
103+
(firstDefault as any).order = 'firstDefault';
104+
const secondDefault = new MockIntegration('bar');
105+
(secondDefault as any).order = 'secondDefault';
106+
const firstUser = new MockIntegration('foo');
107+
(firstUser as any).order = 'firstUser';
108+
const secondUser = new MockIntegration('bar');
109+
(secondUser as any).order = 'secondUser';
110+
111+
const integrations = getIntegrationsToSetup({
112+
defaultIntegrations: [firstDefault, secondDefault],
113+
integrations: [firstUser, secondUser],
114+
});
115+
116+
expect(integrations.map(i => i.name)).toEqual(['foo', 'bar']);
117+
expect((integrations[0] as any).order).toEqual('firstUser');
118+
expect((integrations[1] as any).order).toEqual('secondUser');
119+
});
120+
});

0 commit comments

Comments
 (0)