[rb] fix silent hang on large WebSocket frame

[Chandan25sharma]

• Sets WebSocket.should_raise = true to prevent websocket-ruby from
  returning nil frames on failure, which previously caused infinite read
  loops and silent hangs.
• Increases the default WebSocket.max_frame_size to 100MB (from 20MB)
  to handle large CDP payloads such as Data URLs and screenshots.
• Updates WebSocketConnection#send_cmd to fail immediately if the
  listener thread dies, avoiding a 30-second timeout wait when the
  connection is already broken.
• Changes WebSocket error logging from debug to warn to make failures
  visible to users by default.
• Adds track_cancelled option to Driver#intercept (default: true),
  enabling users to opt-out of the Network domain to reduce bandwidth
  from chatty events.

[qodo-code-review]

Review Summary by Qodo

Fix WebSocket hangs and add network interception options

🐞 Bug fix ✨ Enhancement

Walkthroughs

Description

• Fixes silent hangs on large WebSocket frames by enabling error raising
• Increases max frame size to 100MB to handle large CDP payloads
• Detects dead listener threads to fail immediately instead of timeout
• Elevates WebSocket errors from debug to warn logging level
• Adds optional track_cancelled parameter to reduce Network domain events

File Changes

1. rb/lib/selenium/webdriver/common/websocket_connection.rb 🐞 Bug fix +12/-2

WebSocket configuration and error handling improvements

• Sets WebSocket.should_raise = true to prevent nil frame returns
• Increases WebSocket.max_frame_size to 100MB for large payloads
• Adds listener thread alive check in send_cmd to fail immediately
• Changes WebSocket error logging from debug to warn level

rb/lib/selenium/webdriver/common/websocket_connection.rb

---

2. rb/lib/selenium/webdriver/common/driver_extensions/has_network_interception.rb ✨ Enhancement +2/-2

Add options parameter to intercept method

• Adds **options parameter to intercept method signature
• Forwards options to NetworkInterceptor#intercept call

rb/lib/selenium/webdriver/common/driver_extensions/has_network_interception.rb

---

3. rb/lib/selenium/webdriver/devtools/network_interceptor.rb ✨ Enhancement +3/-3

Add track_cancelled option for network interception

• Adds track_cancelled parameter (default: true) to intercept method
• Conditionally enables Network domain callbacks based on track_cancelled
• Allows users to opt-out of Network domain to reduce bandwidth

rb/lib/selenium/webdriver/devtools/network_interceptor.rb

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (2) 📎 Requirement gaps (0)

1. Global WebSocket config override 🐞 Bug ☼ Reliability

Description

websocket_connection.rb sets WebSocket.should_raise and WebSocket.max_frame_size at file load
time, overriding process-wide websocket-ruby configuration for every Selenium user (even those not
using DevTools/BiDi). This can unexpectedly change behavior and memory limits for other WebSocket
clients in the same Ruby process and makes the new 100MB frame size an unconditional global default.

Code

rb/lib/selenium/webdriver/common/websocket_connection.rb[R22-24]

+WebSocket.should_raise = true
+WebSocket.max_frame_size = 100 * 1024 * 1024
+

Evidence

The assignments are executed at require-time (top-level of websocket_connection.rb), and
selenium/webdriver/common.rb requires this file unconditionally, so the global WebSocket
configuration is modified whenever Selenium is loaded.

rb/lib/selenium/webdriver/common/websocket_connection.rb[20-24]
rb/lib/selenium/webdriver/common.rb[70-103]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`WebSocket.should_raise` and `WebSocket.max_frame_size` are set as global websocket-ruby configuration at file load time. Because this file is required by default, it changes behavior and memory limits process-wide for all Selenium users and potentially any other WebSocket usage in the same app.

### Issue Context
This is a library-level side effect: merely requiring Selenium modifies global WebSocket behavior.

### Fix Focus Areas
- rb/lib/selenium/webdriver/common/websocket_connection.rb[20-24]
- rb/lib/selenium/webdriver/common.rb[70-103]

### What to change
- Avoid unconditional top-level global assignments. Prefer an explicit Selenium-owned configuration surface (e.g., `Selenium::WebDriver::WebSocketConnection.configure(max_frame_size:, should_raise:)`) or, at minimum, only apply defaults if the user hasn’t configured them and document the side effect.
- If websocket-ruby supports per-instance/per-frame configuration, set it on the connection/frame objects instead of mutating global module state.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. track_cancelled opt-out breaks 🐞 Bug ≡ Correctness

Description

When NetworkInterceptor#intercept(track_cancelled: false) is used, Selenium stops recording
cancelled request IDs, but the cancellation-suppression logic still depends on that list to decide
whether to swallow INVALID_INTERCEPTION_ID errors. As a result, normal browser cancellations can
start raising WebDriverError exceptions and break interception for users who disable tracking.

Code

rb/lib/selenium/webdriver/devtools/network_interceptor.rb[R46-52]

