mirror of
https://github.com/dgtlmoon/changedetection.io.git
synced 2026-09-30 01:05:53 +00:00
Puppeteer fetcher - Re-navigation cap must not mean "extract right now" (#4439)
BROWSER_CONTENT_READY_MAX_RESETS exists so a page that re-navigates in a loop cannot extend a fetch forever. On hitting that cap the content-ready wait broke straight out into Page.stopLoading and extraction, giving the document we end up on zero settle time - the exact opposite of what the wait is for. Measured against a page that hops every 500ms and then renders via JS 2s after the final load: before: 2.7s, 130 bytes of an intermediate hop, no final document, no JS-rendered content after: 14.1s, final document, JS-rendered content present It also logged "Content-ready wait of 12s elapsed" immediately before extracting, having waited 0s, which is why this reads as a fetcher that ignores the setting. Note 0.60.4 could not do this: its wait was an unconditional `await asyncio.sleep(1 + extra_wait)` after goto(), so every fetch got its settle time no matter how the page behaved. Now the cap stops the wait from being *restarted*, and the delay is spent one final time before extracting. Total stays bounded at (max_resets + 2) * extra_wait, and whatever we extract has had the same settle time every other fetch gets. The stopLoading log line no longer claims a wait that may not have happened. Unrelated to #4437 - found while reading #4426 for that investigation, which turned out to be a locale/collation bug in the filter layer, not a fetcher problem. Tested: new reset-cap probe checked to fail before and pass after; normal (non-re-navigating) path unchanged at 12.5s with content intact; 3 passed browser fetcher suite on pyppeteer, 323 unit tests pass. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
b0783bce90
commit
d56ec682fe
@@ -443,7 +443,21 @@ class fetcher(Fetcher):
|
||||
# Main frame started a new document
|
||||
resets += 1
|
||||
if resets > max_content_ready_resets:
|
||||
logger.debug(f"Main frame keeps re-navigating, not restarting the content-ready wait again")
|
||||
# The cap is there to stop a page that re-navigates in a loop from
|
||||
# extending the fetch forever - it is NOT permission to extract
|
||||
# immediately. Breaking straight out here landed on whatever document
|
||||
# happened to be mid-flight, with zero settle time: measured against a
|
||||
# page that hops every 500ms, the fetch ended after 2.7s holding 130
|
||||
# bytes of an intermediate hop, no final document and no JS-rendered
|
||||
# content, while logging "content-ready wait of 12s elapsed".
|
||||
#
|
||||
# So spend the delay one last time, just without arming another reset.
|
||||
# Total stays bounded at (max_resets + 2) * extra_wait, and whatever we
|
||||
# extract has had the same settle time every other fetch gets.
|
||||
logger.debug(f"Main frame re-navigated {resets} times (cap "
|
||||
f"{max_content_ready_resets}), waiting {extra_wait}s once "
|
||||
f"more without restarting, then extracting regardless")
|
||||
await asyncio.sleep(extra_wait)
|
||||
break
|
||||
logger.debug(f"Main frame started a new document, restarting the {extra_wait}s "
|
||||
f"content-ready wait ({resets}/{max_content_ready_resets})")
|
||||
@@ -456,7 +470,7 @@ class fetcher(Fetcher):
|
||||
# Stop whatever is still in flight so the DOM and screenshot come from what rendered,
|
||||
# rather than waiting on a subresource that may never answer
|
||||
try:
|
||||
logger.debug(f"Content-ready wait of {extra_wait}s elapsed, issuing Page.stopLoading before extracting")
|
||||
logger.debug(f"Content-ready wait finished, issuing Page.stopLoading before extracting")
|
||||
await self.page._client.send('Page.stopLoading')
|
||||
logger.debug("stopLoading command sent!")
|
||||
|
||||
|
||||
Reference in New Issue
Block a user