Merge pull request #409 from marp-team/puppeteer-timeout

Allow to set timeout for Puppeteer actions by `PUPPETEER_TIMEOUT` env
marp-team · Jan 9, 2022 · 8d37880 · 8d37880
2 parents 5f490b3 + 2f1a4d2
commit 8d37880
Show file tree

Hide file tree

Showing 5 changed files with 79 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## [Unreleased]
 
+### Added
+
+- Allow to set timeout for Puppeteer actions by `PUPPETEER_TIMEOUT` env ([#409](https://github.com/marp-team/marp-cli/pull/409))
+
 ### Changed
 
 - Upgrade Marpit to [v2.2.1](https://github.com/marp-team/marpit/releases/tag/v2.2.1) ([#408](https://github.com/marp-team/marp-cli/pull/408))

diff --git a/src/config.ts b/src/config.ts
@@ -214,11 +214,20 @@ export class MarpCLIConfig {
       return scale
     })()
 
+    const puppeteerTimeout = (() => {
+      if (process.env['PUPPETEER_TIMEOUT']) {
+        const envTimeout = Number.parseInt(process.env['PUPPETEER_TIMEOUT'], 10)
+        if (!Number.isNaN(envTimeout)) return envTimeout
+      }
+      return undefined
+    })()
+
     return {
       imageScale,
       inputDir,
       output,
       preview,
+      puppeteerTimeout,
       server,
       template,
       templateOption,

diff --git a/src/converter.ts b/src/converter.ts
@@ -60,6 +60,7 @@ export interface ConverterOption {
   pages?: boolean | number[]
   pdfNotes?: boolean
   preview?: boolean
+  puppeteerTimeout?: number
   jpegQuality?: number
   server?: boolean
   template: string
@@ -105,6 +106,10 @@ export class Converter {
     return template
   }
 
+  get puppeteerTimeout(): number {
+    return this.options.puppeteerTimeout ?? 30000
+  }
+
   async convert(
     markdown: string,
     file?: File,
@@ -259,7 +264,11 @@ export class Converter {
 
     ret.buffer = await this.usePuppeteer(html, async (page, uri) => {
       await page.goto(uri, { waitUntil: ['domcontentloaded', 'networkidle0'] })
-      return await page.pdf({ printBackground: true, preferCSSPageSize: true })
+      return await page.pdf({
+        printBackground: true,
+        preferCSSPageSize: true,
+        timeout: this.puppeteerTimeout,
+      })
     })
 
     // Apply PDF metadata and annotations
@@ -482,7 +491,9 @@ export class Converter {
     })()
 
     try {
-      const browser = await Converter.runBrowser()
+      const browser = await Converter.runBrowser({
+        timeout: this.puppeteerTimeout,
+      })
 
       const uri = await (async () => {
         if (tmpFile) {
@@ -496,6 +507,8 @@ export class Converter {
       })()
 
       const page = await browser.newPage()
+      page.setDefaultTimeout(this.puppeteerTimeout)
+
       const { missingFileSet, failedFileSet } =
         this.trackFailedLocalFileAccess(page)
 
@@ -560,12 +573,13 @@ export class Converter {
 
   private static browser?: puppeteer.Browser
 
-  private static async runBrowser() {
+  private static async runBrowser({ timeout }: { timeout?: number }) {
     if (!Converter.browser) {
       const baseArgs = generatePuppeteerLaunchArgs()
 
       Converter.browser = await launchPuppeteer({
         ...baseArgs,
+        timeout,
         userDataDir: await generatePuppeteerDataDirPath('marp-cli-conversion', {
           wslHost: isChromeInWSLHost(baseArgs.executablePath),
         }),

diff --git a/test/converter.ts b/test/converter.ts
@@ -8,6 +8,7 @@ import { Options } from '@marp-team/marpit'
 import cheerio from 'cheerio'
 import { imageSize } from 'image-size'
 import { PDFDocument, PDFDict, PDFName, PDFHexString } from 'pdf-lib'
+import { TimeoutError } from 'puppeteer-core'
 import { Page } from 'puppeteer-core/lib/cjs/puppeteer/common/Page'
 import yauzl from 'yauzl'
 import { Converter, ConvertType, ConverterOption } from '../src/converter'
@@ -80,6 +81,17 @@ describe('Converter', () => {
     })
   })
 
+  describe('get #puppeteerTimeout', () => {
+    it('returns specified timeout', () => {
+      expect(instance({ puppeteerTimeout: 1000 }).puppeteerTimeout).toBe(1000)
+      expect(instance({ puppeteerTimeout: 0 }).puppeteerTimeout).toBe(0)
+    })
+
+    it('returns the default timeout 30000ms if not specified', () => {
+      expect(instance().puppeteerTimeout).toBe(30000)
+    })
+  })
+
   describe('#convert', () => {
     const md = '# <i>Hello!</i>'
     const dummyFile = new File(process.cwd())
@@ -518,6 +530,19 @@ describe('Converter', () => {
           )
         })
       })
+
+      describe('with custom puppeteer timeout', () => {
+        it('follows setting timeout', async () => {
+          ;(fs as any).__mockWriteFile()
+
+          await expect(
+            pdfInstance({
+              output: 'test.pdf',
+              puppeteerTimeout: 1,
+            }).convertFile(new File(onePath))
+          ).rejects.toThrow(TimeoutError)
+        })
+      })
     })
 
     describe('when convert type is PPTX', () => {

diff --git a/test/marp-cli.ts b/test/marp-cli.ts
@@ -920,6 +920,30 @@ describe('Marp CLI', () => {
         )
       })
     })
+
+    describe('with PUPPETEER_TIMEOUT env', () => {
+      beforeEach(() => {
+        process.env.PUPPETEER_TIMEOUT = '12345'
+      })
+
+      afterEach(() => {
+        delete process.env.PUPPETEER_TIMEOUT
+      })
+
+      it('follows specified timeout in conversion that is using Puppeteer', async () => {
+        expect(
+          (await conversion(onePath, '--pdf')).options.puppeteerTimeout
+        ).toBe(12345)
+      })
+
+      it('does not follows specified timeout if the env value is not valid number', async () => {
+        process.env.PUPPETEER_TIMEOUT = 'invalid'
+
+        expect(
+          (await conversion(onePath, '--pdf')).options.puppeteerTimeout
+        ).toBeUndefined()
+      })
+    })
   })
 
   describe('with passing directory', () => {