From d56ec682fed93032127de45050867d15f765c97c Mon Sep 17 00:00:00 2001 From: dgtlmoon Date: Mon, 14 Sep 2026 20:31:48 +0200 Subject: [PATCH] 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) --- .../content_fetchers/puppeteer.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/changedetectionio/content_fetchers/puppeteer.py b/changedetectionio/content_fetchers/puppeteer.py index da5e8557c..f7e68e084 100644 --- a/changedetectionio/content_fetchers/puppeteer.py +++ b/changedetectionio/content_fetchers/puppeteer.py @@ -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!")