fix(daemon): bound CDP request waits#372
Open
honor2030 wants to merge 1 commit into
Open
Conversation
4 tasks
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
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.
Summary
Page.navigatepath that previously left the helper with only a client-side_ipc.request()timeout.Refs #370
Why
In #370, the visible symptom is a second navigation timing out in
_ipc.pywhile the daemon later logsconn: Connection lost. I could not reproduce the underlying WebSocket drop on currentmain, but this hardens the daemon boundary so a stuck CDP send produces a usefulPage.navigate timed out after 4.5sresponse instead of letting the helper hit its 5s socket deadline first.Test Plan
uv run --with pytest --with pillow python -m pytest tests/unit/test_daemon.py::test_regular_cdp_calls_timeout_before_client_socket_deadline -qfailed before the fix with an outerTimeoutError.uv run --with pytest --with pillow python -m pytest tests/unit/test_daemon.py::test_regular_cdp_calls_timeout_before_client_socket_deadline -q→1 passed.uv run --with pytest --with pillow python -m pytest tests/unit -q→91 passed.uv run python -m compileall -q src tests→ passed.git diff --check→ passed.BU_NAME=oss370pr2 uv run browser-harnesswithnew_tab('https://example.com')followed bynew_tab('https://httpbin.org/ip')→ both navigations returnedpage_info()successfully on my local Chrome.Notes
This is a scoped mitigation/diagnostic hardening PR, not a claim that the root WebSocket drop reported in #370 is fully fixed. If maintainers prefer a different timeout budget or want this limited to navigation/session-bound calls, I can adjust.
Summary by cubic
Bound CDP request waits in the daemon to 4.5s so stalled Chrome/CDP calls return a clear error before the helper’s 5s IPC socket deadline. Mitigates the timeout symptom in #370 by failing fast with messages like "Page.navigate timed out after 4.5s".
CDP_CALL_TIMEOUT = 4.5and asend_cdp()wrapper that usesasyncio.wait_forand returns a concise timeout error.Written for commit 1689057. Summary will update on new commits. Review in cubic