frame.evaluate freezes / hangs on detached frames.

[sdeprez]

• Puppeteer version: tested with 1.6.2 and 1.8.0
• Platform / OS version: ubuntu 16.04
• Node.js version: 8.9.1

Repro script:

const browser = await puppeteer.launch()

const page = await browser.newPage()
await page.goto('https://example.com')
await page.evaluate(`
  const iframe = document.createElement('iframe')
  iframe.src = 'https://www.example.com'
  document.body.appendChild(iframe)
  setTimeout(() => document.body.removeChild(iframe), 1000)
`)

// The frame does not appear immediately.
await new Promise((resolve) => setTimeout(resolve, 100))

const iframe = page.frames()[1]
console.log(iframe.isDetached())  // -> false
console.log(await iframe.evaluate(() => window.location.href))

await new Promise((resolve) => setTimeout(resolve, 1500))

console.log(iframe.isDetached())  // -> true
// Uncomment this line to make the script hangs here.
// console.log(await iframe.evaluate(() => window.location.href))

await browser.close()

If you un-comment the last .evaluate, the code hangs whereas it should not. I guess it should raise an error like DetachedFrameEvalutionError or something similar to allow to catch it. Note that you can check manually frame.isDetached() to avoid the call to evaluate but IMHO:

1. if you're manipulating many frames in many places, it's much more convient to have one single try-catch to catch all the detached frame errors (and give up with this frame) than to put the check around each call to evaluate.
2. more importantly, it's probably subject to some race conditions.

Thanks!

[thovden]

Seeing this problem in the wild - especially on sites which runs programmatic ads.

It seems frame.childFrames() can return detached frames:

    const frames = frame.childFrames()
    for (const frame of frames) {
      // this happens, which makes me wonder if it can happen later
      if ( frame.isDetached() ) {
        continue
      }
     await frame.evaluate(() => ...)
     // can the frame detach between these async calls?
     await frame.evaluate(() => ...)
   }

Wondering if I have to check before every evaluate call since frames can detach at any time. That means a lot of boilerplate.

[thovden]

Since have a runner class per Frame, I ended up wrapping frame in a getter. This will ensure that I don't call any frame methods for disconnected frames, which will result in hangs:

class Runner {
  private internalFrame
  get frame() {
    if ( this.internalFrame.isDetached() ) {
      throw new Error(`Frame is detached - cannot access`)
    }
    return this.internalFrame
  }
  constructor(frame, ...) {
    this.internalFrame = frame
  }
  async run() {
    // ...
   try {
      await this.frame.evaluate(/* ... */)
   } catch (e) {
     /** Catch errors, including detached frames */
   }
}

@aslushnikov appreciate if this can be fixed in Puppeteer / Chromium.