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

Set request headers to ones provided in transportOpts in XHR transport #2363

Merged
merged 5 commits into from
Dec 20, 2019
Merged

Set request headers to ones provided in transportOpts in XHR transport #2363

merged 5 commits into from
Dec 20, 2019

Conversation

edjmao
Copy link
Contributor

@edjmao edjmao commented Dec 17, 2019

Our instance of Sentry is behind a proxy that requires an OAuth bearer token to be passed in the Authorization header. The default XHR transport does not offer a way of easily setting any headers. In my own project, I copied the XHR transport and added a line to set requests. I thought it might be nice to have in the default XHR transport as well.

Copy link
Member

@HazAT HazAT left a comment

Choose a reason for hiding this comment

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

Seems like a reasonable addition 👍
Can you please fix the lint errors on CI?

@edjmao
Copy link
Contributor Author

edjmao commented Dec 18, 2019

Seems like a reasonable addition +1
Can you please fix the lint errors on CI?

Will do. Probably should have run yarn, but I'll get to that this afternoon.

edjmao and others added 2 commits December 18, 2019 14:58
Co-Authored-By: Sijawusz Pur Rahnama <sija@sija.pl>
@edjmao edjmao requested a review from HazAT December 18, 2019 21:19
Copy link
Member

@HazAT HazAT left a comment

Choose a reason for hiding this comment

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

One thing I noticed, can you please also add this to the FetchTransport to make it feature complete? We use fetch by default if the browser supports it and I assume it will also be helpful there.

@edjmao
Copy link
Contributor Author

edjmao commented Dec 19, 2019

One thing I noticed, can you please also add this to the FetchTransport to make it feature complete? We use fetch by default if the browser supports it and I assume it will also be helpful there.

No problem.

@edjmao edjmao requested a review from HazAT December 19, 2019 23:12
Copy link
Member

@HazAT HazAT left a comment

Choose a reason for hiding this comment

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

Awesome job, thank you! 🥇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants