fix: separate screenshot response timeout from IPC connect timeout - #641
Open
MagMueller wants to merge 1 commit into
Open
fix: separate screenshot response timeout from IPC connect timeout#641MagMueller wants to merge 1 commit into
MagMueller wants to merge 1 commit into
Conversation
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.
What Problem This Solves
Page.captureScreenshotcan take longer than the 5-second local IPC connection budget when the daemon is attached to a remote Cloud browser. The connected socket currently reuses that 5-second value for the entire screenshot response, causing recoverable screenshot calls to time out.Why This Change Was Made
Keep the 5-second IPC connect and default response budget, but give screenshot responses a bounded 60-second budget. Timeout errors remain generic and do not expose endpoint details.
User Impact
Remote viewport and full-page screenshots can complete without weakening the timeout applied to ordinary Browser Harness commands.
Evidence
uv run --with pytest pytest -q tests/unit/test_helpers.py: 24 passed.git diff --check: passed.Summary by cubic
Separates the IPC connect timeout from the response timeout so screenshots don’t inherit the 5s budget. Previously a single 5s timeout covered connect and response; now connect stays 5s and default responses stay 5s, while
Page.captureScreenshotuses a 60s response budget without weakening other commands or exposing endpoint details.IPC_CONNECT_TIMEOUT_SECONDS(5s),DEFAULT_IPC_RESPONSE_TIMEOUT_SECONDS(5s), andSCREENSHOT_IPC_RESPONSE_TIMEOUT_SECONDS(60s);_sendsets per-call socket timeouts and raises_IPCResponseTimeouton response timeouts.cdpaccepts_response_timeout;capture_screenshotuses the 60s budget and converts screenshot timeouts into a contextualRuntimeError. Tests cover the new budgets and error message.Written for commit 742d3ed. Summary will update on new commits.