deps: upgrade puppeteer to 21.7.0 #15724
Conversation
| @@ -540,7 +540,8 @@ function waitForUserToContinue(driver) { | |||
| } | |||
| /* c8 ignore stop */ | |||
|
|
|||
| driver.defaultSession.setNextProtocolTimeout(2 ** 31 - 1); | |||
| // Do "- 51" instead of "- 1" because we always add 50ms for the Puppeteer timeout | |||
| driver.defaultSession.setNextProtocolTimeout(2 ** 31 - 51); | |||
There was a problem hiding this comment.
this timeout is a month, why did you bother reducing it by 50 ms? :P
It was likely set to this number before just to be the max value accepted by the protocol.
this should probably just be the same "big" value as the other one.
There was a problem hiding this comment.
Sorry jinx on the PR update.
This one actually makes sense to be the max int32 since the users manually decides when to continue. I can imagine plenty of scenarios where I want to inspect the page for longer than 5 min.
| * | ||
| * This is ~50 days which is as good as infinity for all practical purposes. | ||
| */ | ||
| const MAX_TIMEOUT = (2 ** 32 - 1) - PPTR_BUFFER; | ||
|
|
There was a problem hiding this comment.
(my previous comment still applies here - why the buffer)
There was a problem hiding this comment.
Is it because of test fake timers shenanigans?
There was a problem hiding this comment.
Puppeteer timeouts need to fit into an int32. We add PPTR_BUFFER to the timeout later on so this is actually the largest number the user can provide without breaking Puppeteer.
Now that I think about it, we should probably make the condition for using MAX_TIMEOUT to be timeout > MAX_TIMEOUT not Number.isFinite(timeout)
There was a problem hiding this comment.
thanks for explaining. those constraints (esp the "we add this number later") should be clarified in a comment. Or, we can just cap to max int just before giving to puppeteer (in which case we could just use infinity here)
currently it's minusing a small magic constant from a number that is a really long time, so it comes off as unnecessary precision (50 days vs 50 days minus a 20th a second...)
https://github.com/puppeteer/puppeteer/releases/tag/puppeteer-core-v21.7.0
Also closes #15510