fix: guard autocapture DOM walks against null parentNode#3479
Draft
posthog[bot] wants to merge 2 commits intomainfrom
Draft
fix: guard autocapture DOM walks against null parentNode#3479posthog[bot] wants to merge 2 commits intomainfrom
posthog[bot] wants to merge 2 commits intomainfrom
Conversation
Autocapture walks the DOM via parentNode in three hot paths that run on every click/submit/change: autocapturePropertiesForElement, getElementAndParentsForElement, and shouldCaptureElement. Each cast parentNode/host to Element without null-checking, so a detached node, a text node whose parent is null, or a shadow-root host that has been removed caused the next .parentNode read to throw "Cannot read properties of null (reading 'parentNode')". The throw was swallowed by the top-level try/catch in _addDomEventHandlers, dropping the autocaptured event. Replace the unsafe casts with the existing getParentElement helper so iteration stops when the parent is not an element, null-check the shadow-root host before assigning, and reject non-element parents when normalizing a text-node target. Generated-By: PostHog Code Task-Id: 13e2c89a-2542-44d1-b2a5-9401a83d3dcd
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 No Changeset FoundThis PR doesn't include a changeset. A changeset is required to release a new version. How to add a changesetRun this command and follow the prompts: pnpm changesetRemember: Never use |
The previous `(parent as ShadowRoot).host` cast tripped TS7022 in the build typecheck under target ES5 + lib dom because TypeScript could not resolve `host` against `ShadowRoot` in this configuration. Pin the local with an explicit `Element | null` annotation and read via `(parent as any).host` to match the original codebase style. Generated-By: PostHog Code Task-Id: 13e2c89a-2542-44d1-b2a5-9401a83d3dcd
Contributor
Contributor
|
Size Change: +494 B (+0.01%) Total Size: 6.95 MB
ℹ️ View Unchanged
|
Contributor
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A
TypeError: Cannot read properties of null (reading 'parentNode')has been recurring in autocapture across multiple customer pages. The autocapture DOM walks unsafely castparentNode(and the shadow-roothost) toElementand immediately re-read.parentNodeon the next iteration. When the click target is a detached node, a text node whose parent is null, or a shadow-root whose host has been removed, the next read throws. The exception is swallowed by the top-level try/catch in_addDomEventHandlers, so the broader SDK keeps running but the autocaptured event is dropped.Changes
autocapturePropertiesForElement(autocapture.ts): replaceparentNode as Elementcast with the existinggetParentElementhelper; null-check the shadow-roothostbefore assigning.getElementAndParentsForElement(autocapture-utils.ts): same null-check on the shadow-roothost; trackcurElasElement | nullso the loop terminates safely when traversal can't continue.shouldCaptureElement(autocapture-utils.ts): replace theparentNode as Elementfor-loop with a while-loop that walks viagetParentElement, so iteration stops when the parent isn't an element instead of forcing an unsafe cast._captureEventtext-node normalization (autocapture.ts): reject non-element parents when defeating the Safari text-node bug, sotargetis null instead of a forcibly-castDocument/DocumentFragment.Test plan
shouldCaptureElementdoes not throw on a fully detached element.shouldCaptureElementdoes not throw when an ancestor's parent is not an element (detached subtree)._captureEventdoes not throw when the click target is a text node with no parent._captureEventdoes not throw when the click target is a detached element._captureEventdoes not throw when a shadow root has no host.Created with PostHog Code