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 21 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
18 changes: 14 additions & 4 deletions packages/browser/test/unit/integrations/linkederrors.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,20 @@
import { Event as SentryEvent, Exception, ExtendedError } from '@sentry/types';
import { createStackParser } from '@sentry/utils';

import { BrowserClient } from '../../../src/client';
import { BrowserClient, BrowserClientOptions } from '../../../src/client';
import * as LinkedErrorsModule from '../../../src/integrations/linkederrors';
import { defaultStackParsers } from '../../../src/stack-parsers';
import { setupBrowserTransport } from '../../../src/transports';
import { NoopTransport } from '@sentry/core/src/transports/noop';

function getDefaultBrowserOptions(options: Partial<BrowserClientOptions> = {}): BrowserClientOptions {
return {
integrations: [],
transport: NoopTransport,
stackParser: () => [],
...options,
};
}

const parser = createStackParser(...defaultStackParsers);

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

const originalException = one;
const options = { stackParser: parser };
const options = getDefaultBrowserOptions({ 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 +86,7 @@ describe('LinkedErrors', () => {
one.reason = two;

const originalException = one;
const options = { stackParser: parser };
const options = getDefaultBrowserOptions({ 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 +113,7 @@ describe('LinkedErrors', () => {
one.cause = two;
two.cause = three;

const options = { stackParser: parser };
const options = getDefaultBrowserOptions({ stackParser: parser });
const client = new BrowserClient(options, setupBrowserTransport(options).transport);
const originalException = one;
return client.eventFromException(originalException).then(event => {
Expand Down
16 changes: 13 additions & 3 deletions packages/browser/test/unit/transports/setup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,16 @@ import {
XHRTransport,
} from '../../../src/transports';
import { SimpleTransport } from '../mocks/simpletransport';
import { BrowserClientOptions } from '../../../src/client';

function getDefaultBrowserOptions(options: Partial<BrowserClientOptions> = {}): BrowserClientOptions {
return {
integrations: [],
transport: NoopTransport,
stackParser: () => [],
...options,
};
}

const DSN = 'https://username@domain/123';

Expand Down Expand Up @@ -64,7 +74,7 @@ describe('setupBrowserTransport', () => {
});

it('returns the instantiated transport passed via the options', () => {
const options = { dsn: DSN, transport: SimpleTransport };
const options = getDefaultBrowserOptions({ dsn: DSN, transport: SimpleTransport });
const { transport, newTransport } = setupBrowserTransport(options);

expect(transport).toBeDefined();
Expand All @@ -73,7 +83,7 @@ describe('setupBrowserTransport', () => {
});

it('returns fetchTransports if fetch is supported', () => {
const options = { dsn: DSN };
const options = getDefaultBrowserOptions({ dsn: DSN });
const { transport, newTransport } = setupBrowserTransport(options);

expect(transport).toBeDefined();
Expand All @@ -86,7 +96,7 @@ describe('setupBrowserTransport', () => {
it('returns xhrTransports if fetch is not supported', () => {
fetchSupported = false;

const options = { dsn: DSN };
const options = getDefaultBrowserOptions({ dsn: DSN });
const { transport, newTransport } = setupBrowserTransport(options);

expect(transport).toBeDefined();
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
import { Scope, Session } from '@sentry/hub';
import {
Client,
ClientOptions,
DsnComponents,
Event,
EventHint,
Integration,
IntegrationClass,
Options,
Severity,
SeverityLevel,
Transport,
Expand Down Expand Up @@ -68,7 +68,7 @@ const ALREADY_SEEN_ERROR = "Not capturing exception because it's already been ca
* // ...
* }
*/
export abstract class BaseClient<O extends Options> implements Client<O> {
export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
/** Options passed to the SDK. */
protected readonly _options: O;

Expand Down
1 change: 1 addition & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export { initAndBind } from './sdk';
export { NoopTransport } from './transports/noop';
export { createTransport } from './transports/base';
export { SDK_VERSION } from './version';
export { getIntegrationsToSetup } from './integration';

import * as Integrations from './integrations';

Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/integration.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { addGlobalEventProcessor, getCurrentHub } from '@sentry/hub';
import { Integration, Options } from '@sentry/types';
import { ClientOptions, Integration, Options } from '@sentry/types';
import { addNonEnumerableProperty, logger } from '@sentry/utils';

import { IS_DEBUG_BUILD } from './flags';
Expand Down Expand Up @@ -70,9 +70,9 @@ export function setupIntegration(integration: Integration): void {
* @param integrations array of integration instances
* @param withDefault should enable default integrations
*/
export function setupIntegrations<O extends Options>(options: O): IntegrationIndex {
export function setupIntegrations<O extends ClientOptions>(options: O): IntegrationIndex {
const integrations: IntegrationIndex = {};
getIntegrationsToSetup(options).forEach(integration => {
options.integrations.forEach(integration => {
integrations[integration.name] = integration;
setupIntegration(integration);
});
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/sdk.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { getCurrentHub } from '@sentry/hub';
import { Client, Options, Transport } from '@sentry/types';
import { Client, ClientOptions, Transport } from '@sentry/types';
import { logger } from '@sentry/utils';

import { IS_DEBUG_BUILD } from './flags';
import { NewTransport } from './transports/base';

/** A class object that can instantiate Client objects. */
export type ClientClass<F extends Client, O extends Options> = new (
export type ClientClass<F extends Client, O extends ClientOptions> = new (
options: O,
transport: Transport,
newTransport?: NewTransport,
Expand All @@ -19,7 +19,7 @@ export type ClientClass<F extends Client, O extends Options> = new (
* @param clientClass The client class to instantiate.
* @param options Options to pass to the client.
*/
export function initAndBind<F extends Client, O extends Options>(
export function initAndBind<F extends Client, O extends ClientOptions>(
clientClass: ClientClass<F, O>,
options: O,
transport: Transport,
Expand Down
Loading