-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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(core): Add new transports to base backend #4752
Conversation
Simplify new transport send by grabbing the envelope category from the envelope instead of passing it in explicitly.
this._options._experiments && | ||
this._options._experiments.newTransport | ||
) { | ||
const api = initAPIDetails(this._options.dsn, this._options._metadata, this._options.tunnel); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's annoying to do this, but considering that the client has access to API, won't be needed in the future!
3f8e5b9
to
ea233df
Compare
ea233df
to
35b0399
Compare
Adds new transports to base backend in core. For now, they are gated behind `options._experiments.newTransport = true`. The next step is to add new transports for fetch, xhr (browser) as well as http, https (node).
35b0399
to
2326bb9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Only comment I would have had was on that extra _setupNewTransport
field but you caught it on your own :)
Nvm base changed. Looking at it some more.
size-limit report
|
Part of #4660
Adds new transports to base backend in core. For now, they are gated behind
options._experiments.newTransport = true
. The reason this is gated is because client reports does not work with the new transports.The next step is to add new transports for fetch, xhr (browser) as well as http, https (node). We can do this in any order!
This is pretty hacky code, but considering that we are going to remove a lot of this in v7 anyway, I decided to keep this as is. Also no tests because we are moving fast!
Resolves https://getsentry.atlassian.net/browse/WEB-588