diff --git a/python/rustwright/sync_api.py b/python/rustwright/sync_api.py index ccc7102..77219cc 100644 --- a/python/rustwright/sync_api.py +++ b/python/rustwright/sync_api.py @@ -4607,6 +4607,19 @@ def _sleep_until_next_poll(deadline: float, interval: float = 0.02) -> None: time.sleep(min(interval, remaining)) +def _actionability_probe_timeout(remaining_ms: float, timeout_disabled: bool = False) -> float: + # Budget a single actionability/state probe with the full remaining action time rather + # than a small fixed cap. A cap shorter than one remote-CDP round trip made every probe + # time out and abort the action while the outer deadline was still far away (observed on + # click/select_option over connect_over_cdp attach). Polling cadence is driven by + # _sleep_until_next_poll, not this bound, so widening it does not busy-loop. For an + # infinite wait (timeout<=0) keep a bounded cadence so owner-availability and locator + # handlers are still re-checked periodically. + if timeout_disabled: + return min(remaining_ms, 1_000.0) + return max(remaining_ms, 1.0) + + def _handle_event_wait_timeout(owner: Any, event: str, timeout_display: str, deadline: Optional[float]) -> None: if owner is not None: _raise_if_owner_unavailable(owner, use_close_reason=True) @@ -15077,6 +15090,8 @@ def goto( self._uses_single_process_fallback() and target_url.lower().startswith("chrome://crash") ) try: + lifecycle_timeout = navigation_timeout if navigation_timeout > 0 else 24 * 60 * 60 * 1000 + deadline = time.monotonic() + (lifecycle_timeout / 1000) self._retain_navigation_response_bodies() self._mark_navigation_history_boundary() self._set_content_html_document_known = None @@ -15092,7 +15107,7 @@ def goto( "Page.goto", self._core.goto, target_url, - normalized_state, + "commit", navigation_timeout, normalized_referer, )) @@ -15131,6 +15146,13 @@ def goto( self._trace_end_action(call_id, result={"response": {"url": response.url, "status": response.status}}) return self._remember_navigation_response(response) raise + if normalized_state != "commit": + remaining = max(1.0, (deadline - time.monotonic()) * 1000) + self._wait_for_document_state( + normalized_state, + timeout=remaining, + timeout_label="navigation", + ) if payload is None or target_url.lower().startswith(("about:", "data:")): if self._context is not None: self._context._apply_storage_state_to_page(self) @@ -15210,6 +15232,12 @@ def _wait_for_document_state( prior_time_origin: Any = None, timeout_label: str = "document", ) -> None: + # Readiness on remote-CDP-attached pages is observed by polling document.readyState via + # Runtime.evaluate. The Page lifecycle events (Page.loadEventFired/domContentEventFired) + # are unreliable here: after set_content's document.write() on an already-loaded page they + # never re-fire, so an event wait would hang. Each poll gets the full remaining budget + # (a high-latency remote-CDP round trip can exceed a fixed short slice), and the single + # caller deadline bounds the whole wait. state = str(wait_until or "load") if timeout <= 0: timeout = 24 * 60 * 60 * 1000 @@ -15226,7 +15254,7 @@ def _wait_for_document_state( && Array.from(document.querySelectorAll('link[rel~="stylesheet"]')).every((link) => !!link.sheet) })""", None, - min(250.0, remaining), + remaining, ))) except Error: time.sleep(0.02) @@ -16591,7 +16619,7 @@ def set_content( self._core.evaluate, "() => document.contentType", None, - timeout_ms, + max(1.0, (deadline - time.monotonic()) * 1000), ) content_type = _decode_json_result(json.loads(content_type_result)) except Exception: @@ -16601,26 +16629,39 @@ def set_content( "text/html", "application/xhtml+xml", } - if isinstance(content_type, str) and content_type.lower() not in {"text/html", "application/xhtml+xml"}: - _call_with_method_prefix( - "Page.set_content", - self._core.evaluate, - """html => { - document.open(); - document.write(html); - document.close(); - }""", - json.dumps(html), - timeout_ms, - ) - self._set_content_html_document_known = False - else: - _call_wait_with_playwright_timeout("Page.set_content", self._core.set_content, html, timeout_ms) - self._set_content_html_document_known = True + # Materialize the document via document.open/write/close through Runtime.evaluate for + # every content type, including the real text/html attach path. This is the mechanism + # the remote-CDP attach probe proves reliable (case c5); the native command path is not + # used for the materialize-and-wait flow because its post-write lifecycle observation + # hangs on connect_over_cdp-attached pages. window.stop() halts any in-flight load first, + # matching Playwright. Stealth defaults re-apply automatically via the registered + # addScriptToEvaluateOnNewDocument script when document.write() creates the new document, + # so no separate injection is needed. The HTML is transported as a JSON-encoded evaluate + # argument, preserving escaping, and every step is bounded by the single caller deadline. + _call_with_method_prefix( + "Page.set_content", + self._core.evaluate, + """html => { + try { window.stop(); } catch (error) {} + document.open(); + document.write(html); + document.close(); + }""", + json.dumps(html), + max(1.0, (deadline - time.monotonic()) * 1000), + ) + self._set_content_html_document_known = not ( + isinstance(content_type, str) + and content_type.lower() not in {"text/html", "application/xhtml+xml"} + ) if normalized_state != "commit": remaining = max(1.0, (deadline - time.monotonic()) * 1000) try: - self._wait_for_document_state(normalized_state, timeout=remaining, timeout_label="set_content") + self._wait_for_document_state( + normalized_state, + timeout=remaining, + timeout_label="set_content", + ) except TimeoutError: raise _method_timeout_error("Page.set_content", timeout_ms) from None self._slow_mo() @@ -21785,16 +21826,26 @@ def _wait_for_single( while True: _raise_if_owner_unavailable(self._page, method=method) remaining_ms = max((deadline - time.monotonic()) * 1000, 1.0) - command_timeout = min(1_000.0 if timeout_disabled else max(timeout_ms, 1.0), remaining_ms, 1_000.0) + command_timeout = _actionability_probe_timeout(remaining_ms, timeout_disabled) self._page._run_locator_handlers(deadline) - last_info = self._target_state( - command_timeout, - scroll=scroll or state in {"actionable", "scrollable"}, - stable=requires_stable, - receives_events=state == "actionable", - stable_position_required=stable_position_required, - action_position=action_position, - ) + try: + last_info = self._target_state( + command_timeout, + scroll=scroll or state in {"actionable", "scrollable"}, + stable=requires_stable, + receives_events=state == "actionable", + stable_position_required=stable_position_required, + action_position=action_position, + ) + except TimeoutError: + # A single actionability probe that exceeds its own budget is transient, + # not fatal: over a high-latency remote-CDP transport one probe's round + # trips can outlast a small fixed cap. Keep polling until the outer + # deadline instead of aborting the whole action (matches Playwright). + if time.monotonic() >= deadline: + break + _sleep_until_next_poll(deadline) + continue if self._strict and not self._explicit_index: self._raise_frame_strict_violation(last_info.get("frame_strict_violation")) count = int(last_info.get("count") or 0) @@ -21857,9 +21908,17 @@ def _wait_for_fill_ready(self, action: str, *, timeout: Optional[float] = None) while True: _raise_if_owner_unavailable(self._page, method=method) remaining_ms = max((deadline - time.monotonic()) * 1000, 1.0) - command_timeout = min(max(timeout_ms, 1.0), remaining_ms, 1_000.0) + command_timeout = _actionability_probe_timeout(remaining_ms) self._page._run_locator_handlers(deadline) - last_info = self._target_state(command_timeout) + try: + last_info = self._target_state(command_timeout) + except TimeoutError: + # A single fill-readiness probe timing out is transient over a slow + # remote-CDP transport: keep polling until the outer deadline. + if time.monotonic() >= deadline: + break + _sleep_until_next_poll(deadline) + continue if self._strict and not self._explicit_index: self._raise_frame_strict_violation(last_info.get("frame_strict_violation")) count = int(last_info.get("count") or 0) @@ -23614,7 +23673,7 @@ def _fill(self, value: str, *, action: str, timeout: Optional[float] = None, for last_info: dict[str, Any] = {} while True: remaining_ms = max((deadline - time.monotonic()) * 1000, 1.0) - command_timeout = min(max(timeout_ms, 1.0), remaining_ms, 1_000.0) + command_timeout = _actionability_probe_timeout(remaining_ms) self._page._run_locator_handlers(deadline) try: result = self._eval(fill_script, command_timeout) @@ -24397,7 +24456,7 @@ def _select_option_impl( deadline = started + max(timeout_ms, 0.0) / 1000 while True: remaining_ms = max((deadline - time.monotonic()) * 1000, 1.0) - command_timeout = min(max(timeout_ms, 1.0), remaining_ms, 1_000.0) + command_timeout = _actionability_probe_timeout(remaining_ms) try: result = self._eval(select_script, command_timeout, method=method) except TimeoutError: diff --git a/src/lib.rs b/src/lib.rs index fdc2509..85f3465 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4174,27 +4174,6 @@ fn resolve_cached_main_frame_url( } impl PageInner { - async fn main_frame_id( - &self, - client: &CdpClient, - session_id: &str, - timeout: Duration, - ) -> RwResult { - if let Some(frame_id) = self.main_frame_id.lock().unwrap().clone() { - return Ok(frame_id); - } - let frame_tree = client - .send("Page.getFrameTree", json!({}), Some(session_id), timeout) - .await?; - let frame_id = frame_tree - .pointer("/frameTree/frame/id") - .and_then(Value::as_str) - .ok_or_else(|| RwError::Message("CDP did not return a main frame id".to_string()))? - .to_string(); - *self.main_frame_id.lock().unwrap() = Some(frame_id.clone()); - Ok(frame_id) - } - fn session_for_frame_id(&self, frame_id: &str) -> String { self.frame_state .lock() @@ -8096,14 +8075,25 @@ return win.__rustwrightCleanupDrag ? win.__rustwrightCleanupDrag() : false; let session_id = page.session_id.clone(); browser .block_on(async move { - let frame_id = page.main_frame_id(&client, &session_id, timeout).await?; - let frame_id_json = serde_json::to_string(&frame_id)?; let html_json = serde_json::to_string(&html)?; - let params_json = format!("{{\"frameId\":{frame_id_json},\"html\":{html_json}}}"); + // Materialize the document via document.open/write/close through + // Runtime.evaluate rather than the Page.setDocumentContent command. + // Page.setDocumentContent deterministically hangs on + // connect_over_cdp-attached pages (the browser never returns the command + // result, so the call blocks until timeout), while Runtime.evaluate is + // reliable over the same remote-CDP transport. This mirrors how Playwright + // sets content, and delivers the byte-identical document. + let materialize = format!( + "(() => {{ try {{ window.stop(); }} catch (e) {{}} document.open(); document.write({html_json}); document.close(); }})()" + ); client - .send_raw_params_json( - "Page.setDocumentContent", - params_json, + .send( + "Runtime.evaluate", + json!({ + "expression": materialize, + "awaitPromise": false, + "returnByValue": true, + }), Some(&session_id), timeout, ) diff --git a/tests/test_rustwright_sync_api.py b/tests/test_rustwright_sync_api.py index 940d1b8..37e680a 100644 --- a/tests/test_rustwright_sync_api.py +++ b/tests/test_rustwright_sync_api.py @@ -2083,6 +2083,158 @@ def test_chromium_connect_over_cdp_adopts_default_context_pages_and_disconnects_ launched.close() +def test_connect_over_cdp_set_content_on_adopted_and_new_pages(playwright): + # Regression: Page.set_content() over a connect_over_cdp attach must materialize the + # document without relying on the Page.setDocumentContent command, which hangs on + # remote-CDP-attached pages (the browser never returns the command result, so the call + # blocks until timeout on both reused and freshly-created pages). set_content now uses + # document.open/write/close through Runtime.evaluate, matching Playwright, and delivers + # the byte-identical document over the same transport. + launched = playwright.chromium.launch(headless=True) + connected = None + try: + origin = launched.new_page() + origin.goto(data_url("Origin

