Skip to content

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.

@edjmao edjmao requested a review from kamilogorek as a code owner December 17, 2019 23:30
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 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.

4 participants