Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resolves #633 Introduce PUPPETEER_PRINT_TIMEOUT for puppeteer pdf rendering #632

Merged

Conversation

siaccarino
Copy link
Contributor

@siaccarino siaccarino commented Mar 9, 2022

The pull request #236 did not change the puppeteer rendering timeout - it changes the time asciidoc-web-pdf waits for puppeteer.

According to puppeteer pdf options a timeout shall be passed to the page.pdf() call instead.

Rendering of complex documents on slow machines could easily exceed the 30s limit.

Solution: Introduce a PUPPETEER_PRINT_TIMEOUT environment variable that is passed to puppeteer - see puppeteer pdf timeout option.

Issue: #633

lib/browser.js Outdated Show resolved Hide resolved
lib/converter.js Outdated Show resolved Hide resolved
@@ -89,10 +89,14 @@ async function convert (processor, inputFile, options, timings, watch, preview)
}
try {
const page = await browserInstance.goto(`file://${tempFile}`, preview)
const puppeteerDefaultTimeout = process.env.PUPPETEER_DEFAULT_TIMEOUT
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be taken from browser instance but in terms of dependency reduction this might be better

@siaccarino siaccarino changed the title Fix the pull request #236 Introduce PUPPETEER_PRINT_TIMEOUT for puppeteer pdf rendering Mar 10, 2022
@siaccarino siaccarino changed the title Introduce PUPPETEER_PRINT_TIMEOUT for puppeteer pdf rendering resulves #633 Introduce PUPPETEER_PRINT_TIMEOUT for puppeteer pdf rendering Mar 10, 2022
@siaccarino siaccarino changed the title resulves #633 Introduce PUPPETEER_PRINT_TIMEOUT for puppeteer pdf rendering resolves #633 Introduce PUPPETEER_PRINT_TIMEOUT for puppeteer pdf rendering Mar 10, 2022
@ggrossetie
Copy link
Owner

Looks good!
One last thing, could you please update the console.log message to include this new environment variable:

> TIP: You can configure the timeout in milliseconds using PUPPETEER_DEFAULT_TIMEOUT, PUPPETEER_NAVIGATION_TIMEOUT, PUPPETEER_RENDERING_TIMEOUT or PUPPETEER_PRINT_TIMEOUT environment variables.

As mentioned in #612 (comment) it would be better to create a separate 'TIP' for each timeout but for now it's good enough. We can revisit this logic later in a follow-up pull request.

Copy link
Owner

@ggrossetie ggrossetie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good, thanks!

@ggrossetie ggrossetie merged commit 643d554 into ggrossetie:main Mar 10, 2022
@siaccarino siaccarino deleted the Fix-puppeteer-timeout-configuration branch February 23, 2023 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants