Skip to content

feat(core): Introduce seperate client options #4927

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 33 commits into from
Apr 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
2c06ccc
feat(core): Introduce seperate client options
AbhiPrasad Apr 12, 2022
3ad3988
add client options to baseclient
AbhiPrasad Apr 14, 2022
945b10a
updates types
AbhiPrasad Apr 14, 2022
d0d03c9
introduce diff option types
AbhiPrasad Apr 14, 2022
23d16a0
introduce diff browser types:
AbhiPrasad Apr 14, 2022
9167493
node client options
AbhiPrasad Apr 14, 2022
61a26dd
switch browser
AbhiPrasad Apr 14, 2022
b411f02
don't do anything with the dsn
AbhiPrasad Apr 14, 2022
f6d09ab
dsn type fix
AbhiPrasad Apr 14, 2022
698a8f7
fix export
AbhiPrasad Apr 14, 2022
0256a7b
make dsn
AbhiPrasad Apr 14, 2022
e8335f2
missing import
AbhiPrasad Apr 14, 2022
5b4eec9
remove default core options
AbhiPrasad Apr 18, 2022
69d7f6a
add comments
AbhiPrasad Apr 18, 2022
f3bce9e
cast option types
AbhiPrasad Apr 18, 2022
4e9fb6b
explicit types
AbhiPrasad Apr 18, 2022
77b4a21
fix import
AbhiPrasad Apr 18, 2022
bd62e26
fix tests
AbhiPrasad Apr 19, 2022
e26e615
fix transport types
AbhiPrasad Apr 19, 2022
e3d1681
yarn fix
AbhiPrasad Apr 19, 2022
5bbcc5c
round 1 of fixing tests
AbhiPrasad Apr 19, 2022
50e9363
rename `TestOptions -> TestClientOptions`
Lms24 Apr 19, 2022
94df681
tmp fix core `sdk.test.ts`
Lms24 Apr 19, 2022
68165d5
Add tests for integration setup in node init
lforst Apr 19, 2022
9371d48
Further fix node tests
lforst Apr 19, 2022
ba61707
add tests for integration setup in browser init
Lms24 Apr 19, 2022
957452f
Fix some eslint errors in node tests
lforst Apr 19, 2022
0e142ab
Extract `getDefaultNodeClientOptions` into helper module
lforst Apr 19, 2022
bb8f917
fix remaning browser unit tests
Lms24 Apr 19, 2022
fed3fd0
fix tracing unit tests
Lms24 Apr 19, 2022
88d4cff
fix linter errors
Lms24 Apr 19, 2022
ea99698
fix another set of linter errors
Lms24 Apr 19, 2022
3d098ff
fix lint in browser sdk unit test
AbhiPrasad Apr 19, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a huge hack, but given we are switching to the new transports, left it as is.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

};

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