feat: ensure close page

Closes #28

Author: Kikobeats

README.md

@@ -260,7 +260,7 @@ These devices are used for emulation purposes.
 
 #### .getDevice(deviceName)
 
-Get an specific device descriptor settings by descriptor name.
+Get a specific device descriptor settings by descriptor name.
 
 ```js
 const browserless = require('browserless')
@@ -299,7 +299,7 @@ const browserless = require('browserless')
 
 ### .evaluate(page, response)
 
-It exposes an interface for creating your own evaluate function.
+It exposes an interface for creating your own evaluate function, passing you the `page` and `response`.
 
 ```js
 const browserless = require('browserless')()
@@ -323,7 +323,9 @@ const getUrlInfo = browserless.evaluate((page, response) => ({
 })()
 ```
 
-Internally the method performs a [.goto](#gotopage-options) operation and it will pass you the `page` and `reponse`.
+Note you don't need to close the page; It will be done under the hood.
+
+Internally the method performs a [.goto](#gotopage-options).
 
 ### .goto(page, options)
 

packages/browserless/package.json

@@ -32,6 +32,7 @@
     "debug": "~4.1.0",
     "extract-domain": "~2.0.4",
     "is-tracking-domain": "~1.1.6",
+    "p-timeout": "~2.0.1",
     "require-one-of": "~1.0.2"
   },
   "devDependencies": {

packages/browserless/src/index.js

@@ -3,6 +3,7 @@
 const extractDomain = require('extract-domain')
 const requireOneOf = require('require-one-of')
 const debug = require('debug')('browserless')
+const pTimeout = require('p-timeout')
 
 const { getDevice } = require('./devices')
 const isTracker = require('./is-tracker')
@@ -18,11 +19,7 @@ const isExternalUrl = (domainOne, domainTwo) => domainOne !== domainTwo
 const isEmpty = val => val == null || !(Object.keys(val) || val).length
 
 module.exports = ({
-  puppeteer = requireOneOf([
-    'puppeteer',
-    'puppeteer-core',
-    'puppeteer-firefox'
-  ]),
+  puppeteer = requireOneOf(['puppeteer', 'puppeteer-core', 'puppeteer-firefox']),
   incognito = false,
   timeout = 30000,
   ...launchOpts
@@ -49,14 +46,28 @@ module.exports = ({
 
   const newPage = () =>
     Promise.resolve(browser).then(async browser => {
-      const context = incognito
-        ? await browser.createIncognitoBrowserContext()
-        : browser
+      const context = incognito ? await browser.createIncognitoBrowserContext() : browser
       const page = await context.newPage()
       page.setDefaultNavigationTimeout(timeout)
       return page
     })
 
+  const wrapError = fn => async (...args) => {
+    const page = await newPage()
+    let error
+    let res
+
+    try {
+      res = await pTimeout(fn(page)(...args), timeout)
+    } catch (err) {
+      error = err
+    }
+
+    await page.close()
+    if (error) throw error
+    return res
+  }
+
   const goto = async (
     page,
     {
@@ -96,8 +107,7 @@ module.exports = ({
       return req.continue()
     })
 
-    const { userAgent: deviceUserAgent, viewport: deviceViewport } =
-      getDevice(device) || {}
+    const { userAgent: deviceUserAgent, viewport: deviceViewport } = getDevice(device) || {}
 
     const userAgent = deviceUserAgent || fallbackUserAgent
     if (userAgent) await page.setUserAgent(userAgent)
@@ -109,50 +119,32 @@ module.exports = ({
     return response
   }
 
-  const evaluate = fn => async (url, opts = {}) => {
-    const {
-      abortTrackers = true,
-      abortTypes = [
-        'image',
-        'imageset',
-        'media',
-        'stylesheet',
-        'font',
-        'object',
-        'sub_frame'
-      ],
-      ...args
-    } = opts
-
-    const page = await newPage()
-    const response = await goto(page, {
-      url,
-      abortTrackers,
-      abortTypes,
-      ...args
+  const evaluate = fn =>
+    wrapError(page => async (url, opts = {}) => {
+      const {
+        abortTrackers = true,
+        abortTypes = ['image', 'imageset', 'media', 'stylesheet', 'font', 'object', 'sub_frame'],
+        ...args
+      } = opts
+
+      const response = await goto(page, {
+        url,
+        abortTrackers,
+        abortTypes,
+        ...args
+      })
+
+      return fn(page, response)
     })
-    const content = await fn(page, response)
-    await page.close()
-    return content
-  }
 
-  const screenshot = async (url, opts = {}) => {
-    const {
-      device = 'macbook pro 13',
-      tmpOpts,
-      type = 'png',
-      viewport,
-      ...args
-    } = opts
+  const screenshot = wrapError(page => async (url, opts = {}) => {
+    const { device = 'macbook pro 13', tmpOpts, type = 'png', viewport, ...args } = opts
 
-    const page = await newPage()
     await goto(page, { url, device, ...args })
-    const file = await page.screenshot({ type, ...args })
-    await page.close()
-    return file
-  }
+    return page.screenshot({ type, ...args })
+  })
 
-  const pdf = async (url, opts = {}) => {
+  const pdf = wrapError(page => async (url, opts = {}) => {
     const {
       format = 'A4',
       margin = {
@@ -169,19 +161,16 @@ module.exports = ({
       ...args
     } = opts
 
-    const page = await newPage()
     await page.emulateMedia(media)
     await goto(page, { url, ...args })
-    const file = await page.pdf({
+    return page.pdf({
       margin,
       format,
       printBackground,
       scale,
       ...args
     })
-    await page.close()
-    return file
-  }
+  })
 
   return {
     browser,

packages/browserless/test/spec.js

@@ -28,9 +28,7 @@ module.exports = createBrowserless => {
       path: filepath
     })
 
-    t.true(
-      await looksSame(filepath, path.resolve(__dirname, 'fixtures/example.png'))
-    )
+    t.true(await looksSame(filepath, path.resolve(__dirname, 'fixtures/example.png')))
   })
   ;(isCI ? test.skip : test)('.screenshot (jpeg)', async t => {
     const browserless = createBrowserless()
@@ -40,12 +38,7 @@ module.exports = createBrowserless => {
       path: filepath
     })
 
-    t.true(
-      await looksSame(
-        filepath,
-        path.resolve(__dirname, 'fixtures/example.jpeg')
-      )
-    )
+    t.true(await looksSame(filepath, path.resolve(__dirname, 'fixtures/example.jpeg')))
   })
   ;(isCI ? test.skip : test)('devices', async t => {
     const browserless = createBrowserless()
@@ -55,17 +48,28 @@ module.exports = createBrowserless => {
       path: filepath
     })
 
-    t.true(
-      await looksSame(
-        filepath,
-        path.resolve(__dirname, 'fixtures/example-iphone.png')
-      )
-    )
+    t.true(await looksSame(filepath, path.resolve(__dirname, 'fixtures/example-iphone.png')))
   })
   ;(isCI ? test.skip : test)('.pdf', async t => {
     const browserless = createBrowserless()
     const buffer = await browserless.pdf('http://example.com')
     const data = await pdf(buffer)
     t.snapshot(data.text.trim())
   })
+  ;['pdf', 'screenshot', 'html', 'text'].forEach(method => {
+    test(`.${method} wrap errors`, async t => {
+      const browserless = createBrowserless()
+      const timeout = 500
+
+      const error = await t.throwsAsync(browserless[method]('https://example.com', { timeout }))
+
+      t.is(error.name, 'TimeoutError')
+      t.is(error.message, `Navigation Timeout Exceeded: ${timeout}ms exceeded`)
+
+      const browser = await browserless.browser
+      const pages = await browser.pages()
+
+      t.is(pages.length, 1) // about:page is always open
+    })
+  })
 }

packages/examples/package.json

@@ -3,6 +3,7 @@
   "version": "",
   "devDependencies": {
     "browserless": "latest",
+    "puppeteer": "latest",
     "lodash": "latest",
     "p-event": "latest",
     "term-img": "latest"