Skip to content

Commit

Permalink
feat(core): Introduce seperate client options (#4927)
Browse files Browse the repository at this point in the history
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`).
  • Loading branch information
AbhiPrasad authored Apr 19, 2022
1 parent a5ef02c commit 6227c7f
Show file tree
Hide file tree
Showing 39 changed files with 685 additions and 451 deletions.
25 changes: 16 additions & 9 deletions packages/browser/src/client.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,13 @@
import { BaseClient, NewTransport, Scope, SDK_VERSION } from '@sentry/core';
import { Event, EventHint, Options, Severity, SeverityLevel, Transport } from '@sentry/types';
import { ClientOptions, Event, EventHint, Options, Severity, SeverityLevel, Transport } from '@sentry/types';
import { getGlobalObject, logger, stackParserFromOptions } from '@sentry/utils';

import { eventFromException, eventFromMessage } from './eventbuilder';
import { IS_DEBUG_BUILD } from './flags';
import { injectReportDialog, ReportDialogOptions } from './helpers';
import { Breadcrumbs } from './integrations';

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

/**
* Configuration options for the Sentry Browser SDK.
* @see @sentry/types Options for more information.
*/
export interface BrowserOptions extends Options, BaseBrowserOptions {}

/**
* Configuration options for the Sentry Browser SDK Client class
* @see BrowserClient for more information.
*/
export interface BrowserClientOptions extends ClientOptions, BaseBrowserOptions {}

/**
* The Sentry Browser SDK Client.
*
* @see BrowserOptions for documentation on configuration options.
* @see SentryClient for usage documentation.
*/
export class BrowserClient extends BaseClient<BrowserOptions> {
export class BrowserClient extends BaseClient<BrowserClientOptions> {
/**
* Creates a new Browser SDK instance.
*
* @param options Configuration options for this SDK.
*/
public constructor(options: BrowserOptions = {}, transport: Transport, newTransport?: NewTransport) {
public constructor(options: BrowserClientOptions, transport: Transport, newTransport?: NewTransport) {
options._metadata = options._metadata || {};
options._metadata.sdk = options._metadata.sdk || {
name: 'sentry.javascript.browser',
Expand All @@ -51,7 +59,6 @@ export class BrowserClient extends BaseClient<BrowserOptions> {
],
version: SDK_VERSION,
};

super(options, transport, newTransport);
}

Expand Down
26 changes: 21 additions & 5 deletions packages/browser/src/sdk.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,20 @@
import { getCurrentHub, initAndBind, Integrations as CoreIntegrations } from '@sentry/core';
import { getCurrentHub, getIntegrationsToSetup, initAndBind, Integrations as CoreIntegrations } from '@sentry/core';
import { Hub } from '@sentry/types';
import { addInstrumentationHandler, getGlobalObject, logger, resolvedSyncPromise } from '@sentry/utils';
import {
addInstrumentationHandler,
getGlobalObject,
logger,
resolvedSyncPromise,
stackParserFromOptions,
supportsFetch,
} from '@sentry/utils';

import { BrowserClient, BrowserOptions } from './client';
import { BrowserClient, BrowserClientOptions, BrowserOptions } from './client';
import { IS_DEBUG_BUILD } from './flags';
import { ReportDialogOptions, wrap as internalWrap } from './helpers';
import { Breadcrumbs, Dedupe, GlobalHandlers, LinkedErrors, TryCatch, UserAgent } from './integrations';
import { defaultStackParsers } from './stack-parsers';
import { FetchTransport, XHRTransport } from './transports';
import { setupBrowserTransport } from './transports/setup';

export const defaultIntegrations = [
Expand Down Expand Up @@ -97,9 +105,17 @@ export function init(options: BrowserOptions = {}): void {
if (options.stackParser === undefined) {
options.stackParser = defaultStackParsers;
}

const { transport, newTransport } = setupBrowserTransport(options);
initAndBind(BrowserClient, options, transport, newTransport);

const clientOptions: BrowserClientOptions = {
...options,
stackParser: stackParserFromOptions(options),
integrations: getIntegrationsToSetup(options),
// TODO(v7): get rid of transport being passed down below
transport: options.transport || (supportsFetch() ? FetchTransport : XHRTransport),
};

initAndBind(BrowserClient, clientOptions, transport, newTransport);

if (options.autoSessionTracking) {
startSessionTracking();
Expand Down
5 changes: 4 additions & 1 deletion packages/browser/src/transports/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ export interface BrowserTransportOptions extends BaseTransportOptions {
* this function will return a ready to use `NewTransport`.
*/
// TODO(v7): Adjust return value when NewTransport is the default
export function setupBrowserTransport(options: BrowserOptions): { transport: Transport; newTransport?: NewTransport } {
export function setupBrowserTransport(options: BrowserOptions): {
transport: Transport;
newTransport?: NewTransport;
} {
if (!options.dsn) {
// We return the noop transport here in case there is no Dsn.
return { transport: new NoopTransport() };
Expand Down
12 changes: 12 additions & 0 deletions packages/browser/test/unit/helper/browser-client-options.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { NoopTransport } from '@sentry/core';

import { BrowserClientOptions } from '../../../src/client';

export function getDefaultBrowserClientOptions(options: Partial<BrowserClientOptions> = {}): BrowserClientOptions {
return {
integrations: [],
transport: NoopTransport,
stackParser: () => [],
...options,
};
}
111 changes: 47 additions & 64 deletions packages/browser/test/unit/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
showReportDialog,
wrap,
} from '../../src';
import { getDefaultBrowserClientOptions } from './helper/browser-client-options';
import { SimpleTransport } from './mocks/simpletransport';

const dsn = 'https://[email protected]/4291';
Expand Down Expand Up @@ -75,7 +76,8 @@ describe('SentryBrowser', () => {
describe('showReportDialog', () => {
describe('user', () => {
const EX_USER = { email: '[email protected]' };
const client = new BrowserClient({ dsn }, new SimpleTransport({ dsn }));
const options = getDefaultBrowserClientOptions({ dsn });
const client = new BrowserClient(options, new SimpleTransport({ dsn }));
const reportDialogSpy = jest.spyOn(client, 'showReportDialog');

beforeEach(() => {
Expand Down Expand Up @@ -139,53 +141,41 @@ describe('SentryBrowser', () => {
});

it('should capture a message', done => {
getCurrentHub().bindClient(
new BrowserClient(
{
beforeSend: (event: Event): Event | null => {
expect(event.message).toBe('test');
expect(event.exception).toBeUndefined();
done();
return event;
},
dsn,
},
new SimpleTransport({ dsn }),
),
);
const options = getDefaultBrowserClientOptions({
beforeSend: (event: Event): Event | null => {
expect(event.message).toBe('test');
expect(event.exception).toBeUndefined();
done();
return event;
},
dsn,
});
getCurrentHub().bindClient(new BrowserClient(options, new SimpleTransport({ dsn })));
captureMessage('test');
});

it('should capture an event', done => {
getCurrentHub().bindClient(
new BrowserClient(
{
beforeSend: (event: Event): Event | null => {
expect(event.message).toBe('event');
expect(event.exception).toBeUndefined();
done();
return event;
},
dsn,
},
new SimpleTransport({ dsn }),
),
);
const options = getDefaultBrowserClientOptions({
beforeSend: (event: Event): Event | null => {
expect(event.message).toBe('event');
expect(event.exception).toBeUndefined();
done();
return event;
},
dsn,
});
getCurrentHub().bindClient(new BrowserClient(options, new SimpleTransport({ dsn })));
captureEvent({ message: 'event' });
});

it('should not dedupe an event on bound client', async () => {
const localBeforeSend = jest.fn();
getCurrentHub().bindClient(
new BrowserClient(
{
beforeSend: localBeforeSend,
dsn,
integrations: [],
},
new SimpleTransport({ dsn }),
),
);
const options = getDefaultBrowserClientOptions({
beforeSend: localBeforeSend,
dsn,
integrations: [],
});
getCurrentHub().bindClient(new BrowserClient(options, new SimpleTransport({ dsn })));

captureMessage('event222');
captureMessage('event222');
Expand All @@ -197,16 +187,12 @@ describe('SentryBrowser', () => {

it('should use inboundfilter rules of bound client', async () => {
const localBeforeSend = jest.fn();
getCurrentHub().bindClient(
new BrowserClient(
{
beforeSend: localBeforeSend,
dsn,
integrations: [new Integrations.InboundFilters({ ignoreErrors: ['capture'] })],
},
new SimpleTransport({ dsn }),
),
);
const options = getDefaultBrowserClientOptions({
beforeSend: localBeforeSend,
dsn,
integrations: [new Integrations.InboundFilters({ ignoreErrors: ['capture'] })],
});
getCurrentHub().bindClient(new BrowserClient(options, new SimpleTransport({ dsn })));

captureMessage('capture');

Expand Down Expand Up @@ -267,7 +253,8 @@ describe('SentryBrowser initialization', () => {
});

it('should set SDK data when instantiating a client directly', () => {
const client = new BrowserClient({ dsn }, new SimpleTransport({ dsn }));
const options = getDefaultBrowserClientOptions({ dsn });
const client = new BrowserClient(options, new SimpleTransport({ dsn }));

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

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

describe('wrap()', () => {
it('should wrap and call function while capturing error', done => {
getCurrentHub().bindClient(
new BrowserClient(
{
beforeSend: (event: Event): Event | null => {
expect(event.exception!.values![0].type).toBe('TypeError');
expect(event.exception!.values![0].value).toBe('mkey');
done();
return null;
},
dsn,
},
new SimpleTransport({ dsn }),
),
);
const options = getDefaultBrowserClientOptions({
beforeSend: (event: Event): Event | null => {
expect(event.exception!.values![0].type).toBe('TypeError');
expect(event.exception!.values![0].value).toBe('mkey');
done();
return null;
},
dsn,
});
getCurrentHub().bindClient(new BrowserClient(options, new SimpleTransport({ dsn })));

try {
wrap(() => {
Expand Down
7 changes: 4 additions & 3 deletions packages/browser/test/unit/integrations/linkederrors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { BrowserClient } from '../../../src/client';
import * as LinkedErrorsModule from '../../../src/integrations/linkederrors';
import { defaultStackParsers } from '../../../src/stack-parsers';
import { setupBrowserTransport } from '../../../src/transports';
import { getDefaultBrowserClientOptions } from '../helper/browser-client-options';

const parser = createStackParser(...defaultStackParsers);

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

const originalException = one;
const options = { stackParser: parser };
const options = getDefaultBrowserClientOptions({ stackParser: parser });
const client = new BrowserClient(options, setupBrowserTransport(options).transport);
return client.eventFromException(originalException).then(event => {
const result = LinkedErrorsModule._handler(parser, 'cause', 5, event, {
Expand Down Expand Up @@ -76,7 +77,7 @@ describe('LinkedErrors', () => {
one.reason = two;

const originalException = one;
const options = { stackParser: parser };
const options = getDefaultBrowserClientOptions({ stackParser: parser });
const client = new BrowserClient(options, setupBrowserTransport(options).transport);
return client.eventFromException(originalException).then(event => {
const result = LinkedErrorsModule._handler(parser, 'reason', 5, event, {
Expand All @@ -103,7 +104,7 @@ describe('LinkedErrors', () => {
one.cause = two;
two.cause = three;

const options = { stackParser: parser };
const options = getDefaultBrowserClientOptions({ stackParser: parser });
const client = new BrowserClient(options, setupBrowserTransport(options).transport);
const originalException = one;
return client.eventFromException(originalException).then(event => {
Expand Down
Loading

0 comments on commit 6227c7f

Please sign in to comment.