search

scullyio / scully

The Static Site Generator for Angular apps
https://scully.io/
MIT License
2.55k stars 257 forks source link

Add ability to tune / remove default puppeteer timeout #1427

Open lamp-of-god opened 3 years ago

lamp-of-god commented 3 years ago

🧩 Feature request

Description

Sometimes I need to wait for a very long time until page gets ready to process it. So I add manualIdle check, but after 25sec puppeteer processes the page, even if there were no signal that page is ready.

Describe the solution you'd like

Some configuration parameter which will allow to tune this timeout or disable it at all.

Also it would be great to add parameter which make puppeteer plugin throw an error and stops processing in case such timeout is reached instead of silently continue process.

SanderElias commented 3 years ago

There is a hard timeout indeed at 25 seconds, to prevent stalling the process. We will discuss making this a config option (in prior discussions we opted for no because 25 seconds is already a really long time and in our eyes an error that should be fixed) Do you have a compelling reason for the long time? That might help sway the discussion!

sergei-rus commented 3 years ago

@SanderElias thank you for response.

My usecase: I develop a frontend app, which fetch data from backend I have no control at. This app is built and deployed via CI. Sometimes back didn't respond for a given amount of time (stuck for unknown reason). Expected behavior there for me was either to wait for data (that why I wanted to disable timeout), or to throw an error and stop process at all (which is more preferable for me, because it is a really long time :). However, I get the pre-generated app with not inited pages silently, which causes to weird UX, SEO crawling issues, etc. So its better to not generate site at all (and keep previous version published) than to have partly inited one.

My point is that we should not suppress the problem with internal timeout (or at least should have an ability to manage it). So throwing error option solves the problem. Tuning timeout is a nice feature for me, but not that important than throwing an error: if I know correct server response time, why to wait more (up to 25 sec), if I can surely fail in 10sec? 5 sec?

lamp-of-god commented 3 years ago

Just noticed I left the comment from another acocunt. @sergei-rus It was me)

SanderElias commented 3 years ago

@lamp-of-god I just now noticed your response. That is a compelling use case. We will have another discussion about this. This will probably swing the discussion. I'll let you know in a little while.

jsanta commented 3 years ago

Remembers me of https://github.com/scullyio/scully/issues/1354 We faced the exact same problem as some pages were taking too long to render and we were also getting some timeout errors. Ended up copying the PuppeteerRenderPlugin as an enterprise plugin and changing most variables to a configurable attribute.

jakubzet commented 2 years ago

Hi! I wonder if changes applied in versions 2.1.36 and 2.1.41 somehow alters the ignored timeout config property? I see there were changes introduced to scully-plugin-puppeteer.

jsanta commented 2 years ago

As I saw recently, nothing has changed. Had to update my Scully configs and ended up hot patching the puppeteer plugin. Made a Gist for this: https://gist.github.com/jsanta/4adbcdb00409c81853c8b9241fe36ccc