Merge 35c7b06 into e4242d5

microlinkhq · Jun 3, 2023 · d2c65af · d2c65af
2 parents e4242d5 + 35c7b06
commit d2c65af
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 35 deletions.
diff --git a/packages/browserless/package.json b/packages/browserless/package.json
@@ -37,6 +37,7 @@
     "@browserless/screenshot": "^9.10.2",
     "debug-logfmt": "~1.0.4",
     "kill-process-group": "~1.0.3",
+    "p-cancelable": "2.1.1",
     "p-reflect": "~2.1.0",
     "p-retry": "~4.6.1",
     "p-timeout": "~4.1.0",

diff --git a/packages/browserless/src/index.js b/packages/browserless/src/index.js
@@ -5,6 +5,7 @@ const createScreenshot = require('@browserless/screenshot')
 const debug = require('debug-logfmt')('browserless')
 const createGoto = require('@browserless/goto')
 const createPdf = require('@browserless/pdf')
+const PCancelable = require('p-cancelable')
 const { withLock } = require('superlock')
 const pReflect = require('p-reflect')
 const pTimeout = require('p-timeout')
@@ -107,46 +108,50 @@ module.exports = ({ timeout: globalTimeout = 30000, ...launchOpts } = {}) => {
       }
     }
 
-    const withPage = (fn, { timeout: evaluateTimeout } = {}) => async (...args) => {
-      let isRejected = false
-
-      async function run () {
-        let page
-
-        try {
-          page = await createPage(args)
-          setTimeout(() => closePage(page), timeout)
-          const value = await fn(page)(...args)
-          await closePage(page)
-          return value
-        } catch (error) {
-          await closePage(page)
-          if (!isRejected) throw ensureError(error)
+    const withPage = (fn, { timeout: evaluateTimeout } = {}) =>
+      PCancelable.fn(async (...args) => {
+        const onCancel = args.pop()
+        let isRejected = false
+
+        async function run () {
+          let page
+
+          try {
+            page = await createPage(args)
+            const close = () => closePage(page)
+            onCancel(close)
+            setTimeout(close, timeout).unref()
+            const value = await fn(page)(...args)
+            await closePage(page)
+            return value
+          } catch (error) {
+            await closePage(page)
+            if (!isRejected) throw ensureError(error)
+          }
         }
-      }
 
-      const task = () =>
-        pRetry(run, {
-          retries: retry,
-          onFailedAttempt: async error => {
-            debug('onFailedAttempt', { name: error.name, code: error.code, isRejected })
-            if (error.name === 'AbortError') throw error
-            if (isRejected) throw new AbortError()
-            if (error.code === 'EBRWSRCONTEXTCONNRESET') {
-              _contextPromise = createBrowserContext(contextOpts)
+        const task = () =>
+          pRetry(run, {
+            retries: retry,
+            onFailedAttempt: async error => {
+              debug('onFailedAttempt', { name: error.name, code: error.code, isRejected })
+              if (error.name === 'AbortError') throw error
+              if (isRejected) throw new AbortError()
+              if (error.code === 'EBRWSRCONTEXTCONNRESET') {
+                _contextPromise = createBrowserContext(contextOpts)
+              }
+              const { message, attemptNumber, retriesLeft } = error
+              debug('retry', { attemptNumber, retriesLeft, message })
             }
-            const { message, attemptNumber, retriesLeft } = error
-            debug('retry', { attemptNumber, retriesLeft, message })
-          }
-        })
+          })
 
-      const timeout = evaluateTimeout || contextTimeout || globalTimeout
+        const timeout = evaluateTimeout || contextTimeout || globalTimeout
 
-      return pTimeout(task(), timeout, () => {
-        isRejected = true
-        throw browserTimeout({ timeout })
+        return pTimeout(task(), timeout, () => {
+          isRejected = true
+          throw browserTimeout({ timeout })
+        })
       })
-    }
 
     const evaluate = (fn, gotoOpts) =>
       withPage(

diff --git a/packages/browserless/test/index.js b/packages/browserless/test/index.js
@@ -58,3 +58,24 @@ test('ensure to destroy browser contexts', async t => {
 
   t.is(browser.browserContexts().length, 1)
 })
+;[
+  { method: 'evaluate', isDecorator: true },
+  { method: 'withPage', isDecorator: true },
+  { method: 'html' },
+  { method: 'pdf' },
+  { method: 'screenshot' },
+  { method: 'text' }
+].forEach(({ method, isDecorator = false }) => {
+  test(`.${method} is cancelable`, async t => {
+    const browserless = await getBrowserContext(t)
+
+    const fn = isDecorator
+      ? browserless[method](() => {})
+      : () => browserless[method]('about:blank')
+
+    const promise = fn()
+    promise.catch(() => {})
+    t.is(!!promise.cancel, true)
+    promise.cancel()
+  })
+})
diff --git a/packages/cli/src/index.js b/packages/cli/src/index.js
@@ -10,6 +10,8 @@ const path = require('path')
 const mri = require('mri')
 const fs = require('fs')
 
+const noop = () => {}
+
 const commands = fs.readdirSync(path.resolve(__dirname, 'commands'))
 
 const { _, ...flags } = mri(process.argv.slice(2), {
@@ -32,7 +34,7 @@ const cli = {
 
 const { verbose, headless } = cli.flags
 
-const spinner = verbose ? require('./spinner') : { start: () => {}, stop: () => {} }
+const spinner = verbose ? require('./spinner') : { start: noop, stop: noop, clear: noop }
 
 process.on('SIGINT', () => {
   spinner.stop({ force: true })