From 80dc9fc2721aa85c0036dc52ebd6355ed363d904 Mon Sep 17 00:00:00 2001 From: ThePhaseless Date: Fri, 9 Oct 2026 00:03:10 +0200 Subject: [PATCH] refactor: read the settled page once, without a retry The retry in read_settled_html covered page.content() racing a navigation. #418 only showed that race on the early read taken right after domcontentloaded, and this branch already removes that read. The late read now matches the Cloudflare path, which reads after the idle wait and has never retried. When the retry fires at the end of the budget, remaining_ms() is 1 ms. The second idle wait then times out at once, and the second read lands in the same 60-150 ms window that failed. So the retry adds a helper and threads timer into content.py for a case it does not cover. Co-Authored-By: Claude Opus 5.5 --- src/content.py | 18 ++---------------- src/endpoints.py | 1 - tests/main_test.py | 14 -------------- 3 files changed, 2 insertions(+), 31 deletions(-) diff --git a/src/content.py b/src/content.py index 81a67d9..8745e39 100644 --- a/src/content.py +++ b/src/content.py @@ -1,18 +1,15 @@ import base64 -from contextlib import suppress -from invisible_playwright.async_api import Error from playwright.async_api import Page from src.models import LinkRequest -from src.utils import TimeoutTimer, logger, remaining_ms +from src.utils import logger async def build_response_content( page: Page, request: LinkRequest, page_request: object, - timer: TimeoutTimer, ) -> tuple[str, str]: """Build (content_type, response_content) from the settled page.""" if request.return_only_cookies: @@ -23,18 +20,7 @@ async def build_response_content( ): return await fetch_pdf_content(page) - return "text/html", await read_settled_html(page, timer) - - -async def read_settled_html(page: Page, timer: TimeoutTimer) -> str: - """Read the page's HTML, reading it again if a navigation pulled it away.""" - try: - return await page.content() - except Error: - logger.info("Page navigated while being read; reading it again once idle") - with suppress(Error): - await page.wait_for_load_state("networkidle", timeout=remaining_ms(timer)) - return await page.content() + return "text/html", await page.content() async def fetch_pdf_content(page: Page) -> tuple[str, str]: diff --git a/src/endpoints.py b/src/endpoints.py index 6663707..bd68b4f 100644 --- a/src/endpoints.py +++ b/src/endpoints.py @@ -88,7 +88,6 @@ async def read_item(request: LinkRequest, dep: BrowserDep) -> LinkResponse: dep.page, request, page_request, - timer, ) user_agent = ( diff --git a/tests/main_test.py b/tests/main_test.py index a5ff17f..4cd41d6 100644 --- a/tests/main_test.py +++ b/tests/main_test.py @@ -225,20 +225,6 @@ async def test_html_is_read_after_a_js_redirect_lands(): assert response.solution.response == "Article" -@pytest.mark.asyncio -async def test_html_read_racing_a_navigation_is_retried(): - """A read that loses its document to a navigation reads the new one instead.""" - dep = fake_dep() - dep.page.content.side_effect = [ - PlaywrightError("Failed to find execution context with id = id-5"), - "Article", - ] - - response = await read_item(LinkRequest(url="https://example.test/login"), dep) - - assert response.solution.response == "Article" - - @pytest.mark.asyncio async def test_domcontentloaded_timeout_returns_408(): """Fatal timeouts during initial page load still return a controlled 408."""