feat: support timeout for page.pdf() call

[nikli2009]

Context

When use page.pdf API, there are some extreme scenarios that can cause the promise never get fulfilled or rejected. Hence this may cause potential problems. These kind of errors most likely happens in Chromium.

Related Issues

• PDF rendering hangs unless page contains the same custom font as header #6834
• Inspector.targetCrashed with custom fonts on footerTemplate in createPdf #7496
• Puppeteer hangs after page crash due to OOM #5846

Changes

• Added timeout parameter for PDFOption which will be used in page.pdf call

[google-cla]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

---

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

[nikli2009]

@googlebot I signed it!

@googlebot I signed it!

[jschfflr]

Can you move to timeout to createPDFStream as it looks like the Page.printToPDF call there is the original source of the timeout?

[jschfflr]

Do you know if there's a corresponding bug reported for Chromium already? And if not, file one on https://crbug.com/new?
That way, we can make sure that we can also address the problem at its source. Thanks!

[nikli2009]

Can you move to timeout to createPDFStream as it looks like the Page.printToPDF call there is the original source of the timeout?

Sounds good ! Thanks for the review

[nikli2009]

Do you know if there's a corresponding bug reported for Chromium already? And if not, file one on https://crbug.com/new?
That way, we can make sure that we can also address the problem at its source. Thanks!

np, will check on that .

[nikli2009]

☑️ Issue created for Chromium

[jschfflr]

Awesome, thanks!
As we wanna make sure that the timeout still works when the bug is eventually fixed in Chromium, the tests should be updated to just use a timeout of 1 and make sure that the call times out. That's how we test the other time outs too.
As for the test that you already wrote: It's a great reproduction that you can maybe put in a gist and reference it in the Chromium bug. That will help the engineer who'll be resolving the issue there a ton!

[nikli2009]

Awesome, thanks!
As we wanna make sure that the timeout still works when the bug is eventually fixed in Chromium, the tests should be updated to just use a timeout of 1 and make sure that the call times out. That's how we test the other time outs too.
As for the test that you already wrote: It's a great reproduction that you can maybe put in a gist and reference it in the Chromium bug. That will help the engineer who'll be resolving the issue there a ton!

Yep, the timeout values in tests are 1.
gist has been created and linked to the Chromium issue :D

Let me know if there are other things I could help with and thanks for taking time review my PR 😄

[jschfflr]

@nikli2009 Thanks for updating the pull request! I left two more comments to reduce the size of the pr a bit - could you address them too?

[nikli2009]

@nikli2009 Thanks for updating the pull request! I left two more comments to reduce the size of the pr a bit - could you address them too?

Happy to :D

[nikli2009]

@nikli2009 Thanks for updating the pull request! I left two more comments to reduce the size of the pr a bit - could you address them too?

@jschfflr Thanks for the review :D, got some quick updates :

1. After some manual tests, it seems that the printToPDF call only hangs when using base64(valid base64 tho) and external url at same time.
2. Removed those problematic headerTemplate config in the tests. I realized that as we set the timeout to be 1 ms and it will just always timeout (even when printToPDF being executed normally).

[jschfflr]

Ah yeah, right. Good catch!