-
Notifications
You must be signed in to change notification settings - Fork 142
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
🐛 [RUM-6411] RUM should not crash with puppeteer injection #3153
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
|
@@ -102,7 +102,9 @@ export function createScrollValuesObservable( | |||
|
|||
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
@@ -228,3 +230,33 @@ async function tearDownTest({ intakeRegistry }: TestContext) { | |||
}) | |||
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
evaluateOnNewDocument
because 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.