Skip to content
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

feat: Switch to new transports #4943

Merged
merged 60 commits into from
Apr 25, 2022
Merged
Show file tree
Hide file tree
Changes from 43 commits
Commits
Show all changes
60 commits
Select commit Hold shift + click to select a range
5745c33
feat(core): Introduce seperate client options
AbhiPrasad Apr 12, 2022
fd4dcd8
introduce diff option types
AbhiPrasad Apr 14, 2022
d2e8764
introduce diff browser types:
AbhiPrasad Apr 14, 2022
f1701b4
node client options
AbhiPrasad Apr 14, 2022
b77cd21
switch browser
AbhiPrasad Apr 14, 2022
7735f79
don't do anything with the dsn
AbhiPrasad Apr 14, 2022
523bdf7
dsn type fix
AbhiPrasad Apr 14, 2022
5a64ba8
remove default core options
AbhiPrasad Apr 18, 2022
cd01213
add comments
AbhiPrasad Apr 18, 2022
0d00e20
cast option types
AbhiPrasad Apr 18, 2022
b3cfe41
explicit types
AbhiPrasad Apr 18, 2022
8cb4fac
fix tests
AbhiPrasad Apr 19, 2022
1463745
fix transport types
AbhiPrasad Apr 19, 2022
4e1c76c
round 1 of fixing tests
AbhiPrasad Apr 19, 2022
71fbb8e
tmp fix core `sdk.test.ts`
Lms24 Apr 19, 2022
35a8000
Add tests for integration setup in node init
lforst Apr 19, 2022
a089559
add tests for integration setup in browser init
Lms24 Apr 19, 2022
b2bd018
Fix some eslint errors in node tests
lforst Apr 19, 2022
4fd61b6
Extract `getDefaultNodeClientOptions` into helper module
lforst Apr 19, 2022
c6f8b1e
fix remaning browser unit tests
Lms24 Apr 19, 2022
381e827
fix linter errors
Lms24 Apr 19, 2022
dd383b5
move types to types
AbhiPrasad Apr 19, 2022
45b982f
delete from old place
AbhiPrasad Apr 19, 2022
24936ae
change client types
AbhiPrasad Apr 19, 2022
8644b0b
the rest of the owl
AbhiPrasad Apr 19, 2022
6e50fa8
yarn fix
AbhiPrasad Apr 19, 2022
3dbae89
optional url
AbhiPrasad Apr 19, 2022
00d8ee6
refactor api usage in client
AbhiPrasad Apr 19, 2022
0dfcd24
fix types
AbhiPrasad Apr 19, 2022
d79a586
remove transport from session flusher
AbhiPrasad Apr 19, 2022
32739b4
add session aggregate typing
AbhiPrasad Apr 19, 2022
8ab6bed
tracing transport fix
AbhiPrasad Apr 19, 2022
8852906
types cleanup
AbhiPrasad Apr 19, 2022
bbd53ae
fix bad rebase
AbhiPrasad Apr 19, 2022
ea27746
delete setup tests
AbhiPrasad Apr 19, 2022
173c7f3
fix hub tests
AbhiPrasad Apr 19, 2022
27b0552
fix core tests
AbhiPrasad Apr 19, 2022
c1dccf0
fix node tests
AbhiPrasad Apr 19, 2022
fec5f7c
fix browser tests
AbhiPrasad Apr 19, 2022
ae0b8f5
fix browser integration tests
AbhiPrasad Apr 19, 2022
5c979df
fix node integration tests
AbhiPrasad Apr 20, 2022
b058af1
delete console log
AbhiPrasad Apr 20, 2022
534ced3
Merge branch '7.x' into v7-abhi-use-new-transport
AbhiPrasad Apr 20, 2022
10b1bf7
lint fix
AbhiPrasad Apr 20, 2022
2d299d4
fix node release health tests
AbhiPrasad Apr 20, 2022
7a86050
work toward fixing nextjs tests
AbhiPrasad Apr 20, 2022
3d1a6a4
fix node integration tests
AbhiPrasad Apr 20, 2022
174362f
fix tracing tests
AbhiPrasad Apr 20, 2022
8f0bdbf
PR review
AbhiPrasad Apr 21, 2022
fec138c
comment out test
AbhiPrasad Apr 21, 2022
6d9eb92
tracing fix
AbhiPrasad Apr 21, 2022
73d6c82
Fix nextjs integration tests nocks
lforst Apr 21, 2022
c12a077
Fix copy paste error
lforst Apr 21, 2022
626f261
Fix next integration tests
lforst Apr 21, 2022
29a2c7a
Fix nextjs unit tests
lforst Apr 21, 2022
2f3105a
Fix linter errors
lforst Apr 21, 2022
d895160
Fix browser integration tests by implementing timout function
lforst Apr 21, 2022
e2b1864
Merge branch '7.x' into v7-abhi-use-new-transport
AbhiPrasad Apr 21, 2022
f301815
fix ember tests
AbhiPrasad Apr 21, 2022
ac1db21
only add 1 event processor
AbhiPrasad Apr 21, 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
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
Copy link
Member

Choose a reason for hiding this comment

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

I think this PR might be a good opportuinity to add the transport changes to the migration docs.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah agree, I'll add those.

Copy link
Member Author

Choose a reason for hiding this comment

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

Decided that I'll do this in a follow up PR!


# Upgrading from 6.17.x to 6.18.0
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, stackParserFromOptions } 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
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 @@ -105,17 +104,15 @@ export function init(options: BrowserOptions = {}): void {
if (options.stackParser === undefined) {
options.stackParser = defaultStackParsers;
}
const { transport, newTransport } = setupBrowserTransport(options);

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),
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 { resolvedSyncPromise } from '@sentry/utils';
import { createTransport } from '@sentry/core';

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 { MockIntegration } from '@sentry/core/test/lib/sdk.test';
import { createTransport } from '@sentry/core';
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