Skip to content

Commit 99cc26d

Browse files
AbhiPrasadAbhiPrasad
committed
feat(core): Introduce separate client options (#4927)
This PR works toward differentiating the options that are passed into `Sentry.init` and the options passed into clients. We want to make this differentiation to minimize internal state in the client, and instead rely on the max number of items being passed in to the client constructor. We do this by explicitly differentiating between the options that are configured in sdk init (`Options`) and the options that are passed into the client constructor (`ClientOptions`).
1 parent 702bcdc commit 99cc26d

39 files changed

+685
-451
lines changed

‎packages/browser/src/client.ts

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,13 @@
11
import { BaseClient, NewTransport, Scope, SDK_VERSION } from '@sentry/core';
2-
import { Event, EventHint, Options, Severity, SeverityLevel, Transport } from '@sentry/types';
2+
import { ClientOptions, Event, EventHint, Options, Severity, SeverityLevel, Transport } from '@sentry/types';
33
import { getGlobalObject, logger, stackParserFromOptions } from '@sentry/utils';
44

55
import { eventFromException, eventFromMessage } from './eventbuilder';
66
import { IS_DEBUG_BUILD } from './flags';
77
import { injectReportDialog, ReportDialogOptions } from './helpers';
88
import { Breadcrumbs } from './integrations';
99

10-
/**
11-
* Configuration options for the Sentry Browser SDK.
12-
* @see BrowserClient for more information.
13-
*/
14-
export interface BrowserOptions extends Options {
10+
export interface BaseBrowserOptions {
1511
/**
1612
* A pattern for error URLs which should exclusively be sent to Sentry.
1713
* This is the opposite of {@link Options.denyUrls}.
@@ -27,19 +23,31 @@ export interface BrowserOptions extends Options {
2723
denyUrls?: Array<string | RegExp>;
2824
}
2925

26+
/**
27+
* Configuration options for the Sentry Browser SDK.
28+
* @see @sentry/types Options for more information.
29+
*/
30+
export interface BrowserOptions extends Options, BaseBrowserOptions {}
31+
32+
/**
33+
* Configuration options for the Sentry Browser SDK Client class
34+
* @see BrowserClient for more information.
35+
*/
36+
export interface BrowserClientOptions extends ClientOptions, BaseBrowserOptions {}
37+
3038
/**
3139
* The Sentry Browser SDK Client.
3240
*
3341
* @see BrowserOptions for documentation on configuration options.
3442
* @see SentryClient for usage documentation.
3543
*/
36-
export class BrowserClient extends BaseClient<BrowserOptions> {
44+
export class BrowserClient extends BaseClient<BrowserClientOptions> {
3745
/**
3846
* Creates a new Browser SDK instance.
3947
*
4048
* @param options Configuration options for this SDK.
4149
*/
42-
public constructor(options: BrowserOptions = {}, transport: Transport, newTransport?: NewTransport) {
50+
public constructor(options: BrowserClientOptions, transport: Transport, newTransport?: NewTransport) {
4351
options._metadata = options._metadata || {};
4452
options._metadata.sdk = options._metadata.sdk || {
4553
name: 'sentry.javascript.browser',
@@ -51,7 +59,6 @@ export class BrowserClient extends BaseClient<BrowserOptions> {
5159
],
5260
version: SDK_VERSION,
5361
};
54-
5562
super(options, transport, newTransport);
5663
}
5764

‎packages/browser/src/sdk.ts

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,20 @@
1-
import { getCurrentHub, initAndBind, Integrations as CoreIntegrations } from '@sentry/core';
1+
import { getCurrentHub, getIntegrationsToSetup, initAndBind, Integrations as CoreIntegrations } from '@sentry/core';
22
import { Hub } from '@sentry/types';
3-
import { addInstrumentationHandler, getGlobalObject, logger, resolvedSyncPromise } from '@sentry/utils';
3+
import {
4+
addInstrumentationHandler,
5+
getGlobalObject,
6+
logger,
7+
resolvedSyncPromise,
8+
stackParserFromOptions,
9+
supportsFetch,
10+
} from '@sentry/utils';
411

5-
import { BrowserClient, BrowserOptions } from './client';
12+
import { BrowserClient, BrowserClientOptions, BrowserOptions } from './client';
613
import { IS_DEBUG_BUILD } from './flags';
714
import { ReportDialogOptions, wrap as internalWrap } from './helpers';
815
import { Breadcrumbs, Dedupe, GlobalHandlers, LinkedErrors, TryCatch, UserAgent } from './integrations';
916
import { defaultStackParsers } from './stack-parsers';
17+
import { FetchTransport, XHRTransport } from './transports';
1018
import { setupBrowserTransport } from './transports/setup';
1119

1220
export const defaultIntegrations = [
@@ -97,9 +105,17 @@ export function init(options: BrowserOptions = {}): void {
97105
if (options.stackParser === undefined) {
98106
options.stackParser = defaultStackParsers;
99107
}
100-
101108
const { transport, newTransport } = setupBrowserTransport(options);
102-
initAndBind(BrowserClient, options, transport, newTransport);
109+
110+
const clientOptions: BrowserClientOptions = {
111+
...options,
112+
stackParser: stackParserFromOptions(options),
113+
integrations: getIntegrationsToSetup(options),
114+
// TODO(v7): get rid of transport being passed down below
115+
transport: options.transport || (supportsFetch() ? FetchTransport : XHRTransport),
116+
};
117+
118+
initAndBind(BrowserClient, clientOptions, transport, newTransport);
103119

104120
if (options.autoSessionTracking) {
105121
startSessionTracking();

‎packages/browser/src/transports/setup.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,10 @@ export interface BrowserTransportOptions extends BaseTransportOptions {
3131
* this function will return a ready to use `NewTransport`.
3232
*/
3333
// TODO(v7): Adjust return value when NewTransport is the default
34-
export function setupBrowserTransport(options: BrowserOptions): { transport: Transport; newTransport?: NewTransport } {
34+
export function setupBrowserTransport(options: BrowserOptions): {
35+
transport: Transport;
36+
newTransport?: NewTransport;
37+
} {
3538
if (!options.dsn) {
3639
// We return the noop transport here in case there is no Dsn.
3740
return { transport: new NoopTransport() };

‎packages/browser/test/unit/helper/browser-client-options.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import { NoopTransport } from '@sentry/core';
2+
3+
import { BrowserClientOptions } from '../../../src/client';
4+
5+
export function getDefaultBrowserClientOptions(options: Partial<BrowserClientOptions> = {}): BrowserClientOptions {
6+
return {
7+
integrations: [],
8+
transport: NoopTransport,
9+
stackParser: () => [],
10+
...options,
11+
};
12+
}

‎packages/browser/test/unit/index.test.ts

Lines changed: 47 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
showReportDialog,
1717
wrap,
1818
} from '../../src';
19+
import { getDefaultBrowserClientOptions } from './helper/browser-client-options';
1920
import { SimpleTransport } from './mocks/simpletransport';
2021

2122
const dsn = 'https://[email protected]/4291';
@@ -75,7 +76,8 @@ describe('SentryBrowser', () => {
7576
describe('showReportDialog', () => {
7677
describe('user', () => {
7778
const EX_USER = { email: '[email protected]' };
78-
const client = new BrowserClient({ dsn }, new SimpleTransport({ dsn }));
79+
const options = getDefaultBrowserClientOptions({ dsn });
80+
const client = new BrowserClient(options, new SimpleTransport({ dsn }));
7981
const reportDialogSpy = jest.spyOn(client, 'showReportDialog');
8082

8183
beforeEach(() => {
@@ -139,53 +141,41 @@ describe('SentryBrowser', () => {
139141
});
140142

141143
it('should capture a message', done => {
142-
getCurrentHub().bindClient(
143-
new BrowserClient(
144-
{
145-
beforeSend: (event: Event): Event | null => {
146-
expect(event.message).toBe('test');
147-
expect(event.exception).toBeUndefined();
148-
done();
149-
return event;
150-
},
151-
dsn,
152-
},
153-
new SimpleTransport({ dsn }),
154-
),
155-
);
144+
const options = getDefaultBrowserClientOptions({
145+
beforeSend: (event: Event): Event | null => {
146+
expect(event.message).toBe('test');
147+
expect(event.exception).toBeUndefined();
148+
done();
149+
return event;
150+
},
151+
dsn,
152+
});
153+
getCurrentHub().bindClient(new BrowserClient(options, new SimpleTransport({ dsn })));
156154
captureMessage('test');
157155
});
158156

159157
it('should capture an event', done => {
160-
getCurrentHub().bindClient(
161-
new BrowserClient(
162-
{
163-
beforeSend: (event: Event): Event | null => {
164-
expect(event.message).toBe('event');
165-
expect(event.exception).toBeUndefined();
166-
done();
167-
return event;
168-
},
169-
dsn,
170-
},
171-
new SimpleTransport({ dsn }),
172-
),
173-
);
158+
const options = getDefaultBrowserClientOptions({
159+
beforeSend: (event: Event): Event | null => {
160+
expect(event.message).toBe('event');
161+
expect(event.exception).toBeUndefined();
162+
done();
163+
return event;
164+
},
165+
dsn,
166+
});
167+
getCurrentHub().bindClient(new BrowserClient(options, new SimpleTransport({ dsn })));
174168
captureEvent({ message: 'event' });
175169
});
176170

177171
it('should not dedupe an event on bound client', async () => {
178172
const localBeforeSend = jest.fn();
179-
getCurrentHub().bindClient(
180-
new BrowserClient(
181-
{
182-
beforeSend: localBeforeSend,
183-
dsn,
184-
integrations: [],
185-
},
186-
new SimpleTransport({ dsn }),
187-
),
188-
);
173+
const options = getDefaultBrowserClientOptions({
174+
beforeSend: localBeforeSend,
175+
dsn,
176+
integrations: [],
177+
});
178+
getCurrentHub().bindClient(new BrowserClient(options, new SimpleTransport({ dsn })));
189179

190180
captureMessage('event222');
191181
captureMessage('event222');
@@ -197,16 +187,12 @@ describe('SentryBrowser', () => {
197187

198188
it('should use inboundfilter rules of bound client', async () => {
199189
const localBeforeSend = jest.fn();
200-
getCurrentHub().bindClient(
201-
new BrowserClient(
202-
{
203-
beforeSend: localBeforeSend,
204-
dsn,
205-
integrations: [new Integrations.InboundFilters({ ignoreErrors: ['capture'] })],
206-
},
207-
new SimpleTransport({ dsn }),
208-
),
209-
);
190+
const options = getDefaultBrowserClientOptions({
191+
beforeSend: localBeforeSend,
192+
dsn,
193+
integrations: [new Integrations.InboundFilters({ ignoreErrors: ['capture'] })],
194+
});
195+
getCurrentHub().bindClient(new BrowserClient(options, new SimpleTransport({ dsn })));
210196

211197
captureMessage('capture');
212198

@@ -267,7 +253,8 @@ describe('SentryBrowser initialization', () => {
267253
});
268254

269255
it('should set SDK data when instantiating a client directly', () => {
270-
const client = new BrowserClient({ dsn }, new SimpleTransport({ dsn }));
256+
const options = getDefaultBrowserClientOptions({ dsn });
257+
const client = new BrowserClient(options, new SimpleTransport({ dsn }));
271258

272259
const sdkData = (client.getTransport() as any)._api.metadata?.sdk;
273260

@@ -309,20 +296,16 @@ describe('SentryBrowser initialization', () => {
309296

310297
describe('wrap()', () => {
311298
it('should wrap and call function while capturing error', done => {
312-
getCurrentHub().bindClient(
313-
new BrowserClient(
314-
{
315-
beforeSend: (event: Event): Event | null => {
316-
expect(event.exception!.values![0].type).toBe('TypeError');
317-
expect(event.exception!.values![0].value).toBe('mkey');
318-
done();
319-
return null;
320-
},
321-
dsn,
322-
},
323-
new SimpleTransport({ dsn }),
324-
),
325-
);
299+
const options = getDefaultBrowserClientOptions({
300+
beforeSend: (event: Event): Event | null => {
301+
expect(event.exception!.values![0].type).toBe('TypeError');
302+
expect(event.exception!.values![0].value).toBe('mkey');
303+
done();
304+
return null;
305+
},
306+
dsn,
307+
});
308+
getCurrentHub().bindClient(new BrowserClient(options, new SimpleTransport({ dsn })));
326309

327310
try {
328311
wrap(() => {

‎packages/browser/test/unit/integrations/linkederrors.test.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { BrowserClient } from '../../../src/client';
55
import * as LinkedErrorsModule from '../../../src/integrations/linkederrors';
66
import { defaultStackParsers } from '../../../src/stack-parsers';
77
import { setupBrowserTransport } from '../../../src/transports';
8+
import { getDefaultBrowserClientOptions } from '../helper/browser-client-options';
89

910
const parser = createStackParser(...defaultStackParsers);
1011

@@ -45,7 +46,7 @@ describe('LinkedErrors', () => {
4546
one.cause = two;
4647

4748
const originalException = one;
48-
const options = { stackParser: parser };
49+
const options = getDefaultBrowserClientOptions({ stackParser: parser });
4950
const client = new BrowserClient(options, setupBrowserTransport(options).transport);
5051
return client.eventFromException(originalException).then(event => {
5152
const result = LinkedErrorsModule._handler(parser, 'cause', 5, event, {
@@ -76,7 +77,7 @@ describe('LinkedErrors', () => {
7677
one.reason = two;
7778

7879
const originalException = one;
79-
const options = { stackParser: parser };
80+
const options = getDefaultBrowserClientOptions({ stackParser: parser });
8081
const client = new BrowserClient(options, setupBrowserTransport(options).transport);
8182
return client.eventFromException(originalException).then(event => {
8283
const result = LinkedErrorsModule._handler(parser, 'reason', 5, event, {
@@ -103,7 +104,7 @@ describe('LinkedErrors', () => {
103104
one.cause = two;
104105
two.cause = three;
105106

106-
const options = { stackParser: parser };
107+
const options = getDefaultBrowserClientOptions({ stackParser: parser });
107108
const client = new BrowserClient(options, setupBrowserTransport(options).transport);
108109
const originalException = one;
109110
return client.eventFromException(originalException).then(event => {

0 commit comments

Comments
 (0)