-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
ref(client): Inject Transports into Client #4921
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
Changes from 16 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
5232b8d
add first PoC implementation for Transport injection
Lms24 a2a7277
fix browser unit tests
Lms24 bee1c78
fix core unit tests
Lms24 5eb327f
fix node unit tests
Lms24 a1fec8a
fix tracing tests
Lms24 798f921
fix more node unit tests
Lms24 6d8120d
fix more browser unit tests
Lms24 3a859a4
fix more tracing unit tests
Lms24 22c0e7c
fix linter errors
Lms24 4b951a7
fix more core unit tests
Lms24 f1bce90
fix linter error
Lms24 71c7058
add setupBrowserTransport tests
Lms24 9c0094a
fix linter errors
Lms24 827b8ca
add basic setupNodeTransport tests
Lms24 a555fad
improve tests
Lms24 86cfd73
delete `setupTransport()`, cleanup TODOs, add doc
Lms24 3afc9ee
apply review suggestions
Lms24 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
import { | ||
BaseTransportOptions, | ||
getEnvelopeEndpointWithUrlEncodedAuth, | ||
initAPIDetails, | ||
NewTransport, | ||
NoopTransport, | ||
} from '@sentry/core'; | ||
import { Transport, TransportOptions } from '@sentry/types'; | ||
import { supportsFetch } from '@sentry/utils'; | ||
|
||
import { BrowserOptions } from '../client'; | ||
import { FetchTransport } from './fetch'; | ||
import { makeNewFetchTransport } from './new-fetch'; | ||
import { makeNewXHRTransport } from './new-xhr'; | ||
import { XHRTransport } from './xhr'; | ||
|
||
export interface BrowserTransportOptions extends BaseTransportOptions { | ||
// options to pass into fetch request | ||
fetchParams: Record<string, string>; | ||
headers?: Record<string, string>; | ||
sendClientReports?: boolean; | ||
} | ||
|
||
/** | ||
* Sets up Browser transports based on the passed `options`. If available, the returned | ||
* transport will use the fetch API. In case fetch is not supported, an XMLHttpRequest | ||
* based transport is created. | ||
* | ||
* @returns an object currently still containing both, the old `Transport` and | ||
* `NewTransport` which will eventually replace `Transport`. Once this is replaced, | ||
* 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 } { | ||
if (!options.dsn) { | ||
// We return the noop transport here in case there is no Dsn. | ||
return { transport: new NoopTransport() }; | ||
} | ||
|
||
const transportOptions: TransportOptions = { | ||
...options.transportOptions, | ||
dsn: options.dsn, | ||
tunnel: options.tunnel, | ||
sendClientReports: options.sendClientReports, | ||
_metadata: options._metadata, | ||
}; | ||
|
||
const api = initAPIDetails(transportOptions.dsn, transportOptions._metadata, transportOptions.tunnel); | ||
const url = getEnvelopeEndpointWithUrlEncodedAuth(api.dsn, api.tunnel); | ||
|
||
if (options.transport) { | ||
return { transport: new options.transport(transportOptions) }; | ||
} | ||
|
||
if (supportsFetch()) { | ||
const requestOptions: RequestInit = { ...transportOptions.fetchParameters }; | ||
const newTransport = makeNewFetchTransport({ requestOptions, url }); | ||
const fetchTransport = new FetchTransport(transportOptions); | ||
return { transport: fetchTransport, newTransport }; | ||
} | ||
|
||
const newTransport = makeNewXHRTransport({ | ||
url, | ||
headers: transportOptions.headers, | ||
}); | ||
const transport = new XHRTransport(transportOptions); | ||
return { transport, newTransport }; | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -75,7 +75,7 @@ describe('SentryBrowser', () => { | |
describe('showReportDialog', () => { | ||
describe('user', () => { | ||
const EX_USER = { email: '[email protected]' }; | ||
const client = new BrowserClient({ dsn }); | ||
const client = new BrowserClient({ dsn }, new SimpleTransport({ dsn })); | ||
const reportDialogSpy = jest.spyOn(client, 'showReportDialog'); | ||
|
||
beforeEach(() => { | ||
|
@@ -140,42 +140,51 @@ describe('SentryBrowser', () => { | |
|
||
it('should capture a message', done => { | ||
getCurrentHub().bindClient( | ||
new BrowserClient({ | ||
beforeSend: (event: Event): Event | null => { | ||
expect(event.message).toBe('test'); | ||
expect(event.exception).toBeUndefined(); | ||
done(); | ||
return event; | ||
new BrowserClient( | ||
{ | ||
beforeSend: (event: Event): Event | null => { | ||
expect(event.message).toBe('test'); | ||
expect(event.exception).toBeUndefined(); | ||
done(); | ||
return event; | ||
}, | ||
dsn, | ||
}, | ||
dsn, | ||
}), | ||
new SimpleTransport({ dsn }), | ||
), | ||
); | ||
captureMessage('test'); | ||
}); | ||
|
||
it('should capture an event', done => { | ||
getCurrentHub().bindClient( | ||
new BrowserClient({ | ||
beforeSend: (event: Event): Event | null => { | ||
expect(event.message).toBe('event'); | ||
expect(event.exception).toBeUndefined(); | ||
done(); | ||
return event; | ||
new BrowserClient( | ||
{ | ||
beforeSend: (event: Event): Event | null => { | ||
expect(event.message).toBe('event'); | ||
expect(event.exception).toBeUndefined(); | ||
done(); | ||
return event; | ||
}, | ||
dsn, | ||
}, | ||
dsn, | ||
}), | ||
new SimpleTransport({ dsn }), | ||
), | ||
); | ||
captureEvent({ message: 'event' }); | ||
}); | ||
|
||
it('should not dedupe an event on bound client', async () => { | ||
const localBeforeSend = jest.fn(); | ||
getCurrentHub().bindClient( | ||
new BrowserClient({ | ||
beforeSend: localBeforeSend, | ||
dsn, | ||
integrations: [], | ||
}), | ||
new BrowserClient( | ||
{ | ||
beforeSend: localBeforeSend, | ||
dsn, | ||
integrations: [], | ||
}, | ||
new SimpleTransport({ dsn }), | ||
), | ||
); | ||
|
||
captureMessage('event222'); | ||
|
@@ -189,11 +198,14 @@ describe('SentryBrowser', () => { | |
it('should use inboundfilter rules of bound client', async () => { | ||
const localBeforeSend = jest.fn(); | ||
getCurrentHub().bindClient( | ||
new BrowserClient({ | ||
beforeSend: localBeforeSend, | ||
dsn, | ||
integrations: [new Integrations.InboundFilters({ ignoreErrors: ['capture'] })], | ||
}), | ||
new BrowserClient( | ||
{ | ||
beforeSend: localBeforeSend, | ||
dsn, | ||
integrations: [new Integrations.InboundFilters({ ignoreErrors: ['capture'] })], | ||
}, | ||
new SimpleTransport({ dsn }), | ||
), | ||
); | ||
|
||
captureMessage('capture'); | ||
|
@@ -248,16 +260,16 @@ describe('SentryBrowser initialization', () => { | |
|
||
const sdkData = (getCurrentHub().getClient() as any).getTransport()._api.metadata?.sdk; | ||
|
||
expect(sdkData.name).toBe('sentry.javascript.browser'); | ||
expect(sdkData.packages[0].name).toBe('npm:@sentry/browser'); | ||
expect(sdkData.packages[0].version).toBe(SDK_VERSION); | ||
expect(sdkData.version).toBe(SDK_VERSION); | ||
expect(sdkData?.name).toBe('sentry.javascript.browser'); | ||
expect(sdkData?.packages[0].name).toBe('npm:@sentry/browser'); | ||
expect(sdkData?.packages[0].version).toBe(SDK_VERSION); | ||
expect(sdkData?.version).toBe(SDK_VERSION); | ||
}); | ||
|
||
it('should set SDK data when instantiating a client directly', () => { | ||
const client = new BrowserClient({ dsn }); | ||
const client = new BrowserClient({ dsn }, new SimpleTransport({ dsn })); | ||
|
||
const sdkData = (client as any).getTransport()._api.metadata?.sdk; | ||
const sdkData = (client.getTransport() as any)._api.metadata?.sdk; | ||
|
||
expect(sdkData.name).toBe('sentry.javascript.browser'); | ||
expect(sdkData.packages[0].name).toBe('npm:@sentry/browser'); | ||
|
@@ -298,15 +310,18 @@ describe('SentryBrowser initialization', () => { | |
describe('wrap()', () => { | ||
it('should wrap and call function while capturing error', done => { | ||
getCurrentHub().bindClient( | ||
new BrowserClient({ | ||
beforeSend: (event: Event): Event | null => { | ||
expect(event.exception!.values![0].type).toBe('TypeError'); | ||
expect(event.exception!.values![0].value).toBe('mkey'); | ||
done(); | ||
return null; | ||
new BrowserClient( | ||
{ | ||
beforeSend: (event: Event): Event | null => { | ||
expect(event.exception!.values![0].type).toBe('TypeError'); | ||
expect(event.exception!.values![0].value).toBe('mkey'); | ||
done(); | ||
return null; | ||
}, | ||
dsn, | ||
}, | ||
dsn, | ||
}), | ||
new SimpleTransport({ dsn }), | ||
), | ||
); | ||
|
||
try { | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.