Session Replay keeps observing blocked iframes, retaining their documents until recording stops

Open
#2,119 1 comment 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 2 days

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
58/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
electron, javascript, typescript

Research direction

Start in packages/rrweb-snapshot/src/snapshot.ts, where serializeNodeWithId reads needBlock and then deletes it (around lines 1091-1093), and the tagName === 'iframe' branch that calls onceIframeLoaded (around lines 1201-1204). Then check the load listener in packages/rrweb/src/record/index.ts (around lines 593-595) that pushes an observer onto handlers. Done means a blocked iframe with srcdoc emits no isAttachIframe event and is not observed, while an unblocked one still does.

Written by the indexing model from the issue text.

Description

bug customer-reported

Packages: @amplitude/session-replay-browser 1.30.4, @amplitude/plugin-session-replay-browser 1.25.5, @amplitude/rrweb-record 2.0.0-alpha.34. The bug is still present in @amplitude/rrweb-record 2.1.5 and on amplitude/rrweb master (06a22bd).

Summary

An iframe that matches privacyConfig.blockSelector is still observed by the recorder. rrweb serializes the blocked iframe as a placeholder, but it still waits for the iframe to load, serializes the iframe's document, and attaches a mutation observer to it.

The observer is pushed onto the recorder's handlers array and is only released when recording stops. Every blocked same-origin iframe therefore keeps its document and window alive after it is removed from the DOM.

In our Electron desktop app, the renderer stays open for many hours. We render each email message in an <iframe srcdoc sandbox="allow-same-origin">. With Session Replay active, detached iframe contexts accumulate (536 and 778 in two crash reports), and the renderer crashes with OutOfMemoryError: Renderer reached heap limit.

Blocking the iframes does not help today, because the blocked iframe is still observed. Blocked iframe content is also still serialized, which is unexpected for a privacy block.

Where it happens

packages/rrweb-snapshot/src/snapshot.ts, in serializeNodeWithId:

  1. Line 1091–1093: needBlock is read for recordChild, then deleted from the serialized node.

  2. Line 1201–1204: the iframe branch only checks tagName === 'iframe', so it runs for blocked iframes too:

    if (
      serializedNode.type === NodeType.Element &&
      serializedNode.tagName === 'iframe'
    ) {
      onceIframeLoaded(n as HTMLIFrameElement, () => {
        // serializes iframeDoc and calls onIframeLoad(...)
    
  3. packages/rrweb/src/record/index.ts line 593–595: onIframeLoad reaches the load listener, which retains an observer per iframe document:

    iframeManager.addLoadListener((iframeEl) => {
      try {
        handlers.push(observe(iframeEl.contentDocument!));
    

Permalinks:

Steps to reproduce

import { record } from "@amplitude/rrweb-record";

const events = [];
const stop = record({
  emit: (event) => events.push(event),
  blockSelector: "iframe[srcdoc]",
});

const iframe = document.createElement("iframe");
iframe.srcdoc = "<p>Email body</p>";
document.body.append(iframe);

// After the iframe loads:
// events contains a mutation with `isAttachIframe: true` for the blocked iframe,
// and the iframe document is serialized and observed.

Expected

A blocked iframe is recorded as a placeholder only. Its document is not serialized, not observed, and not retained.

Actual

The blocked iframe's document is serialized (isAttachIframe: true event) and observed. The observer keeps the iframe document and window in memory until stop() is called.

Suggested fix

Skip the iframe branch for blocked elements. Sentry's rrweb fork already does this (serializedNode.tagName === 'iframe' && !serializedNode.needBlock). Because needBlock is deleted earlier in this fork, either keep it in a local variable before the delete, or check the element again:

   if (
     serializedNode.type === NodeType.Element &&
-    serializedNode.tagName === 'iframe'
+    serializedNode.tagName === 'iframe' &&
+    !_isBlockedElement(n as HTMLElement, blockClass, blockSelector)
   ) {

We apply this change locally with patch-package against the built dist/rrweb-record.js and dist/rrweb-record.cjs. With it, a blocked srcdoc iframe produces no isAttachIframe event, and an unblocked one still does.

Related, not covered by the fix above

For iframes that are not blocked, the observer pushed in addLoadListener is never released when the iframe is removed from the DOM. Long-running sessions that create and remove many same-origin iframes still grow memory with each one. Disconnecting the iframe's observer when the iframe node is removed would address that.

Dominant language
TypeScript
Stars
181
Forks
66
Avg merge
2d 21h
Merged PRs (30d)
41

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from amplitude/Amplitude-TypeScript

All issues in amplitude/Amplitude-TypeScript

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.