Skip to content

ref: Remove install function from client and backend #1888

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 1 commit into from
Feb 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 0 additions & 18 deletions packages/browser/src/backend.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { BaseBackend, Options } from '@sentry/core';
import { SentryEvent, SentryEventHint, Severity, Transport } from '@sentry/types';
import { SentryError } from '@sentry/utils/error';
import { isDOMError, isDOMException, isError, isErrorEvent, isPlainObject } from '@sentry/utils/is';
import { supportsBeacon, supportsFetch } from '@sentry/utils/supports';
import { SyncPromise } from '@sentry/utils/syncpromise';
Expand Down Expand Up @@ -33,23 +32,6 @@ export interface BrowserOptions extends Options {
* @hidden
*/
export class BrowserBackend extends BaseBackend<BrowserOptions> {
/**
* @inheritDoc
*/
public install(): boolean {
// We are only called by the client if the SDK is enabled and a valid Dsn
// has been configured. If no Dsn is present, this indicates a programming
// error.
const dsn = this.options.dsn;
if (!dsn) {
throw new SentryError('Invariant exception: install() must not be called when disabled');
}

Error.stackTraceLimit = 50;

return true;
}

/**
* @inheritdoc
*/
Expand Down
2 changes: 1 addition & 1 deletion packages/browser/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export class BrowserClient extends BaseClient<BrowserBackend, BrowserOptions> {
*
* @param options Configuration options for this SDK.
*/
public constructor(options: BrowserOptions) {
public constructor(options: BrowserOptions = {}) {
super(BrowserBackend, options);
}

Expand Down
2 changes: 2 additions & 0 deletions packages/browser/src/integrations/globalhandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ export class GlobalHandlers implements Integration {
* @inheritDoc
*/
public setupOnce(): void {
Error.stackTraceLimit = 50;

subscribe((stack: TraceKitStackTrace, _: boolean, error: Error) => {
// TODO: use stack.context to get a valuable information from TraceKit, eg.
// [
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/basebackend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export abstract class BaseBackend<O extends Options> implements Backend {
/** Cached transport used internally. */
protected transport: Transport;

/** Creates a new browser backend instance. */
/** Creates a new backend instance. */
public constructor(options: O) {
this.options = options;
if (!this.options.dsn) {
Expand Down
25 changes: 1 addition & 24 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,6 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
*/
private readonly dsn?: Dsn;

/**
* Stores whether installation has been performed and was successful. Before
* installing, this is undefined. Then it contains the success state.
*/
private installed?: boolean;

/** Array of used integrations. */
private readonly integrations: IntegrationIndex;

Expand All @@ -106,25 +100,8 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
if (options.dsn) {
this.dsn = new Dsn(options.dsn);
}
// We have to setup the integrations in the constructor since we do not want
// that anyone needs to call client.install();
this.integrations = setupIntegrations(this.options);
}

/**
* @inheritDoc
*/
public install(): boolean {
if (!this.isEnabled()) {
return (this.installed = false);
}

const backend = this.getBackend();
if (!this.installed && backend.install) {
backend.install();
}

return (this.installed = true);
this.integrations = setupIntegrations(this.options);
}

/**
Expand Down
14 changes: 0 additions & 14 deletions packages/core/src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,17 +145,6 @@ export interface Options {
* captureMessage('Custom message');
*/
export interface Client<O extends Options = Options> {
/**
* Installs the SDK if it hasn't been installed already.
*
* Since this performs modifications in the environment, such as instrumenting
* library functionality or adding signal handlers, this method will only
* execute once and cache its result.
*
* @returns If the installation was the successful or not.
*/
install(): boolean;

/**
* Captures an exception event and sends it to Sentry.
*
Expand Down Expand Up @@ -240,9 +229,6 @@ export interface Client<O extends Options = Options> {
* these any time and they should not be cached.
*/
export interface Backend {
/** Installs the SDK into the environment. */
install?(): boolean;

/** Creates a {@link SentryEvent} from an exception. */
eventFromException(exception: any, hint?: SentryEventHint): SyncPromise<SentryEvent>;

Expand Down
1 change: 0 additions & 1 deletion packages/core/src/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,4 @@ export function initAndBind<F extends Client, O extends Options>(clientClass: Cl

const client = new clientClass(options);
getCurrentHub().bindClient(client);
client.install();
}
38 changes: 0 additions & 38 deletions packages/core/test/lib/base.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,44 +62,6 @@ describe('BaseClient', () => {
});
});

describe('install()', () => {
test('calls install() on Backend', () => {
expect.assertions(1);
const client = new TestClient({ dsn: PUBLIC_DSN });
client.install();
expect(TestBackend.instance!.installed).toBe(1);
});

test('calls install() only once', () => {
expect.assertions(1);
const client = new TestClient({ dsn: PUBLIC_DSN });
client.install();
client.install();
expect(TestBackend.instance!.installed).toBe(1);
});

test('resolves the result of install()', () => {
expect.assertions(1);
const client = new TestClient({ mockInstallFailure: true });
const installed = client.install();
expect(installed).toBeFalsy();
});

test('does not install() when disabled', () => {
expect.assertions(1);
const client = new TestClient({ enabled: false, dsn: PUBLIC_DSN });
client.install();
expect(TestBackend.instance!.installed).toBe(0);
});

test('does not install() without Dsn', () => {
expect.assertions(1);
const client = new TestClient({});
client.install();
expect(TestBackend.instance!.installed).toBe(0);
});
});

describe('getOptions()', () => {
test('returns the options', () => {
expect.assertions(1);
Expand Down