origin

")) + + connected = playwright.chromium.connect_over_cdp(launched._ws_endpoint) + context = connected.contexts[0] + + html = ( + "Adopted SC" + "

Header

" + "" + "" + "" + ) + + # Reused/adopted page (probe c1 path) with wait_until="load". + adopted = next(page for page in context.pages if page.url == origin.url) + adopted.set_content(html, wait_until="load", timeout=5_000) + assert adopted.title() == "Adopted SC" + assert adopted.locator("#hdr").text_content() == "Header" + assert adopted.select_option("#sel", "y") == ["y"] + assert adopted.frame_locator("#frm").locator("#m").text_content() == "frame-hello" + + # Reused page with the weaker lifecycle state (probe c2 path). + adopted.set_content(html, wait_until="domcontentloaded", timeout=5_000) + assert adopted.title() == "Adopted SC" + + # Freshly created page with wait_until="load" (probe c3 path). + fresh = context.new_page() + fresh.set_content(html, wait_until="load", timeout=5_000) + assert fresh.title() == "Adopted SC" + assert fresh.frame_locator("#frm").locator("#m").text_content() == "frame-hello" + + # Deterministic data-URL navigation on the reused page (probe c4 shape). + adopted.goto(data_url(html), wait_until="load", timeout=5_000) + assert adopted.title() == "Adopted SC" + assert adopted.locator("#hdr").text_content() == "Header" + finally: + if connected is not None: + connected.close() + launched.close() + + +def test_document_state_fallback_uses_remaining_budget_for_high_latency_attach(): + from rustwright.sync_api import Page + + class HighLatencyCore: + def __init__(self): + self.timeouts = [] + + def evaluate(self, expression, arg, timeout): + self.timeouts.append(timeout) + if timeout < 300: + raise TimeoutError("remote CDP round trip exceeded command slice") + return json.dumps({"readyState": "complete", "resourcesLoaded": True}) + + owner = Page.__new__(Page) + owner._core = HighLatencyCore() + + owner._wait_for_document_state("load", timeout=700) + + assert len(owner._core.timeouts) == 1 + assert owner._core.timeouts[0] > 600 + + +def test_set_content_text_html_materializes_via_evaluate_not_native_command(): + # Regression (RUS-5, third cycle): on a remote-CDP *attached* text/html page, Page.set_content + # must materialize the document via Runtime.evaluate(document.open/write/close) — the mechanism + # the attach probe proves is reliable (case c5) — and must NOT route the real text/html branch + # through the native command path nor stack an unbounded lifecycle-event wait. After + # document.write() on an already-loaded page the load/domcontentloaded CDP lifecycle events do + # not re-fire, so readiness is confirmed by polling document.readyState, and the whole operation + # is bounded by a single caller deadline. + from rustwright.sync_api import Page + + class RecordingCore: + def __init__(self): + self.evaluate_calls = [] + self.set_content_calls = [] + self.event_waiters = 0 + + def evaluate(self, expression, arg, timeout): + self.evaluate_calls.append((expression, arg, timeout)) + if "document.contentType" in expression: + return json.dumps("text/html") + if "document.write" in expression: + return json.dumps(None) + if "readyState" in expression: + return json.dumps({"readyState": "complete", "resourcesLoaded": True}) + return json.dumps(None) + + def set_content(self, html, timeout): + self.set_content_calls.append((html, timeout)) + + def cdp_session(self): + outer = self + + class _Waiter: + def wait(self, timeout): + raise TimeoutError("no lifecycle event refires after document.write on attach") + + class _Session: + def event_waiter(self, method): + outer.event_waiters += 1 + return _Waiter() + + return _Session() + + html = ( + "Attached" + "

