Skip to content

[NEXT] Frontend -> Client #1323

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.

Sign up for GitHub

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 6 commits into from
May 11, 2018
Merged

[NEXT] Frontend -> Client #1323

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c06d1e6
fix: This can be null under some circumstances
HazAT Apr 20, 2018
ff8b2db
'next' of github.com:getsentry/raven-js into next
HazAT May 9, 2018
eb9d11b
ref: Rename Frontend to Client
HazAT May 9, 2018
7f7904f
ref: ClientBase -> BaseClient
HazAT May 9, 2018
0c83af2
ref: Remove client and pass options instead
HazAT May 9, 2018
7ca8844
ref: Add correct type checks for raven-js send
HazAT May 11, 2018
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
13 changes: 4 additions & 9 deletions packages/browser/src/backend.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Backend, Client, Options, SentryError } from '@sentry/core';
import { Backend, Options, SentryError } from '@sentry/core';
import {
addBreadcrumb,
captureEvent,
Expand Down Expand Up @@ -67,13 +67,8 @@ export interface BrowserOptions extends Options {

/** The Sentry Browser SDK Backend. */
export class BrowserBackend implements Backend {
/** Handle to the SDK client for callbacks. */
private readonly client: Client<BrowserOptions>;

/** Creates a new browser backend instance. */
public constructor(client: Client<BrowserOptions>) {
this.client = client;
}
public constructor(private readonly options: BrowserOptions) {}
Copy link
Member

Choose a reason for hiding this comment

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

We probably also want the DSN here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking that we only pass in the options and if we need some fancy behavior our DSN class supports, we create a DSN object in the backend.

See: https://github.com/getsentry/raven-js/pull/1323/files#diff-261a7b228c5d644e74c94c3f99ba8a9eR147

Copy link
Member

Choose a reason for hiding this comment

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

WFM. We can assume that the DSN must be valid at that point, so there's also no need to catch exceptions anymore.


/**
* @inheritDoc
Expand All @@ -82,14 +77,14 @@ export class BrowserBackend implements Backend {
// 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.client.getDSN();
const dsn = this.options.dsn;
if (!dsn) {
throw new SentryError(
'Invariant exception: install() must not be called when disabled',
);
}

Raven.config(dsn.toString(), this.client.getOptions()).install();
Raven.config(dsn, this.options).install();

// Hook into Raven's breadcrumb mechanism. This allows us to intercept both
// breadcrumbs created internally by Raven and pass them to the Client
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/base.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const MAX_BREADCRUMBS = 100;

/** A class object that can instanciate Backend objects. */
export interface BackendClass<B extends Backend, O extends Options> {
new (client: Client<O>): B;
new (options: O): B;
}

/**
Expand Down Expand Up @@ -93,7 +93,7 @@ export abstract class BaseClient<B extends Backend, O extends Options>
* @param options Options for the client.
*/
protected constructor(backendClass: BackendClass<B, O>, options: O) {
this.backend = new backendClass(this);
this.backend = new backendClass(options);
this.options = options;

if (options.dsn) {
Expand Down
4 changes: 2 additions & 2 deletions packages/core/test/mocks/backend.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ export class TestBackend implements Backend {
public installed: number;
public event?: SentryEvent;

public constructor(private readonly client: Client<TestOptions>) {
public constructor(private readonly options: TestOptions) {
TestBackend.instance = this;
this.installed = 0;
}

public install(): boolean {
this.installed += 1;
return !this.client.getOptions().mockInstallFailure;
return !this.options.mockInstallFailure;
}

public async eventFromException(exception: any): Promise<SentryEvent> {
Expand Down
22 changes: 8 additions & 14 deletions packages/node/src/backend.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Backend, Client, Options, SentryError } from '@sentry/core';
import { Backend, DSN, Options, SentryError } from '@sentry/core';
import { addBreadcrumb, captureEvent, SentryEvent } from '@sentry/shim';
import {
HTTPSTransport,
Expand Down Expand Up @@ -44,13 +44,8 @@ export interface NodeOptions extends Options {

/** The Sentry Node SDK Backend. */
export class NodeBackend implements Backend {
/** Handle to the SDK client for callbacks. */
private readonly client: Client<NodeOptions>;

/** Creates a new Node backend instance. */
public constructor(client: Client<NodeOptions>) {
this.client = client;
}
public constructor(private readonly options: NodeOptions) {}

/**
* @inheritDoc
Expand All @@ -59,17 +54,15 @@ export class NodeBackend implements Backend {
// 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.client.getDSN();
const dsn = this.options.dsn;
if (!dsn) {
throw new SentryError(
'Invariant exception: install() must not be called when disabled',
);
}

const { onFatalError } = this.client.getOptions();
Raven.config(dsn.toString(true), this.client.getOptions()).install(
onFatalError,
);
const { onFatalError } = this.options;
Raven.config(dsn, this.options).install(onFatalError);

// Hook into Raven's breadcrumb mechanism. This allows us to intercept both
// breadcrumbs created internally by Raven and pass them to the Client
Expand Down Expand Up @@ -147,13 +140,14 @@ export class NodeBackend implements Backend {
* @param transport The transport to use for submitting events.
*/
public setTransport(transport: Transport): void {
const dsn = this.client.getDSN();
const dsn = this.options.dsn;
if (!dsn) {
return;
}
const dsnObject = new DSN(dsn);

Raven.transport =
dsn.protocol === 'http'
dsnObject.protocol === 'http'
? new HTTPTransport({ transport })
: new HTTPSTransport({ transport });
}
Expand Down