-
Notifications
You must be signed in to change notification settings - Fork 57
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
[BUG] Fails with "Execution context was destroyed" when iframes are removed #64
Comments
Thanks for the detailed report @mccahan! It looks like the error likely stems from getting the browser frames (which includes iframes) only once, instead of per time iteration on the virtual timeline. I think moving Please let me know if that fixes the issue. Overall, the pull request seems like a good addition regardless (like checking whether a frames is detached, and logging particular URLs of embedded frames), but if the above fixes the issue, I'd prefer to omit the error suppression, since I don't know which other cases can throw that error. In the longer term, it's probably a good idea to allow for the user to create a custom error handler that can suppress or rethrow errors if need be. |
Following up on this, getting the browser frames every capture doesn't seem to work. I'll look into this further. |
Appreciate the quick review! Yeah, confirming that it still returns 2 browser frames (the main frame + the iframe) for 0ms, 16.6ms, and 33.3ms even though the iframe gets removed at 20ms. However, if I do Going forward, it:
Reversing the frame list and running them in the opposite order here overrides/animates the iframe first and then runs the JS in the main frame that removes the iframe and detaches it, so no error. I'm not sure whether just Adding the |
Thanks for the speedy reply! I think you're pretty much correct in your understanding, and here's some additional information and breakdown of what I believe is happening: The ErrorThe error itself seems to be thrown from the frame's ExecutionContext which arises from a response to an evaluate message sent to the Chrome DevTools Protocol. When timesnap iterates through frames, for efficiency's sake, it doesn't currently wait for the response from puppeteer (and Chrome DevTools Protocol). Since Chrome DevTools Protocols doesn't seem to run immediately, commands to all frames are sent in order, all at once. Chromium then runs those commands in order (and not in parallel as I'd thought), first removing the iframe, and then trying to run the second command, and replying with an error message now that the iframe is removed. That error message is interpreted and then thrown as an error by Puppeteer. FixesAs you note, reversing the iteration through the frames seems to solve the issue. After all, reversing the list would process child frames before their parents, and generally child frames do not remove/navigate their parents or siblings (though I think it's still possible via Alternatively, another fix for this could be to change the iteration to something awaitable (i.e. changing the Catching and logging this error might be also fine, since it seems like it only happens if an execution context is destroyed, which seems to happen only on navigation/removal. I'll mull over the possibilities. |
Hi @mccahan, for the fix, I've decided to await each frame evaluation and check to see whether a frame is detached before evaluating, and is reflected in the commits today. It seems to solve the issue in the reproducible example. Could you try it out to see if you run into this error for your use case? You may need to update/reinstall dependencies if you get some new errors, since the more recent commits use the newest version of timeweb. |
That solved it (elegantly!) for my sample page, which I thought was the least common denominator replicable case, but it turns out it didn't fix it for the production page I'm testing against. I'm running against commit 3d16a10, with a fresh I'm not sure why yet, when I have an To reproduce, I've got this file ( <html>
<body>
<img src="test-image.png">
<script src="https://cdnjs.cloudflare.com/ajax/libs/html2canvas/1.4.1/html2canvas.min.js"></script>
<script>
html2canvas(document.body, { removeContainer: true }).then(canvas => {
document.body.appendChild(canvas)
})
</script>
</body>
</html> Running:
Output looks like:
There's a configuration parameter in that HTML file,
I do know that html2canvas adds images to the container using a Promise so they load asynchronously, which is the only hunch I've got at the moment for why it might feel like it's getting the execution context rug pulled out from underneath it, but if I try/catch the exception it works as intended (correctly screenshots the image into a canvas and appends it to the document) on the next frame. Sorry, I thought I'd boiled it down to the minimum replicable test case, since it worked with my version that just catches that exception and moves on. |
Describe the bug
I have a page that uses html2canvas to "screenshot" part of the page to use as a texture with ThreeJS for a particular effect. When html2canvas runs, it creates an iframe with a copy of the page, takes its screenshot, and then destroys the iframe. Running timecut and timesnap on a page that removes an iframe results in a fatal error as it attempts to execute the time overwriting on an invalid execution context:
On the first captured frame after the iframe is removed, Puppeteer throws the "Execution context was destroyed" exception message. If we catch that exception, a captured frame or two later Puppeteer will throw a different exception that the (browser) frame has been detached:
I do have my own version of a fix at https://github.com/mccahan/timesnap/pull/1/files if you'd like me to Pull Request it - I didn't want to assume where you might want to catch the exception.
Checking the exception message for the string "Execution context was destroyed" feels flimsy, but it mirrors the check that Puppeteer uses internally. I was unable to find a way to determine ahead of time whether the context was destroyed, the only way I could find to test for it is to try and catch an exception afterwards.
I also added in a check to skip processing on (browser) frames that are marked as detached. This negates the second fatal exception.
To Reproduce
remove-iframe.html
with the contents from the Attachments section belownode cli.js remove-iframe.html
Expected behavior
Expected it to gracefully handle (intentional) removals of iframes, continuing to capture.
Attachments and Screenshots
Simplest sample recreation page:
Desktop (please complete the following information):
Additional context
I hit the same issue in timecut, but it seems this is the appropriate repo to address it?
The text was updated successfully, but these errors were encountered: