Skip to content

Commit

Permalink
feat: Switch to new transports (#4943)
Browse files Browse the repository at this point in the history
This PR switches over from the old transports to the new ones. It deletes the functionality from the client of passing in transports explicitly and instead expects the transport to be passed in through options.

Instead of passing in an instance of a transport to the client, we pass in a constructor function. This allows the client to control exactly what options are passed into the transport.

```ts
this._transport = options.transport({ ...options.transportOptions, url });
```
  • Loading branch information
AbhiPrasad authored Apr 25, 2022
1 parent 96b37e7 commit 6267b54
Show file tree
Hide file tree
Showing 136 changed files with 932 additions and 1,494 deletions.
6 changes: 0 additions & 6 deletions MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,6 @@ For our efforts to reduce bundle size of the SDK we had to remove and refactor p
[#4919](https://github.com/getsentry/sentry-javascript/pull/4919)). `Backend` was an unnecessary abstraction which is
not present in other Sentry SDKs. For the sake of reducing complexity, increasing consistency with other Sentry SDKs and
decreasing bundle-size, `Backend` was removed.
<!-- TODO(v7): Add more info and PR link for passing transports in options once this is done -->
<!-- TODO(v7): This needs refinement once NewTransport is the default (maybe this should get its own section with an expamp) -->
- Inject transport into client instead of initializing it in the client in `setupTransport` (see
[#4921](https://github.com/getsentry/sentry-javascript/pull/4921/)). If you are creating your own `Client` or
calling `initAndBind`, you will have to supply your desired transport. Either provide a custom one or call
`setupBrowserTransport` or `setupNodeTransport` for default transports, depending on your requirements.
- Remove support for Opera browser pre v15

## Sentry Angular SDK Changes
Expand Down
8 changes: 4 additions & 4 deletions packages/browser/src/client.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { BaseClient, NewTransport, Scope, SDK_VERSION } from '@sentry/core';
import { ClientOptions, Event, EventHint, Options, Severity, SeverityLevel, Transport } from '@sentry/types';
import { BaseClient, Scope, SDK_VERSION } from '@sentry/core';
import { ClientOptions, Event, EventHint, Options, Severity, SeverityLevel } from '@sentry/types';
import { getGlobalObject, logger } from '@sentry/utils';

import { eventFromException, eventFromMessage } from './eventbuilder';
Expand Down Expand Up @@ -47,7 +47,7 @@ export class BrowserClient extends BaseClient<BrowserClientOptions> {
*
* @param options Configuration options for this SDK.
*/
public constructor(options: BrowserClientOptions, transport: Transport, newTransport?: NewTransport) {
public constructor(options: BrowserClientOptions) {
options._metadata = options._metadata || {};
options._metadata.sdk = options._metadata.sdk || {
name: 'sentry.javascript.browser',
Expand All @@ -59,7 +59,7 @@ export class BrowserClient extends BaseClient<BrowserClientOptions> {
],
version: SDK_VERSION,
};
super(options, transport, newTransport);
super(options);
}

/**
Expand Down
1 change: 1 addition & 0 deletions packages/browser/src/exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export {
captureEvent,
captureMessage,
configureScope,
createTransport,
getHubFromCarrier,
getCurrentHub,
Hub,
Expand Down
9 changes: 3 additions & 6 deletions packages/browser/src/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ 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';
import { makeNewFetchTransport, makeNewXHRTransport } from './transports';

export const defaultIntegrations = [
new CoreIntegrations.InboundFilters(),
Expand Down Expand Up @@ -102,17 +101,15 @@ export function init(options: BrowserOptions = {}): void {
if (options.sendClientReports === undefined) {
options.sendClientReports = true;
}
const { transport, newTransport } = setupBrowserTransport(options);

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

initAndBind(BrowserClient, clientOptions, transport, newTransport);
initAndBind(BrowserClient, clientOptions);

if (options.autoSessionTracking) {
startSessionTracking();
Expand Down
2 changes: 0 additions & 2 deletions packages/browser/src/transports/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,3 @@ export { XHRTransport } from './xhr';

export { makeNewFetchTransport } from './new-fetch';
export { makeNewXHRTransport } from './new-xhr';

export { setupBrowserTransport } from './setup';
9 changes: 2 additions & 7 deletions packages/browser/src/transports/new-fetch.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
import {
BaseTransportOptions,
createTransport,
NewTransport,
TransportMakeRequestResponse,
TransportRequest,
} from '@sentry/core';
import { createTransport } from '@sentry/core';
import { BaseTransportOptions, NewTransport, TransportMakeRequestResponse, TransportRequest } from '@sentry/types';

import { FetchImpl, getNativeFetchImplementation } from './utils';

Expand Down
9 changes: 2 additions & 7 deletions packages/browser/src/transports/new-xhr.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
import {
BaseTransportOptions,
createTransport,
NewTransport,
TransportMakeRequestResponse,
TransportRequest,
} from '@sentry/core';
import { createTransport } from '@sentry/core';
import { BaseTransportOptions, NewTransport, TransportMakeRequestResponse, TransportRequest } from '@sentry/types';
import { SyncPromise } from '@sentry/utils';

/**
Expand Down
71 changes: 0 additions & 71 deletions packages/browser/src/transports/setup.ts

This file was deleted.

5 changes: 3 additions & 2 deletions packages/browser/test/unit/helper/browser-client-options.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { NoopTransport } from '@sentry/core';
import { createTransport } from '@sentry/core';
import { resolvedSyncPromise } from '@sentry/utils';

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

export function getDefaultBrowserClientOptions(options: Partial<BrowserClientOptions> = {}): BrowserClientOptions {
return {
integrations: [],
transport: NoopTransport,
transport: () => createTransport({}, _ => resolvedSyncPromise({ statusCode: 200 })),
stackParser: () => [],
...options,
};
Expand Down
24 changes: 12 additions & 12 deletions packages/browser/test/unit/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
wrap,
} from '../../src';
import { getDefaultBrowserClientOptions } from './helper/browser-client-options';
import { SimpleTransport } from './mocks/simpletransport';
import { makeSimpleTransport } from './mocks/simpletransport';

const dsn = 'https://[email protected]/4291';

Expand All @@ -31,7 +31,7 @@ describe('SentryBrowser', () => {
init({
beforeSend,
dsn,
transport: SimpleTransport,
transport: makeSimpleTransport,
});
});

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

beforeEach(() => {
Expand Down Expand Up @@ -150,7 +150,7 @@ describe('SentryBrowser', () => {
},
dsn,
});
getCurrentHub().bindClient(new BrowserClient(options, new SimpleTransport({ dsn })));
getCurrentHub().bindClient(new BrowserClient(options));
captureMessage('test');
});

Expand All @@ -164,7 +164,7 @@ describe('SentryBrowser', () => {
},
dsn,
});
getCurrentHub().bindClient(new BrowserClient(options, new SimpleTransport({ dsn })));
getCurrentHub().bindClient(new BrowserClient(options));
captureEvent({ message: 'event' });
});

Expand All @@ -175,7 +175,7 @@ describe('SentryBrowser', () => {
dsn,
integrations: [],
});
getCurrentHub().bindClient(new BrowserClient(options, new SimpleTransport({ dsn })));
getCurrentHub().bindClient(new BrowserClient(options));

captureMessage('event222');
captureMessage('event222');
Expand All @@ -192,7 +192,7 @@ describe('SentryBrowser', () => {
dsn,
integrations: [new Integrations.InboundFilters({ ignoreErrors: ['capture'] })],
});
getCurrentHub().bindClient(new BrowserClient(options, new SimpleTransport({ dsn })));
getCurrentHub().bindClient(new BrowserClient(options));

captureMessage('capture');

Expand Down Expand Up @@ -244,7 +244,7 @@ describe('SentryBrowser initialization', () => {
it('should set SDK data when Sentry.init() is called', () => {
init({ dsn });

const sdkData = (getCurrentHub().getClient() as any).getTransport()._api.metadata?.sdk;
const sdkData = (getCurrentHub().getClient() as any).getOptions()._metadata.sdk;

expect(sdkData?.name).toBe('sentry.javascript.browser');
expect(sdkData?.packages[0].name).toBe('npm:@sentry/browser');
Expand All @@ -254,9 +254,9 @@ describe('SentryBrowser initialization', () => {

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

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

expect(sdkData.name).toBe('sentry.javascript.browser');
expect(sdkData.packages[0].name).toBe('npm:@sentry/browser');
Expand Down Expand Up @@ -284,7 +284,7 @@ describe('SentryBrowser initialization', () => {
},
});

const sdkData = (getCurrentHub().getClient() as any).getTransport()._api.metadata?.sdk;
const sdkData = (getCurrentHub().getClient() as any).getOptions()._metadata?.sdk;

expect(sdkData.name).toBe('sentry.javascript.angular');
expect(sdkData.packages[0].name).toBe('npm:@sentry/angular');
Expand All @@ -305,7 +305,7 @@ describe('wrap()', () => {
},
dsn,
});
getCurrentHub().bindClient(new BrowserClient(options, new SimpleTransport({ dsn })));
getCurrentHub().bindClient(new BrowserClient(options));

try {
wrap(() => {
Expand Down
7 changes: 3 additions & 4 deletions packages/browser/test/unit/integrations/linkederrors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { createStackParser } from '@sentry/utils';
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 @@ -47,7 +46,7 @@ describe('LinkedErrors', () => {

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

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

const options = getDefaultBrowserClientOptions({ stackParser: parser });
const client = new BrowserClient(options, setupBrowserTransport(options).transport);
const client = new BrowserClient(options);
const originalException = one;
return client.eventFromException(originalException).then(event => {
const result = LinkedErrorsModule._handler(parser, 'cause', 2, event, {
Expand Down
17 changes: 4 additions & 13 deletions packages/browser/test/unit/mocks/simpletransport.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,6 @@
import { eventStatusFromHttpCode, resolvedSyncPromise } from '@sentry/utils';
import { createTransport } from '@sentry/core';
import { resolvedSyncPromise } from '@sentry/utils';

import { Event, Response } from '../../../src';
import { BaseTransport } from '../../../src/transports';

// @ts-ignore It's okay that we're not implementing the `_sendRequest()` method because we don't use it in our tests
export class SimpleTransport extends BaseTransport {
public sendEvent(_: Event): PromiseLike<Response> {
return this._buffer.add(() =>
resolvedSyncPromise({
status: eventStatusFromHttpCode(200),
}),
);
}
export function makeSimpleTransport() {
return createTransport({}, () => resolvedSyncPromise({ statusCode: 200 }));
}
6 changes: 4 additions & 2 deletions packages/browser/test/unit/sdk.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
/* eslint-disable @typescript-eslint/unbound-method */
import { NoopTransport, Scope } from '@sentry/core';
import { Scope } from '@sentry/core';
import { createTransport } from '@sentry/core';
import { MockIntegration } from '@sentry/core/test/lib/sdk.test';
import { Client, Integration } from '@sentry/types';
import { resolvedSyncPromise } from '@sentry/utils';

import { BrowserOptions } from '../../src';
import { init } from '../../src/sdk';
Expand All @@ -13,7 +15,7 @@ const PUBLIC_DSN = 'https://username@domain/123';
function getDefaultBrowserOptions(options: Partial<BrowserOptions> = {}): BrowserOptions {
return {
integrations: [],
transport: NoopTransport,
transport: () => createTransport({}, _ => resolvedSyncPromise({ statusCode: 200 })),
stackParser: () => [],
...options,
};
Expand Down
3 changes: 2 additions & 1 deletion packages/browser/test/unit/transports/base.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ const envelopeEndpoint = 'https://sentry.io/api/42/envelope/?sentry_key=123&sent
// assert on what the class provides and what it leaves to the concrete class to implement
class SimpleTransport extends BaseTransport {}

describe('BaseTransport', () => {
// TODO(v7): Re-enable these tests with client reports
describe.skip('BaseTransport', () => {
describe('Client Reports', () => {
const sendBeaconSpy = jest.fn();
let visibilityState: string;
Expand Down
Loading

0 comments on commit 6267b54

Please sign in to comment.