Fix two remote-CDP defects: set_content hang and premature interaction timeouts#94
Closed
LawyZheng wants to merge 3 commits into
Closed
Fix two remote-CDP defects: set_content hang and premature interaction timeouts#94LawyZheng wants to merge 3 commits into
LawyZheng wants to merge 3 commits into
Conversation
…n timeouts Page.set_content() over connect_over_cdp deterministically hung: the Rust core issued the Page.setDocumentContent CDP command, which never returns on a remote-CDP-attached page (the browser does not send the command result, so the call blocks until timeout on reused and freshly-created pages alike). Evaluate and data-URL navigation succeed over the same attached session, and rustwright's response demux matches by command id, so the hang is the command itself. Materialize the document with document.open/write/close via Runtime.evaluate instead (as Playwright does); this delivers the byte-identical document over the transport the attach path is proven to support. Remove the now-unused PageInner::main_frame_id helper (set_content was its only caller). Locator actionability/state polling capped each CDP probe at a fixed 1000 ms and, in the actionability paths, did not catch a probe timeout. A single probe issues several CDP round trips; over a high-latency remote-CDP transport those exceed 1000 ms, so every probe timed out and the raw TimeoutError aborted the whole click/select_option/fill after ~0.3-0.4 s even though the outer 30 s deadline was far off. Give each probe the full remaining action budget (new helper _actionability_probe_timeout) and treat a per-probe timeout as a transient retry until the real deadline, matching Playwright. Polling cadence is still driven by _sleep_until_next_poll, so widening the bound does not busy-loop. Add regression tests: set_content on adopted and new connect_over_cdp pages, and an actionability probe that tolerates slow remote-CDP round trips (fails with the old 1000 ms cap, passes now). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…tach path Root cause of the remaining remote-CDP set_content/goto hang was the post-materialization lifecycle wait, not the materialization itself: - Page.set_content routed the actual text/html branch through a lifecycle wait that first peeked for a Page.loadEventFired/domContentEventFired CDP event. On a connect_over_cdp-attached page, document.write() on an already-loaded document does not re-fire those lifecycle events, so the event wait can never complete; combined with the per-sub-call timeout budgets introduced in the prior change, the operation stopped honoring a single caller deadline and overran (bounded 20s timeout regressed to a hard isolation kill). Fix: - Materialize every document (including text/html) via Runtime.evaluate(window.stop(); document.open/write/close) — the mechanism the remote-CDP attach probe proves reliable — instead of the native command path for the materialize-and-wait flow. Stealth defaults re-apply automatically via the registered addScriptToEvaluateOnNewDocument script when document.write() creates the new document. HTML is transported as a JSON-encoded evaluate argument, preserving escaping. - Remove the unreliable Page lifecycle-event waiter from _wait_for_document_state, Page.goto, and Page.set_content, and delete the now-unused _document_lifecycle_waiter helper. Readiness is confirmed by polling document.readyState with the full remaining budget per poll (the one independently-justified improvement from the prior change), and the whole operation is bounded by a single caller deadline. - goto keeps the commit + readyState-poll structure required for attached pages; the native command fast path is retained only for wait_until="commit". TDD: test_set_content_text_html_materializes_via_evaluate_not_native_command proves the text/html branch materializes via Runtime.evaluate(document.write), never calls the native command, creates no lifecycle-event waiter, and bounds the materialize timeout by the single caller deadline. RED before this change, GREEN after. The Anchor remote-attach hang is not reproducible in this host (local Chromium launch is blocked), so external run3 against the private Tier-A attach probe remains the gate for Defect A. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
Closing — superseded and split. Three fresh external remote-CDP attach validations were run against successive revisions of The independent interaction-timeout fix (Defect B — premature This branch is retained for diagnostics; it is not deleted and not merged anywhere. Closing |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Outcome of external validation
Three fresh external remote-CDP attach validations were executed against successive revisions
of this branch. On every revision the reused/new-page
set_contentcases and thegoto(data:)diagnostic hung and were hard-killed at the isolation wall — they did not even honor the
driver op-timeout — while a bare
evaluatematerialization and the non-rustwright controlssucceeded on identical fresh sessions. The transport, the session, and the document itself are
healthy; the fault is in rustwright's composed navigation/materialize-and-wait sequence on
already-attached targets.
The final revision on this branch (materialize
set_contentviaRuntime.evaluateand drop thelifecycle event-waiter) did not change external attach behavior and, relative to the earlier
revision, turned a clean bounded op-timeout into an unbounded hang. Defect A is not resolved,
and the residual is a native-transport property (a synchronous native call on the attach path
that does not honor its timeout), not something a further Python-level lifecycle-wait change can
remove. No additional workaround was stacked.
Disposition
set_content/ navigation lifecycle over remote attach): unresolved — sunset.Kept as Draft for diagnostics; the branch is retained. Next step is a bounded native-layer
diagnostic experiment, not another Python workaround.
click/select_option/filltimeouts over remote CDP):extracted to a clean PR. It is independently justified, scoped to four actionability/state
polling loops, carries its own regression test, and is not implicated in the attach hang.
External reliability validation for Defect B remains pending.
This PR will be closed once the extracted Defect B PR is verified. It is not merged anywhere.