Transient errors in visual diff tests, likely due to unexpected navigation

[rsimha]

@danielrozenberg Not sure if this is a one off, or a recurring problem. I'll post an update if I see this happen again.

https://travis-ci.org/ampproject/amphtml/jobs/498357687#L1351-L1368

[20:19:46] ERROR: Error: Execution context was destroyed, most likely because of a navigation.
    at rewriteError (/home/travis/build/ampproject/amphtml/build-system/tasks/visual-diff/node_modules/puppeteer/lib/ExecutionContext.js:144:15)
    at process._tickCallback (internal/process/next_tick.js:68:7)
  -- ASYNC --
    at ExecutionContext.<anonymous> (/home/travis/build/ampproject/amphtml/build-system/tasks/visual-diff/node_modules/puppeteer/lib/helper.js:144:27)
    at _documentPromise._contextPromise.then (/home/travis/build/ampproject/amphtml/build-system/tasks/visual-diff/node_modules/puppeteer/lib/FrameManager.js:459:38)
    at process._tickCallback (internal/process/next_tick.js:68:7)
  -- ASYNC --
    at Frame.<anonymous> (/home/travis/build/ampproject/amphtml/build-system/tasks/visual-diff/node_modules/puppeteer/lib/helper.js:144:27)
    at Page.$$ (/home/travis/build/ampproject/amphtml/build-system/tasks/visual-diff/node_modules/puppeteer/lib/Page.js:323:29)
    at Page.<anonymous> (/home/travis/build/ampproject/amphtml/build-system/tasks/visual-diff/node_modules/puppeteer/lib/helper.js:145:23)
    at waitForElementVisibility (/home/travis/build/ampproject/amphtml/build-system/tasks/visual-diff/helpers.js:150:44)
    at waitForLoaderDots (/home/travis/build/ampproject/amphtml/build-system/tasks/visual-diff/helpers.js:122:35)
    at page.goto.then.then (/home/travis/build/ampproject/amphtml/build-system/tasks/visual-diff/index.js:396:19)
    at process._tickCallback (internal/process/next_tick.js:68:7)
[20:19:46] FATAL: Some tests have failed locally.
Percy is now processing. You can view the visual diffs here: https://percy.io/ampproject/amphtml/builds/1485866
[20:19:46] Finished 'visual-diff' after 57 s

[rsimha]

Seems like this is happening inside waitForLoaderDots. I'm guessing the page should be able to change its content as it loads, but I can't tell for sure which visual test caused this. I wonder if there's a way to add a try-catch block inside waitForElementVisibility?

[rsimha]

Definitely a recurring problem. For example:
https://travis-ci.org/ampproject/amphtml/jobs/498546221#L1360
https://travis-ci.org/ampproject/amphtml/jobs/499440074#L1358

[rsimha]

Marking this high priority since it's disrupting visual tests more often than not.

[rsimha]

@danielrozenberg I found two leads that might contain a fix for this:

• When to use networkidle2? puppeteer/puppeteer#1552
• Execution context was destroyed, most likely because of a navigation puppeteer/puppeteer#3323

To fix this:

• I've attempted the networkidle2 fix in Use Puppeteer's networkidle2 heuristic event for page idleness #21808, and sent it to you for review.
• There's a responseHandler approach to detect and handle navigations in this comment. Will let you try that if the above attempt fails.

[danielrozenberg]

Looking at pull requests on Travis over the past 5 days, I believe this is now fixed. Feel free to reopen if you encounter these again