fix(rrweb): Do not call setAttribute on TEXT nodes #253
Conversation
Not sure yet how this is happening during recording, but prevent player from calling `setAttribute` on TEXT nodes (which does not have that method). This additionally prevents the player from freezing while trying to process these invalid mutations.
billyvg
force-pushed
the
billy/fix-set-attribute-text-node
branch
from
c8e0fc8 to
aba4b9b
Compare
billyvg
changed the title
|
@sentry review |
![seer-by-sentry[bot]](https://avatars.githubusercontent.com/in/801464?s=60&v=4){kind=small}
packages/rrweb/src/replay/index.ts
Outdated
There was a problem hiding this comment.
The guard condition only checks for Text nodes (nodeType !== 3), but other node types such as COMMENT_NODE (8), PROCESSING_INSTRUCTION_NODE (7), or DOCUMENT_FRAGMENT_NODE (11) also lack the setAttribute method. While the immediate issue may only manifest with Text nodes, consider using a more robust check like instanceof Element or typeof target.setAttribute === 'function' to make the code more defensive against future edge cases and document types that might appear during recording.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
I don't think we record any of those types, so this should suffice
| // prevents calling `setAttribute` on Text nodes (which does not | ||
| // have `setAttribute` method). | ||
| if (target.nodeType !== 3) { | ||
| (target as Element | RRElement).setAttribute( |
There was a problem hiding this comment.
The Sentry suggestion to check typeof target.setAttribute === 'function' seems reasonable instead of going with nodeType=3 no?
There was a problem hiding this comment.
Yeah, I want it to be more specific since this shouldn't happen at all. Something seems to be wrong during recording where we are getting the wrong mutations (or the wrong element references for the mutation), so ideally this is just temporary assuming we can repro and fix the recording side.
Not sure yet how this is happening during recording, but prevent player from calling
setAttributeon TEXT nodes (which does not have that method). This additionally prevents the player from freezing while trying to process these invalid mutations.