Skip to content
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Merge branch 'master' into meg-fix-mutation-observer-leak
  • Loading branch information
megboehlert authored Oct 27, 2025
commit 7b2416ad0b863d1c70e4ac375135c4818171801e
20 changes: 19 additions & 1 deletion packages/rrweb/src/record/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@
plugins
?.filter((p) => p.observer)
?.map((p) => ({
observer: p.observer!,

Check warning on line 565 in packages/rrweb/src/record/index.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/rrweb/src/record/index.ts#L565

[@typescript-eslint/no-non-null-assertion] Forbidden non-null assertion.
options: p.options,
callback: (payload: object) =>
wrappedEmit({
Expand All @@ -580,7 +580,7 @@

iframeManager.addLoadListener((iframeEl) => {
try {
iframeHandlersMap.set(iframeEl, observe(iframeEl.contentDocument!));

Check warning on line 583 in packages/rrweb/src/record/index.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/rrweb/src/record/index.ts#L583

[@typescript-eslint/no-non-null-assertion] Forbidden non-null assertion.
Copy link

Copilot AI Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The handler is added to iframeHandlersMap but not to the handlers array. This creates an inconsistency where the cleanup in the return function iterates both collections separately. Consider adding the handler to both collections or documenting why this handler should only be in the map.

Suggested change
iframeHandlersMap.set(iframeEl, observe(iframeEl.contentDocument!));
const handler = observe(iframeEl.contentDocument!);
iframeHandlersMap.set(iframeEl, handler);
handlers.push(handler);

Copilot uses AI. Check for mistakes.
} catch (error) {
// TODO: handle internal error
console.warn(error);
Expand Down Expand Up @@ -631,8 +631,26 @@
);
}
return () => {
handlers.forEach((h) => h());
iframeHandlersMap.forEach((h) => h());
handlers.forEach((handler) => {
try {
handler();
} catch (error) {
const msg = String(error).toLowerCase();
/**
* https://github.com/rrweb-io/rrweb/pull/1695
* This error can occur in a known scenario:
* If an iframe is initially same-origin and observed, but later its
location is changed in an opaque way to a cross-origin URL (perhaps within the iframe via its `document.location` or a redirect)
* attempting to execute the handler in the stop record function will
throw a "cannot access cross-origin frame" error.
* This error is expected and can be safely ignored.
*/
if (!msg.includes('cross-origin')) {
console.warn(error);
}
}
});
processedNodeManager.destroy();
recording = false;
unregisterErrorHandler();
Expand Down
Loading
You are viewing a condensed version of this merge commit. You can view the full changes here.