Substantial amount of random errors snapshotting static pages

[mitar]

See this CI run. I see many errors like:

[percy] Encountered an error for page: http://localhost:40611/post/146314072245/do-publicly-owned-planned-economies-work/index.html
[percy] TypeError: Cannot read property 'outerHTML' of null
    at serializeDOM (<anonymous>:206:33)
    at serializeFrames (<anonymous>:87:28)
    at serializeDOM (<anonymous>:187:7)
    at serializeFrames (<anonymous>:87:28)
    at Object.serializeDOM (<anonymous>:187:7)
    at <anonymous>:5:35
    at withPercyHelpers (<anonymous>:9:14)

I am surprised because all pages are more or less the same (blog posts) so there is not really a reason that one would fail and another one not.

Is there any way to get those errors down?

[wwilsman]

Hey @mitar! I debugged this and it seems related to some nested iframes that are accessible for serialization, but don't actually contain expected document elements. Essentially, iframe.contentDocument exists, but document.documentElement does not for one of the iframes on some of your pages. From my brief research, this can only be possible if the document is accessed before the HTML element for the document have been rendered, or if the document is purposefully empty.

After I figure out how to test this one properly, I'll add a fix which will skip serializing iframes that do not have document elements. This will suppress that error, but might result in unexpected or flakey snapshots when re-rendering in Percy's infrastructure without serialized frames (for example, if they aren't intended to be empty and are not publicly accessible from within Percy's rendering environment).

In the meantime, you might be able to work around the issue either by waiting longer for the page to settle before taking the snapshot, or by finding and removing troublesome frames before the DOM is serialized. I see you're using the snapshot command on a static directory, so it may take a bit of work to convert to a page listing file to be able to pass in wait or execute options. We are going to be adding better documentation and a few more features to this command soon that will make these options easier to use with static directories.

Another quick option you might try is the --network-idle-timeout flag (which defaults to 100ms). This is meant to signal to asset discovery how long to wait after network activity has stopped before the page is considered to be idle. This might have the effect of waiting long enough for those iframes to be settled, but the the other wait options would be more reliable. Of course, if the iframes purposefully have empty documents, neither this nor the wait options will work and you'll need to wait for the fix to ship or to remove the iframes before they get serialized.

[mitar]

This might have the effect of waiting long enough for those iframes to be settled, but the the other wait options would be more reliable.

The CI above already included 1000 ms as the timeout value. Are you sure that network timeout captures the networking in iframes, too? Because it does not feel to me like it really influences anything for iframes.

Of course, if the iframes purposefully have empty documents

No, none is empty on purpose. And on all pages they are the same iframes (twitter and disqus).

In my case I think simply removing all iframes would probably work. Maybe an option to CLI which would do that would be best? I already hide some iframes.

[wwilsman]

Ah, I missed the timeout option in the CI run. "Purposefully empty" could be that Twitter or Disqus have nested iframes and one of the intermediary iframes or maybe even a main iframe could have some protection in place to prevent reading the DOM. I'll be working on a simple reproduction for tests shortly, but it may be as simple as the script they run unsets the document element property.

I'll start working on a test now for ignoring iframes that are missing document elements. If you don't see a mention in a PR in the next hour or so, I'll have it finished up tomorrow. As for more options for the snapshot CLI command, that work will probably happen over the next couple of weeks alongside better documentation.

If you want to work around this right away, you'll need to convert to using a page listing file in order to provide pages an execute option to run before the snapshot is serialized. That unfortunately would involve you starting up your own server and iterating over the pages like the command does internally.

Based on the example JS page listing in the linked doc and the page iteration in the command source, you could map over the filenames and return the local URL, name, and then the execute function. To use the page listing, start your server and run the snapshot command with the page listing file. Here's what that might look like (untested):

// percy-snapshots.js
const globby = require('globby');

const SERVER_ADDR = 'http://localhost:8008/';
const PUBLIC_DIR = './public';
const SNAPSHOT_FILES = ['*.html'];
const IGNORE_FILES = [];

module.exports = async () => {
  let paths = await globby(SNAPSHOT_FILES, { 
    cwd: PUBLIC_DIR,
    ignore: IGNORE_FILES 
  });
  
  return paths.map(path => ({
    name: path,
    url: `${SERVER_ADDR}${path}`,
    // this function runs in the browser before the snapshot is taken
    execute() {
      document.querySelector('.problem-iframe').remove();
    }
  }));
}

# start your server first, then run
$ percy snapshot percy-snapshots.js

[wwilsman]

Fixed in v1.0.0-beta.50

[mitar]

Awesome! I also made #343 as a followup.