+        def intercept(track_cancelled: true, &block)
+          devtools.network.on(:loading_failed) { |params| track_cancelled_request(params) } if track_cancelled
          devtools.fetch.on(:request_paused) { |params| request_paused(params, &block) }

          devtools.network.set_cache_disabled(cache_disabled: true)
-          devtools.network.enable
+          devtools.network.enable if track_cancelled
          devtools.fetch.enable(patterns: [{requestStage: 'Request'}, {requestStage: 'Response'}])

Evidence

The PR makes installation of the Network.loading_failed handler conditional, and that handler is
the only writer to cancelled_requests. However, request_paused always wraps interception in
with_cancellable_request(network_id), which raises on INVALID_INTERCEPTION_ID unless
cancelled?(network_id) is true; with tracking disabled, cancelled? can never be true. The code
comment explicitly notes cancellation as a typical cause of this error.

rb/lib/selenium/webdriver/devtools/network_interceptor.rb[46-53]
rb/lib/selenium/webdriver/devtools/network_interceptor.rb[37-40]
rb/lib/selenium/webdriver/devtools/network_interceptor.rb[71-75]
rb/lib/selenium/webdriver/devtools/network_interceptor.rb[77-89]
rb/lib/selenium/webdriver/devtools/network_interceptor.rb[161-169]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
With `track_cancelled: false`, the interceptor can no longer mark cancellations, but `with_cancellable_request` still requires a cancellation marker to suppress `INVALID_INTERCEPTION_ID` errors. This makes expected browser cancellations surface as exceptions and can break request interception.

### Issue Context
- `cancelled_requests` is only populated by `track_cancelled_request`, which only runs if the `loading_failed` handler is installed.
- `with_cancellable_request` treats `INVALID_INTERCEPTION_ID` as cancellable only when `cancelled?(network_id)` returns true.

### Fix Focus Areas
- rb/lib/selenium/webdriver/devtools/network_interceptor.rb[46-53]
- rb/lib/selenium/webdriver/devtools/network_interceptor.rb[71-75]
- rb/lib/selenium/webdriver/devtools/network_interceptor.rb[77-89]
- rb/lib/selenium/webdriver/devtools/network_interceptor.rb[161-169]

### What to change
Implement a behavior for `track_cancelled: false` that does not rely on `cancelled_requests`:
- Option A: Store `@track_cancelled` in `intercept(...)` and update `with_cancellable_request` to swallow `INVALID_INTERCEPTION_ID` unconditionally when tracking is disabled.
- Option B: Derive cancellation from other available signals in `Fetch.requestPaused` (if feasible) so `cancelled?` can still be true without enabling Network events.
- Ensure the opt-out reduces bandwidth *without* turning expected cancellations into hard failures.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. send_cmd change untested 📘 Rule violation ☼ Reliability

Description

WebSocketConnection#send_cmd now raises immediately when @socket_thread is not alive, which is
an observable behavior change (exception type/timing) that should be locked in with a unit test.
Without coverage, regressions could reintroduce the silent hang/timeout behavior this change intends
to fix.

Code

rb/lib/selenium/webdriver/common/websocket_connection.rb[R115-118]

+        wait.until do
+          @messages_mtx.synchronize { messages.delete(id) } ||
+            (raise Error::WebDriverError, "WebSocket listener thread is dead" unless @socket_thread&.alive?)
+        end

Evidence

The checklist requires adding/updating tests for functional and observable behavior changes. The PR
changes send_cmd waiting behavior by introducing a new failure path when the listener thread dies,
but no corresponding test change is shown alongside this new logic.

AGENTS.md
rb/lib/selenium/webdriver/common/websocket_connection.rb[115-118]
Best Practice: Learned patterns

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`WebSocketConnection#send_cmd` now raises `Error::WebDriverError` when the listener thread is dead (instead of waiting for the timeout), but this observable behavior change is not covered by a unit test.

## Issue Context
This change is intended to prevent silent hangs/infinite waits when the listener dies; a regression test should assert the new fail-fast behavior.

## Fix Focus Areas
- rb/lib/selenium/webdriver/common/websocket_connection.rb[115-118]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. track_cancelled untested option 📘 Rule violation ☼ Reliability

Description

NetworkInterceptor#intercept introduces the new track_cancelled: option and changes which CDP
domains/events are enabled based on it, which is an observable behavior change. This should be
validated with unit tests to ensure both true and false paths behave as intended.

Code

rb/lib/selenium/webdriver/devtools/network_interceptor.rb[R46-52]

+        def intercept(track_cancelled: true, &block)
+          devtools.network.on(:loading_failed) { |params| track_cancelled_request(params) } if track_cancelled
          devtools.fetch.on(:request_paused) { |params| request_paused(params, &block) }

          devtools.network.set_cache_disabled(cache_disabled: true)
-          devtools.network.enable
+          devtools.network.enable if track_cancelled
          devtools.fetch.enable(patterns: [{requestStage: 'Request'}, {requestStage: 'Response'}])

