mirror of
https://github.com/dgtlmoon/changedetection.io.git
synced 2026-10-15 08:19:50 +00:00
Puppeteer fetcher - One content-ready deadline instead of a stopLoading watchdog per frame event
Page.stopLoading is what stops a page that would otherwise load forever waiting on a subresource
that never answers, so that we can still screenshot and scrape what rendered. That intent was
right, but it was implemented as a fire-and-forget task armed by every frame event, which measured
on a single fetch of an iframe-heavy page came to:
14 watchdog tasks spawned
11 page-wide Page.stopLoading calls
3 tasks outliving the fetch and firing against a closed page
Page.stopLoading takes no frame or loader argument - it is the Stop button, and it stops the whole
page. Verified directly: one call stopped a pending main frame and a pending iframe in the same
instant. So the other 10 calls were redundant, and because they landed at arbitrary later times
they could stop a *subsequent* navigation we actually wanted - which is the likeliest reason the
same URL fetched in 8s on one run and 35s on the next.
Replaced with a single deadline, awaited inline so nothing can outlive the fetch (there is no
create_task left in this file at all):
navigate (bounded) -> wait the configured delay -> Page.stopLoading -> extract
The delay is measured from when navigation finished, not from when it started. Anchoring it to the
start would quietly rob a slow-loading page of its settle time, and letting JS-rendered content
appear after load is the whole point of the setting. Verified with a server that takes 5s to answer
and renders via JS 2s after load: total 9.6s for a 4s delay, and the late content is captured.
Because a page is never reliably "finished" - many sites navigate as part of their normal design -
the delay restarts when the MAIN frame replaces its document, so a redirect or interstitial gets
the same settle time the first document got. Iframes do not restart it, and it is capped by
BROWSER_CONTENT_READY_MAX_RESETS (default 2).
Only the existing "wait n seconds before extracting text" stays user-facing;
BROWSER_NAVIGATION_TIMEOUT_SECONDS is a safety net with a sane default rather than a second knob
for users to reason about. This matches what other scrapers do: bound the navigation, do not fail
when it times out, settle, then extract.
Timings are also more predictable now - the four reported URLs went from 5-35s of variance to
4.6-7.2s at a 3s delay, all with full content and a 200.
Tested: 11 passed pyppeteer browser suite, 7 passed playwright, 494 unit + llm.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
dgtlmoonandClaude Opus 5 committed
1 parent
e93e45d7cf
commit
8b23a35d3f
1 file changed
+56
-34
@@ -399,36 +399,59 @@ class fetcher(Fetcher):
|
||||
# Enable Network domain to detect when first bytes arrive
|
||||
await self.page._client.send('Network.enable')
|
||||
|
||||
# Now set up the frame navigation handlers
|
||||
async def handle_frame_navigation(event=None):
|
||||
# Wait n seconds after the frameStartedLoading, not from any frameStartedLoading/frameStartedNavigating
|
||||
logger.debug(f"Frame navigated: {event}")
|
||||
w = extra_wait - 2 if extra_wait > 4 else 2
|
||||
logger.debug(f"Waiting {w} seconds before calling Page.stopLoading...")
|
||||
await asyncio.sleep(w)
|
||||
# Navigate (bounded), then wait the configured "wait n seconds before extracting text"
|
||||
# delay, then stop whatever is still loading, then extract. The delay is measured from
|
||||
# when navigation finished, not from when it started, because the point of it is to let
|
||||
# JS-rendered content appear *after* load - anchoring it to the start would quietly give a
|
||||
# slow-loading page almost no settle time.
|
||||
#
|
||||
# Only that delay is a user-facing setting. The navigation bound above is a safety net with
|
||||
# a sane default, not a tuning knob, so there is still one number for users to think about.
|
||||
#
|
||||
# There is no way to know a page is "finished" - plenty of sites navigate as part of their
|
||||
# normal design, and some sit forever on a subresource that never answers. So the delay
|
||||
# restarts if the MAIN frame replaces its document (a redirect or interstitial gets the
|
||||
# same settle time the first document got), iframes do not restart it, and it is capped so
|
||||
# a page that re-navigates in a loop cannot extend it indefinitely.
|
||||
max_content_ready_resets = int(os.getenv("BROWSER_CONTENT_READY_MAX_RESETS", 2))
|
||||
|
||||
# Check if page still exists (might have been closed due to error during sleep)
|
||||
if not self.page or not hasattr(self.page, '_client'):
|
||||
logger.debug("Page already closed, skipping stopLoading")
|
||||
return
|
||||
async def wait_for_content_ready_then_stop_loading():
|
||||
main_frame_id = self.page.mainFrame._id
|
||||
renavigated = asyncio.Event()
|
||||
|
||||
logger.debug("Issuing stopLoading command...")
|
||||
await self.page._client.send('Page.stopLoading')
|
||||
logger.debug("stopLoading command sent!")
|
||||
def _on_main_frame_navigation(event):
|
||||
if event.get('frameId') == main_frame_id:
|
||||
renavigated.set()
|
||||
|
||||
async def setup_frame_handlers_on_first_response(event):
|
||||
# Only trigger for the main document response
|
||||
if event.get('type') == 'Document':
|
||||
logger.debug("First response received, setting up frame handlers for forced page stop load.")
|
||||
self.page._client.on('Page.frameStartedNavigating', lambda e: asyncio.create_task(handle_frame_navigation(e)))
|
||||
self.page._client.on('Page.frameStartedLoading', lambda e: asyncio.create_task(handle_frame_navigation(e)))
|
||||
self.page._client.on('Page.frameStoppedLoading', lambda e: logger.debug(f"Frame stopped loading: {e}"))
|
||||
logger.debug("First response received, setting up frame handlers for forced page stop load DONE SETUP")
|
||||
# De-register this listener - we only need it once
|
||||
self.page._client.remove_listener('Network.responseReceived', setup_frame_handlers_on_first_response)
|
||||
self.page._client.on('Page.frameStartedLoading', _on_main_frame_navigation)
|
||||
self.page._client.on('Page.frameStoppedLoading', lambda e: logger.debug(f"Frame stopped loading: {e}"))
|
||||
try:
|
||||
resets = 0
|
||||
while True:
|
||||
renavigated.clear()
|
||||
try:
|
||||
await asyncio.wait_for(renavigated.wait(), timeout=extra_wait)
|
||||
# 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")
|
||||
break
|
||||
logger.debug(f"Main frame started a new document, restarting the {extra_wait}s "
|
||||
f"content-ready wait ({resets}/{max_content_ready_resets})")
|
||||
except asyncio.TimeoutError:
|
||||
# Quiet for the whole delay - the page is as ready as it is going to get
|
||||
break
|
||||
finally:
|
||||
self.page._client.remove_listener('Page.frameStartedLoading', _on_main_frame_navigation)
|
||||
|
||||
# Listen for first response to trigger frame handler setup
|
||||
self.page._client.on('Network.responseReceived', setup_frame_handlers_on_first_response)
|
||||
# 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")
|
||||
await self.page._client.send('Page.stopLoading')
|
||||
logger.debug("stopLoading command sent!")
|
||||
except Exception as e:
|
||||
logger.debug(f"Page.stopLoading skipped, page is most likely already gone: {e}")
|
||||
|
||||
# Track the LATEST main-frame document response for the whole fetch, not just the one that
|
||||
# goto() happens to return. This app compares the text of the page the browser ends up on,
|
||||
@@ -461,7 +484,6 @@ class fetcher(Fetcher):
|
||||
try:
|
||||
while not response:
|
||||
logger.debug(f"Attempting page fetch {url} attempt {attempt}")
|
||||
asyncio.create_task(handle_frame_navigation())
|
||||
# Race goto() against the main frame actually firing 'load'. In the re-navigation
|
||||
# case goto() can never resolve, but the replacement document does fire 'load' -
|
||||
# usually within a few seconds - so this returns then instead of sitting out the
|
||||
@@ -505,11 +527,6 @@ class fetcher(Fetcher):
|
||||
for t in (goto_task, load_task):
|
||||
if not t.done():
|
||||
t.cancel()
|
||||
await asyncio.sleep(1 + extra_wait)
|
||||
# Check if page still exists before sending command
|
||||
if self.page and hasattr(self.page, '_client'):
|
||||
await self.page._client.send('Page.stopLoading')
|
||||
|
||||
if response:
|
||||
break
|
||||
if not response:
|
||||
@@ -519,8 +536,13 @@ class fetcher(Fetcher):
|
||||
raise EmptyReply(url=url, status_code=None)
|
||||
attempt+=1
|
||||
|
||||
# The sleep above is where a meta-refresh interstitial typically swaps in the real page,
|
||||
# so re-check which document we are actually on before judging the status code.
|
||||
# Navigation is done; now honour "wait n seconds before extracting text" and then
|
||||
# force-stop whatever is still loading, so extraction always gets what rendered.
|
||||
# Awaited inline rather than fired off as a task, so nothing can outlive the fetch.
|
||||
await wait_for_content_ready_then_stop_loading()
|
||||
|
||||
# That wait is where a meta-refresh interstitial typically swaps in the real page, so
|
||||
# re-check which document we are actually on before judging the status code.
|
||||
latest = navigation_response.get('response')
|
||||
if latest is not None and latest is not response:
|
||||
logger.debug(f"Page navigated again while waiting, judging the fetch on {latest.url} "
|
||||
|
||||
Reference in new issue
Block a user