-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix: rrweb recorder may throw error when stopping recording after an iframe becomes cross-origin #1695
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
base: master
Are you sure you want to change the base?
fix: rrweb recorder may throw error when stopping recording after an iframe becomes cross-origin #1695
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"rrweb": patch | ||
--- | ||
|
||
fix: rrweb recorder may throw error when stopping recording after an iframe becomes cross-origin |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -557,7 +557,7 @@ | |||||
plugins | ||||||
?.filter((p) => p.observer) | ||||||
?.map((p) => ({ | ||||||
observer: p.observer!, | ||||||
Check warning on line 560 in packages/rrweb/src/record/index.ts
|
||||||
options: p.options, | ||||||
callback: (payload: object) => | ||||||
wrappedEmit({ | ||||||
|
@@ -575,7 +575,7 @@ | |||||
|
||||||
iframeManager.addLoadListener((iframeEl) => { | ||||||
try { | ||||||
handlers.push(observe(iframeEl.contentDocument!)); | ||||||
Check warning on line 578 in packages/rrweb/src/record/index.ts
|
||||||
} catch (error) { | ||||||
// TODO: handle internal error | ||||||
console.warn(error); | ||||||
|
@@ -617,7 +617,25 @@ | |||||
); | ||||||
} | ||||||
return () => { | ||||||
handlers.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 | ||||||
src attribute is changed to a cross-origin URL, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
* 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')) { | ||||||
YunFeng0817 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
console.warn(error); | ||||||
} | ||||||
} | ||||||
}); | ||||||
processedNodeManager.destroy(); | ||||||
recording = false; | ||||||
unregisterErrorHandler(); | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -990,6 +990,21 @@ describe('record', function (this: ISuite) { | |
|
||
await assertSnapshot(ctx.events); | ||
}); | ||
|
||
it('does not throw error when stopping recording after iframe becomes cross-origin', async () => { | ||
await ctx.page.evaluate(async () => { | ||
const { record } = (window as unknown as IWindow).rrweb; | ||
const stopRecord = record({ | ||
emit: (window as unknown as IWindow).emit, | ||
}); | ||
const iframe = document.createElement('iframe'); | ||
document.body.appendChild(iframe); | ||
await new Promise((resolve) => setTimeout(resolve, 1000)); | ||
iframe.src = 'https://www.example.com'; // Change the same origin iframe to a cross origin iframe after it's recorded | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should generate a mutation I believe |
||
await new Promise((resolve) => setTimeout(resolve, 1000)); | ||
YunFeng0817 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
stopRecord?.(); | ||
}); | ||
}); | ||
}); | ||
|
||
describe('record iframes', function (this: ISuite) { | ||
|
Uh oh!
There was an error while loading. Please reload this page.