Evidence

The checklist requires adding/updating tests for functional and observable behavior changes (such as
new defaults and conditionally-enabled behavior). The PR adds conditional behavior controlled by
track_cancelled: in intercept, but no corresponding test change is shown alongside this new
branching logic.

AGENTS.md
rb/lib/selenium/webdriver/devtools/network_interceptor.rb[46-52]
Best Practice: Learned patterns

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`NetworkInterceptor#intercept` now has a `track_cancelled:` option (default `true`) that changes which handlers/domains are enabled. This new behavior is not protected by unit tests.

## Issue Context
Tests should assert the default (`track_cancelled: true`) preserves prior behavior, and that `track_cancelled: false` avoids registering `loading_failed` handling / enabling Network while still allowing request interception via Fetch.

## Fix Focus Areas
- rb/lib/selenium/webdriver/devtools/network_interceptor.rb[46-52]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[qodo-code-review]

1. Global websocket config override 🐞 Bug ☼ Reliability

websocket_connection.rb sets WebSocket.should_raise and WebSocket.max_frame_size at file load
time, overriding process-wide websocket-ruby configuration for every Selenium user (even those not
using DevTools/BiDi). This can unexpectedly change behavior and memory limits for other WebSocket
clients in the same Ruby process and makes the new 100MB frame size an unconditional global default.

Agent Prompt

### Issue description
`WebSocket.should_raise` and `WebSocket.max_frame_size` are set as global websocket-ruby configuration at file load time. Because this file is required by default, it changes behavior and memory limits process-wide for all Selenium users and potentially any other WebSocket usage in the same app.

### Issue Context
This is a library-level side effect: merely requiring Selenium modifies global WebSocket behavior.

### Fix Focus Areas
- rb/lib/selenium/webdriver/common/websocket_connection.rb[20-24]
- rb/lib/selenium/webdriver/common.rb[70-103]

### What to change
- Avoid unconditional top-level global assignments. Prefer an explicit Selenium-owned configuration surface (e.g., `Selenium::WebDriver::WebSocketConnection.configure(max_frame_size:, should_raise:)`) or, at minimum, only apply defaults if the user hasn’t configured them and document the side effect.
- If websocket-ruby supports per-instance/per-frame configuration, set it on the connection/frame objects instead of mutating global module state.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[qodo-code-review]

2. Track_cancelled opt-out breaks 🐞 Bug ≡ Correctness

When NetworkInterceptor#intercept(track_cancelled: false) is used, Selenium stops recording
cancelled request IDs, but the cancellation-suppression logic still depends on that list to decide
whether to swallow INVALID_INTERCEPTION_ID errors. As a result, normal browser cancellations can
start raising WebDriverError exceptions and break interception for users who disable tracking.

Agent Prompt

### Issue description
With `track_cancelled: false`, the interceptor can no longer mark cancellations, but `with_cancellable_request` still requires a cancellation marker to suppress `INVALID_INTERCEPTION_ID` errors. This makes expected browser cancellations surface as exceptions and can break request interception.

### Issue Context
- `cancelled_requests` is only populated by `track_cancelled_request`, which only runs if the `loading_failed` handler is installed.
- `with_cancellable_request` treats `INVALID_INTERCEPTION_ID` as cancellable only when `cancelled?(network_id)` returns true.

### Fix Focus Areas
- rb/lib/selenium/webdriver/devtools/network_interceptor.rb[46-53]
- rb/lib/selenium/webdriver/devtools/network_interceptor.rb[71-75]
- rb/lib/selenium/webdriver/devtools/network_interceptor.rb[77-89]
- rb/lib/selenium/webdriver/devtools/network_interceptor.rb[161-169]

### What to change
Implement a behavior for `track_cancelled: false` that does not rely on `cancelled_requests`:
- Option A: Store `@track_cancelled` in `intercept(...)` and update `with_cancellable_request` to swallow `INVALID_INTERCEPTION_ID` unconditionally when tracking is disabled.
- Option B: Derive cancellation from other available signals in `Fetch.requestPaused` (if feasible) so `cancelled?` can still be true without enabling Network events.
- Ensure the opt-out reduces bandwidth *without* turning expected cancellations into hard failures.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[aguspe]

hey thanks for looking into this, the websocket hang is a real pain

the global WebSocket config setting should_raise and max_frame_size at require time will affect any other websocket-ruby usage in the same process which is not great, maybe we can scope this to the connection instance instead?

also when track_cancelled: false the cancelled_requests never gets populated but with_cancellable_request still checks cancelled?, so browser cancellations will raise instead of being swallowed, that needs to be handled

this PR is doing a lot of things at once though, websocket config, dead thread detection, logging changes and the track_cancelled feature, wouldnt it be easier to review if we split into smaller PRs? maybe start with the websocket hang fix first and then the track_cancelled feature separately, we'd also need tests for the new behavior