Browser not properly closing

[gmetais]

Hi @macbre, how are you?

While updating Phantomas, I noticed a behavior that doesn't properly close the headless browser and lets it run in background forever (or until timeout).

How to reproduce:

Run a test on a domain that doesn't exist, for example https://www.sdkjfhsklhdfhjhbj.com

Here is a minimalist script to reproduce:

var phantomas = require('phantomas');

phantomas('https://www.sdkjfhsklhdfhjhbj.com', {})
  .catch((err) => {
    console.log("Error: %s", err);
  })
  .then(async (results) => {
    console.log("Success");
  });

Phantomas will fail but will not stop running. It will even start analysing the Chrome's error page instead.

To better view what I mean, you can switch the option headless: "new" by headless: false in lib/browser.js. The browser stays open until it reaches the timeout.

Why is it failing?

The Browser.prototype.close function will try closing the page with await this.page.close({ runBeforeUnload: true });
Puppeteer responds Protocol error (Page.close): Not attached to an active page.
Phantomas goes directly to the catch function and skip the job of calling this.browser.close().

Error log:

  phantomas:network Request failed with "net::ERR_NAME_NOT_RESOLVED" +74ms
  puppeteer:protocol:RECV ◀ [
  puppeteer:protocol:RECV ◀   '{"method":"Network.loadingFailed","params":{"requestId":"DC441A26FD63AF6EAFB27E6135080A8A","timestamp":811672.901891,"type":"Document","errorText":"net::ERR_NAME_NOT_RESOLVED","canceled":false},"sessionId":"1F4ACD4C1193C826105BDE0F904FE608"}'
  puppeteer:protocol:RECV ◀ ] +0ms
  puppeteer:protocol:RECV ◀ [
  puppeteer:protocol:RECV ◀   '{"id":34,"result":{"frameId":"5BD9C87E49AE5C1DAE9C630BF7A3FF2A","loaderId":"DC441A26FD63AF6EAFB27E6135080A8A","errorText":"net::ERR_NAME_NOT_RESOLVED"},"sessionId":"1F4ACD4C1193C826105BDE0F904FE608"}'
  puppeteer:protocol:RECV ◀ ] +0ms
  phantomas:browser Opening URL failed: Error: net::ERR_NAME_NOT_RESOLVED at https://www.sdkjfhsklhdfhjhbj.com +102ms
  phantomas:core Exception caught: Error: net::ERR_NAME_NOT_RESOLVED at https://www.sdkjfhsklhdfhjhbj.com +154ms
  phantomas:core Error: net::ERR_NAME_NOT_RESOLVED at https://www.sdkjfhsklhdfhjhbj.com
  phantomas:core     at navigate (/Users/gaelmetais/Documents/GitHub/phantomas/node_modules/puppeteer-core/lib/cjs/puppeteer/common/Frame.js:111:23)
  phantomas:core     at processTicksAndRejections (node:internal/process/task_queues:96:5)
  phantomas:core     at async Function.race (/Users/gaelmetais/Documents/GitHub/phantomas/node_modules/puppeteer-core/lib/cjs/puppeteer/util/Deferred.js:89:20)
  phantomas:core     at async Frame.goto (/Users/gaelmetais/Documents/GitHub/phantomas/node_modules/puppeteer-core/lib/cjs/puppeteer/common/Frame.js:77:21)
  phantomas:core     at async CDPPage.goto (/Users/gaelmetais/Documents/GitHub/phantomas/node_modules/puppeteer-core/lib/cjs/puppeteer/common/Page.js:444:16)
  phantomas:core     at async /Users/gaelmetais/Documents/GitHub/phantomas/lib/browser.js:301:7 +0ms
  puppeteer:protocol:SEND ► [
  puppeteer:protocol:SEND ►   '{"method":"Page.close","id":35,"sessionId":"1F4ACD4C1193C826105BDE0F904FE608"}'
  puppeteer:protocol:SEND ► ] +0ms
  puppeteer:protocol:RECV ◀ [
  puppeteer:protocol:RECV ◀   '{"id":35,"error":{"code":-32000,"message":"Not attached to an active page"},"sessionId":"1F4ACD4C1193C826105BDE0F904FE608"}'
  puppeteer:protocol:RECV ◀ ] +0ms
  phantomas:browser An exception was raised in Browser.prototype.close(): ProtocolError: Protocol error (Page.close): Not attached to an active page +8ms
  phantomas:browser ProtocolError: Protocol error (Page.close): Not attached to an active page
  phantomas:browser     at new Callback (/Users/gaelmetais/Documents/GitHub/phantomas/node_modules/puppeteer-core/lib/cjs/puppeteer/common/Connection.js:62:35)
  phantomas:browser     at CallbackRegistry.create (/Users/gaelmetais/Documents/GitHub/phantomas/node_modules/puppeteer-core/lib/cjs/puppeteer/common/Connection.js:108:26)
  phantomas:browser     at Connection._rawSend (/Users/gaelmetais/Documents/GitHub/phantomas/node_modules/puppeteer-core/lib/cjs/puppeteer/common/Connection.js:224:26)
  phantomas:browser     at CDPSessionImpl.send (/Users/gaelmetais/Documents/GitHub/phantomas/node_modules/puppeteer-core/lib/cjs/puppeteer/common/Connection.js:433:78)
  phantomas:browser     at CDPPage.close (/Users/gaelmetais/Documents/GitHub/phantomas/node_modules/puppeteer-core/lib/cjs/puppeteer/common/Page.js:676:70)
  phantomas:browser     at Browser.close (/Users/gaelmetais/Documents/GitHub/phantomas/lib/browser.js:336:23)
  phantomas:browser     at /Users/gaelmetais/Documents/GitHub/phantomas/lib/index.js:203:21
  phantomas:browser     at processTicksAndRejections (node:internal/process/task_queues:96:5) +0ms
  phantomas:AwaitEventEmitter:emit close +93ms
  phantomas:browser Browser closed +1ms
Error: Error: net::ERR_NAME_NOT_RESOLVED at https://www.sdkjfhsklhdfhjhbj.com
    at navigate (/Users/gaelmetais/Documents/GitHub/phantomas/node_modules/puppeteer-core/lib/cjs/puppeteer/common/Frame.js:111:23)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async Function.race (/Users/gaelmetais/Documents/GitHub/phantomas/node_modules/puppeteer-core/lib/cjs/puppeteer/util/Deferred.js:89:20)
    at async Frame.goto (/Users/gaelmetais/Documents/GitHub/phantomas/node_modules/puppeteer-core/lib/cjs/puppeteer/common/Frame.js:77:21)
    at async CDPPage.goto (/Users/gaelmetais/Documents/GitHub/phantomas/node_modules/puppeteer-core/lib/cjs/puppeteer/common/Page.js:444:16)
    at async /Users/gaelmetais/Documents/GitHub/phantomas/lib/browser.js:301:7
Error: net::ERR_NAME_NOT_RESOLVED at https://www.sdkjfhsklhdfhjhbj.com

Notice that the Browser.close puppeteer method is not called.

How to fix:

My approach would be to duplicate the try/catch in lib/browser.js. One try/catch for page.close, then second try/catch for browser.close. Even if page.close() fails, the browser.close would still get called.

I'll create a pull request, but don't hesitate to rewrite it if you know a better fix.

Related?

This issue could maybe explain #1288.

[macbre]

@gmetais, thanks for both reporting an issue and submitting a PR with a fix. Long time, no see 😉