percy / percy-agent

[Deprecated in favor of `@percy/cli`] An agent process for integrating with Percy.
https://percy.io
MIT License
22 stars 25 forks source link

No data found for resource with given identifier (Possible race condition) #92

Closed Robdel12 closed 5 years ago

Robdel12 commented 5 years ago

Problem

As of @percy/agent version 0.1.15 we introduced responsive width detection, which renders the DOM snapshot we take in the widths that are specified to the SDK concurrently. When trying to snapshot some sites, this can cause a race condition where this error is thrown:

Error: Protocol error (Network.getResponseBody): No data found for resource with given identifier
    at Promise (/Users/robertdeluca/Projects/Work/percy-agent/node_modules/puppeteer/lib/Connection.js:186:56)
    at new Promise (<anonymous>)
    at CDPSession.send (/Users/robertdeluca/Projects/Work/percy-agent/node_modules/puppeteer/lib/Connection.js:185:12)
    at _contentPromise._bodyLoadedPromise.then (/Users/robertdeluca/Projects/Work/percy-agent/node_modules/puppeteer/lib/NetworkManager.js:591:45)
    at process._tickCallback (internal/process/next_tick.js:68:7)

Googling that error returns a few different issues that might be helpful:

You can recreate this issue repeatedly with this puppeteer script:

const puppeteer = require('puppeteer');
const {percySnapshot} = require('@percy/puppeteer');

(async () => {
  const browser = await puppeteer.launch({ headless: true });
  const page = await browser.newPage();

  await page.goto('https://www.nash.io/company/careers');

  await percySnapshot(page, 'Does this snapshot?');
  await browser.close();
})();

If you downgrade @percy/agent to v0.1.14 the above snippet captures the snapshot properly. Singling out a width also does not help in the latest version.

Where does this blow up in agent? 💥

Given the above puppeteer script, this error comes up on an .mp4 video file when calling .buffer()on the response: https://github.com/percy/percy-agent/blob/master/src/services/response-service.ts#L70

image

anaulin commented 5 years ago

Digging deeper into what the agent is doing in this case, here is a Puppeteer-only script that reproduces this issue:

const puppeteer = require('puppeteer');

const URL = 'https://www.nash.io/company/careers';

(async() => {

const browser = await puppeteer.launch();
const page = await browser.newPage();
page.on('response', async (response) => {
  if ([200, 201, 304].includes(response.status())) {
    await response.buffer()
  }
});

console.log('waiting for url to load')
await page.goto(URL, { waitUntil: 'networkidle0' })

await browser.close()

})();
anaulin commented 5 years ago

~In the repro script above, adding a wait until there are no more network requests fixes the issue, i.e. changing the page.goto(URL) call to be:~

await page.goto(URL, { waitUntil: 'networkidle0' })

~However, adding that same wait to our agent code on line https://github.com/percy/percy-agent/blob/master/src/services/asset-discovery-service.ts#L162 did not help the issue we're seeing.~

anaulin commented 5 years ago

I'm taking back my previous comment -- the repro script fails even with { waitUntil: 'networkidle0' }, it just fails less frequently. :-/

anaulin commented 5 years ago

I added my reproduce example to https://github.com/GoogleChrome/puppeteer/issues/1274#issuecomment-471793396, where the OP was having a similar situation involving videos.

anaulin commented 5 years ago

I believe this issue can be marked as closed or resolved, if we believe the conclusion in https://github.com/GoogleChrome/puppeteer/issues/1274#issuecomment-473486095 about how the underlying issue is the server intermittently resetting the connection.

Would also be good to confirm/deny that the change in https://github.com/percy/percy-agent/pull/98 made at least things more robust.

Robdel12 commented 5 years ago

It did from what I can tell! I think this is good to close. 👍 Nice work on this one