Make websocket ping_interval/ping_timeout configurable (default off) to avoid false 1011 disconnects

[immineal]

What

CDPClient currently connects with the websockets library's own defaults for ping_interval/ping_timeout (20s/20s). This PR makes both configurable and defaults them to None (disabled).

Why

CDP is a request/response protocol carried over this same socket -- real CDP traffic is already sufficient evidence the connection is alive. websockets' own keepalive ping is a separate, independent liveness check: it can time out under perfectly normal conditions (the browser busy servicing a heavy command, extra latency on a remote/cloud session) well before the connection has actually died, and the library then force-closes with code 1011. From the client's point of view this is indistinguishable from a real disconnect -- it just looks like an unexplained mid-session drop.

I hit and root-caused the identical failure in a separate from-scratch CDP-over-websockets client I built and have run against a range of real, hostile-to-automation production sites (ping_interval=None is the one-line fix). Worth noting WebSocketLogFilter in this file already has dedicated logic to track ping-send-time and warn on "PING not answered by browser... (>3s and no PONG received)" -- that's exactly the timeout window this PR removes, which reads like this exact class of failure has already been noticed and worked around by logging it rather than by addressing why it happens.

Possibly related: browser-use/browser-use#4688 describes CDP websocket connections intermittently dropping mid-interaction on cloud/remote browser sessions, which is the shape (added latency, no clear cause) this failure mode produces -- flagging it as a plausible match, not a confirmed one, since the report doesn't include a close code.

What this changes

• ping_interval/ping_timeout become constructor params on CDPClient, both defaulting to None.
• A caller who wants the transport-level liveness check back (e.g. to detect a genuinely dead TCP connection with no CDP traffic at all) can pass explicit values to restore it.
• No other behavior change.

Happy to adjust the defaults/API shape if you'd rather this be strictly opt-in instead of on-by-default.

[cubic-dev-ai]

1 issue found across 1 file

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="cdp_use/client.py">

<violation number="1" location="cdp_use/client.py:290">
P2: With keepalive pings disabled by default (ping_interval=None), a connection that dies silently—network partition or drop with no TCP FIN/RST—is no longer detected. Both `await self.ws.recv()` in `_handle_messages` and `await future` in `send_raw` block indefinitely, so CDP commands hang forever instead of failing fast as they did when a missed pong closed the socket after ~20s. The comment's claim that CDP's own traffic is 'sufficient liveness detection' only holds while traffic is flowing; during idle moments a dead connection is indistinguishable from a live one. Consider keeping detection available (e.g., a conservative non-None default such as a long ping_interval, or an application-level request timeout) rather than removing it entirely, and document that None means commands can hang when the socket dies without an RST.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

P2: With keepalive pings disabled by default (ping_interval=None), a connection that dies silently—network partition or drop with no TCP FIN/RST—is no longer detected. Both await self.ws.recv() in _handle_messages and await future in send_raw block indefinitely, so CDP commands hang forever instead of failing fast as they did when a missed pong closed the socket after ~20s. The comment's claim that CDP's own traffic is 'sufficient liveness detection' only holds while traffic is flowing; during idle moments a dead connection is indistinguishable from a live one. Consider keeping detection available (e.g., a conservative non-None default such as a long ping_interval, or an application-level request timeout) rather than removing it entirely, and document that None means commands can hang when the socket dies without an RST.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At cdp_use/client.py, line 290:

<comment>With keepalive pings disabled by default (ping_interval=None), a connection that dies silently—network partition or drop with no TCP FIN/RST—is no longer detected. Both `await self.ws.recv()` in `_handle_messages` and `await future` in `send_raw` block indefinitely, so CDP commands hang forever instead of failing fast as they did when a missed pong closed the socket after ~20s. The comment's claim that CDP's own traffic is 'sufficient liveness detection' only holds while traffic is flowing; during idle moments a dead connection is indistinguishable from a live one. Consider keeping detection available (e.g., a conservative non-None default such as a long ping_interval, or an application-level request timeout) rather than removing it entirely, and document that None means commands can hang when the socket dies without an RST.</comment>

<file context>
@@ -271,6 +287,8 @@ async def start(self):
         )
         connect_kwargs = {
             "max_size": self.max_ws_frame_size,
+            "ping_interval": self.ping_interval,
+            "ping_timeout": self.ping_timeout,
         }
</file context>

[immineal]

Both points are fair, thanks. Pushed a fix:

• P3 (ping_timeout with ping_interval=None is a silent no-op): ping_interval now defaults to 20 (the websockets library's own default) instead of None, so the common case of only overriding ping_timeout does what you'd expect. Added a comment stating the coupling explicitly for anyone who does override ping_interval to None.
• P2 (fully disabling detection means a dead connection can hang forever): agreed that removing keepalive entirely was too blunt. Kept ping_interval enabled by default so pings still go out (useful as a liveness/log signal and for NAT/proxy keepalive) and only disabled ping_timeout — i.e. a slow pong under load no longer forces a close, but the ping traffic itself isn't gone. Documented the remaining gap (a connection that dies with no TCP-level signal can still hang) and that callers who need a hard bound should restore ping_timeout or add an application-level timeout around commands.

Commit: bbce4bb

[cubic-dev-ai]

All reported issues were addressed across 1 file (changes from recent commits).

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[immineal]

Good catch — fixed in 401545e.

The comment implied the PING/PONG log warning "tracks exactly that timeout window," i.e. that it stays in sync with whatever `ping_timeout` is set to. It doesn't: `check_timeout()` above hard-codes `asyncio.sleep(3)`, so it always warns at 3s regardless of the configured `ping_timeout` value. Reworded to describe it as a separate, hard-coded 3s check rather than implying synchronization.

[immineal]

All CI checks are green and both review comments from last week are addressed (401545e, and the ping_interval default fix before that). Let me know if anything else would help get this over the line, or if you'd rather I change the API shape (e.g. make the disabled ping strictly opt-in instead of the new default).