-
Notifications
You must be signed in to change notification settings - Fork 162
🐛 [RUM-6411] RUM should not crash with puppeteer injection #3153
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3153 +/- ##
==========================================
+ Coverage 93.43% 93.45% +0.01%
==========================================
Files 280 280
Lines 7679 7680 +1
Branches 1719 1720 +1
==========================================
+ Hits 7175 7177 +2
+ Misses 504 503 -1 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
Bundles Sizes Evolution
🚀 CPU Performance
🧠 Memory Performance
|
| const observerTarget = document.scrollingElement || document.documentElement | ||
| const resizeObserver = new ResizeObserver(monitor(throttledNotify.throttled)) | ||
| resizeObserver.observe(observerTarget) | ||
| if (observerTarget instanceof Element) { |
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.
💬 suggestion:
| if (observerTarget instanceof Element) { | |
| if (observerTarget) { |
test/e2e/lib/framework/createTest.ts
Outdated
| await deleteAllCookies() | ||
| } | ||
|
|
||
| export async function injectRumWithPuppeteer() { |
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.
💬 suggestion: Move this function out of this module as it has little to do with the createTest helper. Since it is only one test case, you could even inline the code inside of the test
15e724f to
ef5afb3
Compare
ef5afb3 to
9f78ec5
Compare
Motivation
RUM would crash when injected with puppeteer
evaluateOnNewDocumentbecause resizeObserver is observing on non-document targets.Changes
Add check for observe target
Add e2e test for inject rum with puppeteer
Testing
I have gone over the contributing documentation.