Closing the browser when last page is closed sometimes throws an error

[MJPA]

Tell us about your environment:

• Puppeteer version: 1.2.0
• Platform / OS version: CentOS 7.4.1708
• URLs (if applicable): N/A
• Node.js version: v8.10.0

What steps will reproduce the problem?

const puppeteer = require('puppeteer');

const run = async () => {
  const browser = await puppeteer.launch();

  // close any open pages
  let pages = await browser.pages();
  await Promise.all(pages.map(async page => await page.close()));

  // handle a page being closed
  browser.on('targetdestroyed', async target => {
    const openPages = await browser.pages();
    console.log('Open pages:', openPages.length);
    if (openPages.length == 0) {
      console.log('Closing empty browser');
      await browser.close();
      console.log('Browser closed');
    }
  });

  pages = await browser.pages();
  for (let i = 0; i < 5; i++) {
    await browser.newPage();
  }

  pages = await browser.pages();
  pages.forEach((page) => page.close());
};

run();

What is the expected result?

Count: 0
Open pages: 4
Open pages: 3
Open pages: 2
Open pages: 1
Open pages: 0
Closing empty browser
Browser closed

What happens instead?

Open pages: 4
Open pages: 3
Open pages: 2
Open pages: 1
Open pages: 0
Closing empty browser
(node:29074) UnhandledPromiseRejectionWarning: Error: Protocol error (Target.closeTarget): Target closed.
    at Promise (node_modules/puppeteer/lib/Connection.js:86:56)
    at new Promise (<anonymous>)
    at Connection.send (node_modules/puppeteer/lib/Connection.js:85:12)
    at Page.close (node_modules/puppeteer/lib/Page.js:802:36)
    at pages.forEach (test.js:27:32)
    at Array.forEach (<anonymous>)
    at run (test.js:27:9)
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)
(node:29074) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:29074) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
Browser closed

This does not happen every run but it happens every so often. It's entirely possible I'm doing something bad here!

[dragosrotaru]

+1

[MJPA]

Did a bit of quick debugging, the targetDestroyed event is fired before page.close() message has been acknowledged (or rather, puppeteer has processed the acknowledgement).

I can think of ways to fix it - eg wait for a Target.closeTarget acknowledgement before handling the targetDestroyed event. How to properly do that though is another question!

[aslushnikov]

Thanks @MJPA for the investigation.

The issue comes from the fact that browser removes targets from the list before the associated pages are closed.

In the following example, I'd expect browser.pages() to report 1 page until the page.close() call is finished:

const puppeteer = require('puppeteer');

(async () => {
  const browser = await puppeteer.launch();
  const [page] = await browser.pages();
  browser.on('targetdestroyed', async () => console.log('Target destroyed. Pages count: ' + (await browser.pages()).length));
  console.log('closing page...');
  await page.close();
  console.log('page closed.');
  await browser.close();
})();

The output:

closing page...
Target destroyed. Pages count: 0
page closed.

@JoelEinbinder what do you think?

[awesomenkj]

Same issue as @MJPA

        await page.waitForSelector('aside.aside a[href="/checkout"]')
	.then(() => {
		logMessage('Checkout is available');
	})
	.catch(err => {
		log('Checkout not available');
		success = false;
		browser.close();
	});
	// click checkout button
	await Promise.all([
		page.click('aside.aside a[href="/checkout"]'),
		page.waitForNavigation()
	]);

In this case, always same errors occurred.

Maybe this puppeteer error.

[ansonlouis]

I get the same error with a slightly similar implementation, but through listening to the close event on a page. The documentation isn't overly explicit on what "close" means, but according to it's implementation, close is fired before the page is closed, so the following example doesn't work, similar to the targetdestroyed example.

const puppeteer = require('puppeteer');
const browser = await puppeteer.launch();

// create 5 test pages
for(let i=0; i<5; i++){

  let page = await browser.newPage();
  
  // listen to page close event and close browser
  // when all get closed
  page.on('close', async () => {
    let pages = await browser.pages();
    if(!pages.length){
      // will not get here!
      browser.close();
    }
  });

}

let pages = await browser.pages();

pages.forEach(async (page) => await page.close());  

I guess this sort of makes sense, since the event is called close and not closed. I got around this on my own by emitting my own closed event in the close() promise callback and listening to that instead, which does work.

page.close().then(() => {
  page.emit('closed');
});

The moral of the story seems to be there is no 100% guaranteed method of knowing when a browser is safe to close (without having to catch the exception). Please let me know if I'm missing something.

[jacobweber]

Possibly related to #3423?

[kevupton]

I get this after closing all tabs in a browser and then attempting to open a new page

[JoelEinbinder]

In the following example, I'd expect browser.pages() to report 1 page until the page.close() call is finished:

This would break the code in the above example. The last page closed, brower.pages().length would be 1, the browser would not be closed. Then no more target destroyed events would come and the browser would live forever.

I'd say this is user error, although it is subtle. Any calls to page.close should be awaited before calling browser.close().

If there are no user controlled pages, simply Promise.all all the page closes, then close the browser. There should be no need to listen to the targetdestroyed events. Or just call browser.close and let all the pages close by themselves.

If there are user controlled pages, then listen to the targetdestroyed events, but there should be no need to call page.close.

[Yokutto]

+1.
I started to see this error after closing popups with page.on("popup"). When a popup fires inside an Frame context sometimes I can't close it with "popup.close()" and the puppeteer leaves the tab opened, so when I try to close the entire browser, it throws an "Target Closed" error, and the script stop.

I use puppeteer 1.14, with Chrome 73 (I don't use Chromium)
The code that I use to close the browser

async close_browser() {
    for (let page of await this.browser.pages()) {
      await page.close({
        "runBeforeUnload": true
      });
    }
   await this.browser.close();
}

[chebotarevmichael]

I added kill command:

  if (process.platform == 'win32') {
      executableBrowserPath = 'C:/Program Files/Google/Chrome/Application/chrome.exe'
      cmdKill = 'taskkill /F /T /PID '
  } else {
      executableBrowserPath = '/usr/bin/google-chrome'
      cmdKill = 'kill -s SIGKILL '
  }

  {
     const browser = await puppeteer.launch({headless: false, args: [....], executablePath: executableBrowserPath});
     const browser_pid = browser.process().pid;
     
     // close pages
     let pages = await browser.pages()
     pages.map(el => el.close())
     // close browser
     browser.close()
     // recommended to add sleep 1 second
     // delay(1000)
  } catch (e) {
     ....
  }
  exec(cmdKill + browser_pid, (error, stdout, stderr) => { });

[devzarghami]

i use promise and worked for me
Promise.all([ page.close(), browser.close() ])

[stale]

We're marking this issue as unconfirmed because it has not had recent activity and we weren't able to confirm it yet. It will be closed if no further activity occurs within the next 30 days.

[stale]

We are closing this issue. If the issue still persists in the latest version of Puppeteer, please reopen the issue and update the description. We will try our best to accomodate it!