Header

" + ) + + owner = Page.__new__(Page) + owner._core = RecordingCore() + owner._context = None + owner._slow_mo_ms = 0 + owner._set_content_html_document_known = None + owner._mark_request_cookie_sync_required = lambda: None + owner._trace_begin_action = lambda *args, **kwargs: None + owner._trace_end_action = lambda *args, **kwargs: None + owner._slow_mo = lambda: None + + owner.set_content(html, wait_until="load", timeout=5_000) + + # The text/html attach path must not hang on the native command; it materializes via evaluate. + assert owner._core.set_content_calls == [] + materialize = [call for call in owner._core.evaluate_calls if "document.write" in call[0]] + assert len(materialize) == 1, owner._core.evaluate_calls + expression, arg, materialize_timeout = materialize[0] + assert "document.open()" in expression + assert "document.close()" in expression + # HTML is transported as a JSON-encoded evaluate argument (escaping preserved), not interpolated. + assert json.loads(arg) == html + # A single caller deadline bounds the whole operation: the materialize evaluate receives a + # remaining-bounded timeout, never a fresh full budget stacked on top of the content-type probe. + assert materialize_timeout <= 5_000 + # The unreliable Page lifecycle-event waiter is gone (it never re-fires after document.write). + assert owner._core.event_waiters == 0 + # The document was materialized (evaluate) and readiness confirmed via the readyState poll. + assert any("readyState" in call[0] for call in owner._core.evaluate_calls) + + def test_connect_over_cdp_context_request_syncs_remote_cookies_for_download(playwright, http_server): launched = playwright.chromium.launch(headless=True) connected = None @@ -10177,6 +10329,41 @@ def test_locator_click_waits_for_delayed_element(page): assert page.evaluate("document.body.dataset.clicked") == "yes" +def test_actionability_probe_tolerates_slow_remote_cdp_round_trips(page, monkeypatch): + # Regression for remote-CDP interaction timeouts (connect_over_cdp attach): over a + # high-latency transport a single actionability probe's CDP round-trips can outlast the + # per-probe budget. The action must keep polling until the outer deadline instead of + # aborting on that transient probe timeout. Before the fix, click()/select_option() + # failed in ~1s with a raw "timed out after 1000 ms" (the old fixed per-probe cap) even + # though the outer timeout was 30s, so 13/60 remote-CDP interactions timed out. + from rustwright.sync_api import Locator + + page.set_content( + "" + "" + "before" + ) + + real_target_state = Locator._target_state + probe_calls = {"count": 0} + + def slow_probe_target_state(self, timeout=None, **kwargs): + probe_calls["count"] += 1 + # Model a probe whose round-trips need more than the old 1000ms cap to complete. + if timeout is not None and timeout < 1_500.0: + raise TimeoutError(f"timed out after {int(timeout)} ms") + return real_target_state(self, timeout, **kwargs) + + monkeypatch.setattr(Locator, "_target_state", slow_probe_target_state) + + # With the outer timeout at 30s, each probe must be granted well over 1500ms, so the + # interaction completes rather than dying on the first under-budgeted probe. + page.click("#mutate", timeout=30_000) + assert page.locator("#state").text_content() == "after" + assert page.select_option("#sel", "b", timeout=30_000) == ["b"] + assert probe_calls["count"] > 0 + + def test_locator_fill_waits_for_delayed_editable(page): page.set_content( """