Skip to content

ref: Rework XHR wrapping logic to make sure it always triggers#2438

Merged
kamilogorek merged 2 commits intomasterfrom
apm-xhr-warning
Feb 19, 2020
Merged

ref: Rework XHR wrapping logic to make sure it always triggers#2438
kamilogorek merged 2 commits intomasterfrom
apm-xhr-warning

Conversation

@kamilogorek
Copy link
Contributor

No description provided.

@kamilogorek kamilogorek requested a review from HazAT February 18, 2020 09:33
@getsentry-bot
Copy link
Contributor

getsentry-bot commented Feb 18, 2020

Warnings
⚠️ Please add a changelog entry for your changes.
Messages
📖 ✅ TSLint passed
📖

@sentry/browser bundle gzip'ed minified size: (ES5: 16.6875 kB) (ES6: 15.707 kB)

Generated by 🚫 dangerJS against 6dc4ffd

@kamilogorek kamilogorek force-pushed the apm-xhr-warning branch 2 times, most recently from 54c2dc9 to 1b21167 Compare February 19, 2020 09:28
@kamilogorek kamilogorek changed the title ref: Emit warning when xhr onreadystatechange is missing ref: Rework XHR wrapping logic to make sure it always triggers Feb 19, 2020
@kamilogorek kamilogorek requested a review from HazAT February 19, 2020 10:01
@kamilogorek kamilogorek force-pushed the apm-xhr-warning branch 4 times, most recently from 64235d4 to 6dc4ffd Compare February 19, 2020 11:21
assert.equal(summary.breadcrumbs[0].type, "http");
assert.equal(summary.breadcrumbs[0].category, "xhr");
assert.equal(summary.breadcrumbs[0].data.method, "GET");
assert.typeOf(summary.breadcrumbs[0].timestamp, "number");
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we check for endTimestamp to make sure we actually got what we wanted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's not registered handler for this in breadcrumbs without an amp package :3
I'll see if we can go around this somehow.

@kamilogorek kamilogorek merged commit 5d340fb into master Feb 19, 2020
@kamilogorek kamilogorek deleted the apm-xhr-warning branch February 19, 2020 14:05
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.

3 participants