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: Give access to XHR requests body in breadcrumb hint #2904

Merged
merged 1 commit into from
Sep 16, 2020

Conversation

kamilogorek
Copy link
Contributor

Closes #2166

@kamilogorek kamilogorek requested a review from a team September 15, 2020 13:57
@github-actions
Copy link
Contributor

size-limit report

Path Size
@sentry/browser - CDN Bundle (gzipped) 17.8 KB (+0.6% 🔺)
@sentry/browser - Webpack 18.64 KB (+0.66% 🔺)
@sentry/react - Webpack 18.64 KB (+0.66% 🔺)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped) 23.58 KB (+0.49% 🔺)

@@ -220,6 +223,9 @@ function instrumentXHR(): void {
return;
}

// Poor man implementation of ES6 `Map` by tracking and keeping in sync key and value separately.
Copy link
Contributor

@rhcarvalho rhcarvalho Sep 15, 2020

Choose a reason for hiding this comment

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

Could concurrent requests get this "map" out of sync? I think not, no pre-emption.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nah, it's not Go 😅

@walkerdb
Copy link

Would it be possible to make this configurable? Including XRH request bodies at my org would be a no-go since they often will contain PHI that we can't log. We'd basically have to turn off breadcrumbs entirely.

@kamilogorek kamilogorek merged commit dcba3fb into master Sep 16, 2020
@kamilogorek kamilogorek deleted the xhr-body branch September 16, 2020 10:08
@kamilogorek
Copy link
Contributor Author

Great catch @walkerdb, fixed in #2911

@walkerdb
Copy link

great -- thanks @kamilogorek !

lobsterkatie added a commit that referenced this pull request Jan 18, 2022

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
This simplifies the implementation of XHR request body harvesting for breadcrumbs, introduced in #2904. Instead of caching the data and grabbing it at a later point, this grabs it as soon as it's available, in the xhr `send` method.
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.

breadcrumbCallback - allow to capture request headers/payload/etc
3 participants