[rb] fix silent hang on oversized WebSocket frames

[aguspe]

💥 What does this PR do?

The Ruby devtools/bidi websocket listener can hang when it gets a frame bigger than WebSocket.max_frame_size (websocket-ruby defaults to 20MB).

Websocket-ruby raises TooLong internally but rescues it unless you set should_raise, so incoming_frame.next just returns nil, the oversized bytes stay buffered, and the loop spins forever at ~100% cpu.

builds on #17286, thanks @Chandan25sharma. left the track_cancelled part out, that's separate.

🔧 Implementation Notes

the bump happens in WebSocketConnection#initialize, not at load time, and only when the current limit is lower, so non-devtools users or anyone who already set a bigger value aren't affected. websocket-ruby only
exposes max_frame_size globally so there's no per-instance option. didn't touch should_raise, detection uses the per-instance incoming_frame.error? instead.

🤖 AI assistance

• AI assisted (complete below)
  • Tool(s): Claude Code
  • What was generated: Partially the description description and RBS signatures
  • I reviewed all AI output and can explain the change

🔄 Types of changes

• Bug fix (backwards compatible)

[qodo-code-review]

Review Summary by Qodo

Fix silent hang on oversized WebSocket frames

🐞 Bug fix

Walkthroughs

Description

• Raise WebSocket frame size limit to 100MB for DevTools connections
• Surface dropped frames as logged errors instead of silent hangs
• Add frame error detection to stop listener gracefully
• Include unit tests for frame size and error handling

Diagram

flowchart LR
  A["WebSocket Frame Received"] --> B["Check Frame Size"]
  B --> C{"Exceeds Limit?"}
  C -->|No| D["Process Frame"]
  C -->|Yes| E["frame_dropped? Detects Error"]
  E --> F["Log Error Message"]
  F --> G["Stop Listener Gracefully"]
  D --> H["Continue Processing"]

Loading

File Changes

1. rb/lib/selenium/webdriver/common/websocket_connection.rb 🐞 Bug fix +39/-9

Implement frame size limit and error detection

• Added MAX_FRAME_SIZE constant set to 100MB
• Introduced apply_frame_size_limit method to conditionally raise global WebSocket limit
• Extracted frame processing logic into process_incoming_frames method
• Added frame_dropped? method to detect and log frame decoding errors
• Modified attach_socket_listener to break on dropped frames instead of spinning

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

---

2. rb/spec/unit/selenium/webdriver/common/websocket_connection_spec.rb 🧪 Tests +75/-0

Add unit tests for frame handling

• New unit test file for WebSocketConnection
• Tests apply_frame_size_limit raises low limits and preserves higher ones
• Tests frame_dropped? returns false for valid frames and true for oversized frames
• Uses frame allocation to test logic without socket connections

rb/spec/unit/selenium/webdriver/common/websocket_connection_spec.rb

---

3. rb/sig/gems/websocket/websocket.rbs 📝 Documentation +3/-0

Add WebSocket max_frame_size type signatures

• Added type signatures for WebSocket.max_frame_size getter and setter methods

rb/sig/gems/websocket/websocket.rbs

---

View more (1)

4. rb/sig/lib/selenium/webdriver/common/websocket_connection.rbs 📝 Documentation +8/-0

Update WebSocketConnection type signatures

• Added MAX_FRAME_SIZE constant type signature
• Added type signatures for process_incoming_frames, frame_dropped?, and
 apply_frame_size_limit methods

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

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (4) 📘 Rule violations (2) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 19 rules

1. Listener stops without closing ✓ Resolved 🐞 Bug ☼ Reliability

Description

When frame_dropped? becomes true, attach_socket_listener breaks out of its read loop without
closing the socket or setting @closing, leaving the connection in a “dead listener, open socket”
state. Subsequent send_cmd calls can hang until Wait#until times out because no thread is
reading responses anymore.

Code

rb/lib/selenium/webdriver/common/websocket_connection.rb[R141-145]

+            process_incoming_frames

-              @messages_mtx.synchronize { callbacks[message['method']].dup }.each do |callback|
-                @callback_threads.add(callback_thread(message['params'], &callback))
-              end
-            end
+            # Stop instead of spinning forever on a frame that can never be parsed.
+            break if frame_dropped?
          end

Evidence

The listener loop breaks on frame_dropped? without closing the socket or setting @closing, but
send_cmd depends on the listener thread to read and populate messages; without it, Wait#until
will only raise after its timeout, appearing as a hang.

rb/lib/selenium/webdriver/common/websocket_connection.rb[132-149]
rb/lib/selenium/webdriver/common/websocket_connection.rb[164-175]
rb/lib/selenium/webdriver/common/websocket_connection.rb[105-119]
rb/lib/selenium/webdriver/common/wait.rb[51-77]

Agent prompt

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

## Issue description
When an oversized/undecodable frame is detected, the listener thread exits via `break if frame_dropped?`, but the connection is not transitioned to a closed/closing state and the socket is not closed. This leaves `send_cmd` able to write, but with no listener to ever populate `messages`, causing callers to block until timeout.

## Issue Context
- `send_cmd` waits for a response ID to appear in `messages` via `Wait#until`.
- The listener thread is the only mechanism that reads from the socket and calls `process_frame` to populate `messages`.

## Fix Focus Areas
- rb/lib/selenium/webdriver/common/websocket_connection.rb[105-119]
- rb/lib/selenium/webdriver/common/websocket_connection.rb[132-149]
- rb/lib/selenium/webdriver/common/websocket_connection.rb[164-179]

## Proposed fix
1. When `frame_dropped?` is true, transition the connection to a closed/closing state and close the socket.
  - Avoid calling `close` directly from the socket thread (it joins threads); instead introduce a helper like `shutdown_socket!(reason)` that:
    - sets `@closing = true` under `@closing_mtx`
    - closes `socket` (rescuing `CONNECTION_ERRORS`)
    - optionally stores `@listener_error` for later reporting.
2. Make `send_cmd` fail fast when the listener has terminated due to a decode error (e.g., if `@listener_error` is set, or if `@socket_thread` is not alive and `@closing` is false), raising a `WebDriverError` with an actionable message rather than waiting 30s.
3. Consider emitting a debug log when the listener exits normally via this path, so user logs show a clear lifecycle event.

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

---

2. around block lacks ensure ✓ Resolved 📘 Rule violation ☼ Reliability

Description

The spec mutates global WebSocket.max_frame_size but restores it only after example.call, so an
exception/failing expectation can skip restoration and contaminate subsequent tests. This violates
the requirement to restore mutated global process state in an ensure/finally block.

Code

rb/spec/unit/selenium/webdriver/common/websocket_connection_spec.rb[R29-33]

+      around do |example|
+        original = WebSocket.max_frame_size
+        example.call
+        WebSocket.max_frame_size = original
+      end

Evidence

PR Compliance ID 9 requires restoring mutated global state using ensure/finally. The added
around hook sets WebSocket.max_frame_size, runs example.call, and restores afterward without
an ensure, so restoration can be skipped on error.

rb/spec/unit/selenium/webdriver/common/websocket_connection_spec.rb[29-33]
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
The spec changes global `WebSocket.max_frame_size` but does not guarantee it is restored if the example raises/fails, which can leak state into later tests.

## Issue Context
RSpec expectation failures raise exceptions; without `ensure`, the cleanup line may not run.

## Fix Focus Areas
- rb/spec/unit/selenium/webdriver/common/websocket_connection_spec.rb[29-33]

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

---

3. WebSocketConnection missing @api private 📘 Rule violation ✧ Quality ⭐ New

Description

Selenium::WebDriver::WebSocketConnection appears to be an internal helper (used by
BiDi/DevTools) but the class lacks a YARD # @api private tag after being modified in this PR.
This can accidentally document/stabilize an internal API surface contrary to the project’s
internal-API marking policy.

Code

rb/lib/selenium/webdriver/common/websocket_connection.rb[R38-41]

+      # websocket-ruby defaults to a 20MB limit and silently drops larger
+      # frames, which can stall the listener. CDP payloads (e.g. large data:
+      # URLs) can exceed that, so raise the ceiling for our connections.
+      MAX_FRAME_SIZE = 100 * 1024 * 1024 # 100MB

Evidence

Rule 389237 requires internal Ruby APIs to be marked with # @api private in YARD docs.
WebSocketConnection is instantiated by higher-level components (BiDi/DevTools), and the
codebase demonstrates the expected pattern via other internal classes (e.g., BiDi::Session), but
WebSocketConnection has no @api private tag despite being modified in this PR.

Rule 389237: Mark internal Ruby APIs with @api private in YARD docs
rb/lib/selenium/webdriver/common/websocket_connection.rb[22-25]
rb/lib/selenium/webdriver/common/websocket_connection.rb[38-43]
rb/lib/selenium/webdriver/bidi.rb[34-36]
rb/lib/selenium/webdriver/devtools.rb[31-33]
rb/lib/selenium/webdriver/bidi/session.rb[23-27]

Agent prompt

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

## Issue description
`Selenium::WebDriver::WebSocketConnection` is used as an internal helper by other components, but it is not marked with YARD `@api private`. Per project policy, internal Ruby APIs should be explicitly tagged to avoid being treated as supported public API in generated docs.

## Issue Context
This PR modifies `WebSocketConnection` (e.g., adds `MAX_FRAME_SIZE` and related helper logic). Other internal Selenium Ruby classes/modules use `# @api private` consistently.

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

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

---

4. Close skips cleanup 🐞 Bug ☼ Reliability ⭐ New

Description

If the listener hits frame_dropped? it calls close_socket, which sets @closing=true; a later user
call to close will return early and skip joining the socket/callback threads. This can leave
background callback work running longer than intended and makes shutdown behavior inconsistent
between the normal close path and the dropped-frame path.

Code

rb/lib/selenium/webdriver/common/websocket_connection.rb[R57-69]

      end

Evidence

The dropped-frame path sets @closing via close_socket; close then short-circuits on @closing, so it
won’t execute its join/unwind logic after a frame-induced shutdown.

rb/lib/selenium/webdriver/common/websocket_connection.rb[59-75]
rb/lib/selenium/webdriver/common/websocket_connection.rb[144-174]

Agent prompt

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

### Issue description
`frame_dropped?` closes the socket via `close_socket`, which sets `@closing = true`. Later, `WebSocketConnection#close` returns early when it sees `@closing`, so it skips the normal cleanup (`@socket_thread.join` and joining callback threads). This makes the dropped-frame path leave cleanup work unperformed.

### Issue Context
- `frame_dropped?` triggers an internal shutdown by calling `close_socket`.
- `close_socket` sets `@closing = true`.
- `close` currently returns immediately if `@closing` is already true, preventing thread-join cleanup.

### Fix Focus Areas
- rb/lib/selenium/webdriver/common/websocket_connection.rb[59-75]
- rb/lib/selenium/webdriver/common/websocket_connection.rb[144-174]

### Suggested fix approach
- Make `close` idempotent but *not* a no-op when `@closing` is already true.
 - Example: record `already_closing = @closing` under `@closing_mtx`, set `@closing = true`, and proceed to perform joins regardless.
 - Optionally, only call `close_socket` when not already closing, but still run the join/unwind section.
- Ensure you don’t attempt to join the current thread if `close` is ever called from inside the socket listener thread (guard with `Thread.current != @socket_thread`).

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

---

5. No cross-binding note for MAX_FRAME_SIZE 📘 Rule violation ≡ Correctness

Description

This PR changes user-visible DevTools/BiDi WebSocket behavior by raising the effective frame size
ceiling and closing the connection when a frame is dropped, but the changed code does not document
any comparison/alignment with other Selenium language bindings or an intentional divergence. This
risks inconsistent behavior across bindings without an auditable rationale.

Code

rb/lib/selenium/webdriver/common/websocket_connection.rb[R38-41]

+      # websocket-ruby defaults to a 20MB limit and silently drops larger
+      # frames, which can stall the listener. CDP payloads (e.g. large data:
+      # URLs) can exceed that, so raise the ceiling for our connections.
+      MAX_FRAME_SIZE = 100 * 1024 * 1024 # 100MB

Evidence

The checklist requires cross-language comparison/documentation when changing user-visible binding
behavior (PR Compliance ID 389265). The PR introduces a new MAX_FRAME_SIZE and applies it, and
adds logic to close the connection when an oversized frame is detected, but the surrounding comments
do not reference any cross-binding comparison or intentional divergence rationale.

Rule 389265: Compare cross-language bindings when changing user-visible behavior
rb/lib/selenium/webdriver/common/websocket_connection.rb[38-41]
rb/lib/selenium/webdriver/common/websocket_connection.rb[167-178]

Agent prompt

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

## Issue description
A user-visible behavior change was introduced in the Ruby binding (increasing the effective WebSocket frame size ceiling and closing the connection when an oversized frame is detected), but there is no nearby documentation indicating which other Selenium binding(s) were compared or why Ruby should differ.

## Issue Context
PR Compliance requires cross-language comparison evidence for user-visible behavior changes. Adding a short comment (or a doc/changelog note) referencing the binding(s) checked and the expected alignment/divergence is sufficient.

## Fix Focus Areas
- rb/lib/selenium/webdriver/common/websocket_connection.rb[38-41]
- rb/lib/selenium/webdriver/common/websocket_connection.rb[167-178]

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

---

View more (3)

6. send_cmd waits after close 🐞 Bug ☼ Reliability

Description

When frame_dropped? closes the socket and sets @closing, any send_cmd already waiting for its
response will still block until RESPONSE_WAIT_TIMEOUT and then raise a generic TimeoutError,
obscuring the actual connection failure. This can add a ~30s delay to failures triggered by
oversized/invalid frames.

Code

rb/lib/selenium/webdriver/common/websocket_connection.rb[R167-173]

+      def frame_dropped?
+        return false unless incoming_frame.error?
+
+        WebDriver.logger.error("WebSocket frame dropped (#{incoming_frame.error}): exceeds max_frame_size " \
+                               "(#{WebSocket.max_frame_size} bytes). Raise WebSocket.max_frame_size=", id: :ws)
+        close_socket
+        true

Evidence

The PR change makes frame_dropped? close the socket (via close_socket), and close_socket sets
@closing = true. However, send_cmd waits solely for a matching message ID to appear and does not
consult @closing, so if the socket is closed before the response arrives, the wait can only end by
timing out (Wait raises Error::TimeoutError).

rb/lib/selenium/webdriver/common/websocket_connection.rb[100-114]
rb/lib/selenium/webdriver/common/websocket_connection.rb[144-174]
rb/lib/selenium/webdriver/common/wait.rb[51-77]

Agent prompt

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

### Issue description
`frame_dropped?` now closes the socket and sets `@closing`, but `send_cmd` continues to wait for a response via `Wait#until` without checking connection/closing state. If the listener has been torn down (e.g., due to an oversized frame), the response can never arrive, so callers wait the full timeout and get a generic `Error::TimeoutError`.

### Issue Context
This PR correctly prevents the previous hot-spin by closing the websocket when `incoming_frame.error?` is set. However, callers already blocked inside `send_cmd` can still experience a long, misleading timeout.

### Fix Focus Areas
- rb/lib/selenium/webdriver/common/websocket_connection.rb[100-114]
- rb/lib/selenium/webdriver/common/websocket_connection.rb[144-174]

### Suggested fix
In `send_cmd`, make the wait condition fail fast when the connection is closing/closed. For example:
- Check `@closing` inside the `wait.until` block and raise a clearer error (e.g., `Error::WebDriverError, "WebSocket closed while waiting for response"`) when true.
- Optionally also guard with `socket.closed?` if available.
This ensures dropped-frame shutdowns surface promptly rather than timing out.

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

---

7. Incomplete error log hint 🐞 Bug ◔ Observability

Description

frame_dropped? logs an error message ending with Raise WebSocket.max_frame_size= but provides no
suggested value (despite having a known MAX_FRAME_SIZE constant), making the remediation unclear
when a connection is terminated due to an oversized frame. This slows diagnosis of a production
failure mode where the listener closes itself on decode errors.

Code

rb/lib/selenium/webdriver/common/websocket_connection.rb[R170-171]

+        WebDriver.logger.error("WebSocket frame dropped (#{incoming_frame.error}): exceeds max_frame_size " \
+                               "(#{WebSocket.max_frame_size} bytes). Raise WebSocket.max_frame_size=", id: :ws)

Evidence

The log message currently ends with Raise WebSocket.max_frame_size= without providing any value;
the class defines MAX_FRAME_SIZE (100MB), which can be referenced to make the log actionable.

rb/lib/selenium/webdriver/common/websocket_connection.rb[38-42]
rb/lib/selenium/webdriver/common/websocket_connection.rb[170-172]

Agent prompt

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

### Issue description
The error log emitted when a frame is dropped ends with `Raise WebSocket.max_frame_size=` but does not include a recommended value (or example assignment), so it’s not actionable.

### Issue Context
`MAX_FRAME_SIZE` is defined (100MB) and the log already prints the current `WebSocket.max_frame_size`; operators need a concrete next step when this terminal condition occurs.

### Fix Focus Areas
- rb/lib/selenium/webdriver/common/websocket_connection.rb[170-172]

### Suggested change
Update the log string to include a recommended value, e.g.:
- `... Raise WebSocket.max_frame_size=#{MAX_FRAME_SIZE} (or higher).`
- Or `... Set WebSocket.max_frame_size to >= #{MAX_FRAME_SIZE}.`
Also remove the trailing `=` if no assignment example is included.

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

---

8. Global frame size persists 🐞 Bug ⛨ Security

Description

apply_frame_size_limit permanently raises the process-wide WebSocket.max_frame_size when it is
below 100MB, and close never restores the previous value. This can unexpectedly override a user’s
intentionally-lower global limit for the remainder of the process after the first DevTools/BiDi
connection.

Code

rb/lib/selenium/webdriver/common/websocket_connection.rb[R177-179]

+      def apply_frame_size_limit
+        WebSocket.max_frame_size = MAX_FRAME_SIZE if WebSocket.max_frame_size < MAX_FRAME_SIZE
+      end

Evidence

The code calls apply_frame_size_limit in initialization and conditionally sets the global
WebSocket.max_frame_size, but no restoration occurs in close, so the process-wide setting
persists beyond the connection’s lifecycle.

rb/lib/selenium/webdriver/common/websocket_connection.rb[43-57]
rb/lib/selenium/webdriver/common/websocket_connection.rb[177-179]
rb/lib/selenium/webdriver/common/websocket_connection.rb[59-80]
rb/spec/unit/selenium/webdriver/common/websocket_connection_spec.rb[29-33]

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.max_frame_size` is global module state; `WebSocketConnection#initialize` conditionally increases it, but no code restores the prior value when the connection is closed. If an application intentionally configured a smaller limit (e.g., for memory/DoS constraints), using Selenium DevTools/BiDi will override that policy for the remainder of the process.

## Issue Context
- The test suite explicitly saves/restores `WebSocket.max_frame_size`, which highlights that this state is global and sticky.
- Because it’s global, multiple concurrent connections need careful handling to avoid restoring too early.

## Fix Focus Areas
- rb/lib/selenium/webdriver/common/websocket_connection.rb[43-57]
- rb/lib/selenium/webdriver/common/websocket_connection.rb[59-80]
- rb/lib/selenium/webdriver/common/websocket_connection.rb[177-179]
- rb/spec/unit/selenium/webdriver/common/websocket_connection_spec.rb[29-33]

## Proposed fix
1. Track whether this instance modified the global value:
  - Store `@original_max_frame_size` and a flag like `@bumped_frame_size_limit` when raising it.
2. Restore safely on `close`:
  - Prefer a class-level refcount + mutex (e.g., `@@frame_size_bumps`) so restoration happens only when the last connection that bumped the value closes.
  - Restore only if `WebSocket.max_frame_size` is still equal to `MAX_FRAME_SIZE` (or the value you set) to avoid clobbering user changes made after initialization.
3. Add/update specs to validate restoration behavior (including multiple connections bumping concurrently if your test suite supports it).

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

---

Previous review results

Review updated until commit f23041f

Results up to commit ce6fe48

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Listener stops without closing ✓ Resolved 🐞 Bug ☼ Reliability

Description

When frame_dropped? becomes true, attach_socket_listener breaks out of its read loop without
closing the socket or setting @closing, leaving the connection in a “dead listener, open socket”
state. Subsequent send_cmd calls can hang until Wait#until times out because no thread is
reading responses anymore.

Code

rb/lib/selenium/webdriver/common/websocket_connection.rb[R141-145]

+            process_incoming_frames

-              @messages_mtx.synchronize { callbacks[message['method']].dup }.each do |callback|
-                @callback_threads.add(callback_thread(message['params'], &callback))
-              end
-            end
+            # Stop instead of spinning forever on a frame that can never be parsed.
+            break if frame_dropped?
          end

Evidence

The listener loop breaks on frame_dropped? without closing the socket or setting @closing, but
send_cmd depends on the listener thread to read and populate messages; without it, Wait#until
will only raise after its timeout, appearing as a hang.

rb/lib/selenium/webdriver/common/websocket_connection.rb[132-149]
rb/lib/selenium/webdriver/common/websocket_connection.rb[164-175]
rb/lib/selenium/webdriver/common/websocket_connection.rb[105-119]
rb/lib/selenium/webdriver/common/wait.rb[51-77]

Agent prompt

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

## Issue description
When an oversized/undecodable frame is detected, the listener thread exits via `break if frame_dropped?`, but the connection is not transitioned to a closed/closing state and the socket is not closed. This leaves `send_cmd` able to write, but with no listener to ever populate `messages`, causing callers to block until timeout.

## Issue Context
- `send_cmd` waits for a response ID to appear in `messages` via `Wait#until`.
- The listener thread is the only mechanism that reads from the socket and calls `process_frame` to populate `messages`.

## Fix Focus Areas
- rb/lib/selenium/webdriver/common/websocket_connection.rb[105-119]
- rb/lib/selenium/webdriver/common/websocket_connection.rb[132-149]
- rb/lib/selenium/webdriver/common/websocket_connection.rb[164-179]

## Proposed fix
1. When `frame_dropped?` is true, transition the connection to a closed/closing state and close the socket.
  - Avoid calling `close` directly from the socket thread (it joins threads); instead introduce a helper like `shutdown_socket!(reason)` that:
    - sets `@closing = true` under `@closing_mtx`
    - closes `socket` (rescuing `CONNECTION_ERRORS`)
    - optionally stores `@listener_error` for later reporting.
2. Make `send_cmd` fail fast when the listener has terminated due to a decode error (e.g., if `@listener_error` is set, or if `@socket_thread` is not alive and `@closing` is false), raising a `WebDriverError` with an actionable message rather than waiting 30s.
3. Consider emitting a debug log when the listener exits normally via this path, so user logs show a clear lifecycle event.

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

---

2. around block lacks ensure ✓ Resolved 📘 Rule violation ☼ Reliability

Description

The spec mutates global WebSocket.max_frame_size but restores it only after example.call, so an
exception/failing expectation can skip restoration and contaminate subsequent tests. This violates
the requirement to restore mutated global process state in an ensure/finally block.

Code

rb/spec/unit/selenium/webdriver/common/websocket_connection_spec.rb[R29-33]

+      around do |example|
+        original = WebSocket.max_frame_size
+        example.call
+        WebSocket.max_frame_size = original
+      end

Evidence

PR Compliance ID 9 requires restoring mutated global state using ensure/finally. The added
around hook sets WebSocket.max_frame_size, runs example.call, and restores afterward without
an ensure, so restoration can be skipped on error.

rb/spec/unit/selenium/webdriver/common/websocket_connection_spec.rb[29-33]
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
The spec changes global `WebSocket.max_frame_size` but does not guarantee it is restored if the example raises/fails, which can leak state into later tests.

## Issue Context
RSpec expectation failures raise exceptions; without `ensure`, the cleanup line may not run.

## Fix Focus Areas
- rb/spec/unit/selenium/webdriver/common/websocket_connection_spec.rb[29-33]

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

---

3. Global frame size persists 🐞 Bug ⛨ Security

Description

apply_frame_size_limit permanently raises the process-wide WebSocket.max_frame_size when it is
below 100MB, and close never restores the previous value. This can unexpectedly override a user’s
intentionally-lower global limit for the remainder of the process after the first DevTools/BiDi
connection.

Code

rb/lib/selenium/webdriver/common/websocket_connection.rb[R177-179]

+      def apply_frame_size_limit
+        WebSocket.max_frame_size = MAX_FRAME_SIZE if WebSocket.max_frame_size < MAX_FRAME_SIZE
+      end

Evidence

The code calls apply_frame_size_limit in initialization and conditionally sets the global
WebSocket.max_frame_size, but no restoration occurs in close, so the process-wide setting
persists beyond the connection’s lifecycle.

rb/lib/selenium/webdriver/common/websocket_connection.rb[43-57]
rb/lib/selenium/webdriver/common/websocket_connection.rb[177-179]
rb/lib/selenium/webdriver/common/websocket_connection.rb[59-80]
rb/spec/unit/selenium/webdriver/common/websocket_connection_spec.rb[29-33]

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.max_frame_size` is global module state; `WebSocketConnection#initialize` conditionally increases it, but no code restores the prior value when the connection is closed. If an application intentionally configured a smaller limit (e.g., for memory/DoS constraints), using Selenium DevTools/BiDi will override that policy for the remainder of the process.

## Issue Context
- The test suite explicitly saves/restores `WebSocket.max_frame_size`, which highlights that this state is global and sticky.
- Because it’s global, multiple concurrent connections need careful handling to avoid restoring too early.

## Fix Focus Areas
- rb/lib/selenium/webdriver/common/websocket_connection.rb[43-57]
- rb/lib/selenium/webdriver/common/websocket_connection.rb[59-80]
- rb/lib/selenium/webdriver/common/websocket_connection.rb[177-179]
- rb/spec/unit/selenium/webdriver/common/websocket_connection_spec.rb[29-33]

## Proposed fix
1. Track whether this instance modified the global value:
  - Store `@original_max_frame_size` and a flag like `@bumped_frame_size_limit` when raising it.
2. Restore safely on `close`:
  - Prefer a class-level refcount + mutex (e.g., `@@frame_size_bumps`) so restoration happens only when the last connection that bumped the value closes.
  - Restore only if `WebSocket.max_frame_size` is still equal to `MAX_FRAME_SIZE` (or the value you set) to avoid clobbering user changes made after initialization.
3. Add/update specs to validate restoration behavior (including multiple connections bumping concurrently if your test suite supports it).

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

---

Results up to commit 510ee4e

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Incomplete error log hint 🐞 Bug ◔ Observability

Description

frame_dropped? logs an error message ending with Raise WebSocket.max_frame_size= but provides no
suggested value (despite having a known MAX_FRAME_SIZE constant), making the remediation unclear
when a connection is terminated due to an oversized frame. This slows diagnosis of a production
failure mode where the listener closes itself on decode errors.

Code

rb/lib/selenium/webdriver/common/websocket_connection.rb[R170-171]

+        WebDriver.logger.error("WebSocket frame dropped (#{incoming_frame.error}): exceeds max_frame_size " \
+                               "(#{WebSocket.max_frame_size} bytes). Raise WebSocket.max_frame_size=", id: :ws)

Evidence

The log message currently ends with Raise WebSocket.max_frame_size= without providing any value;
the class defines MAX_FRAME_SIZE (100MB), which can be referenced to make the log actionable.

rb/lib/selenium/webdriver/common/websocket_connection.rb[38-42]
rb/lib/selenium/webdriver/common/websocket_connection.rb[170-172]

Agent prompt

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

### Issue description
The error log emitted when a frame is dropped ends with `Raise WebSocket.max_frame_size=` but does not include a recommended value (or example assignment), so it’s not actionable.

### Issue Context
`MAX_FRAME_SIZE` is defined (100MB) and the log already prints the current `WebSocket.max_frame_size`; operators need a concrete next step when this terminal condition occurs.

### Fix Focus Areas
- rb/lib/selenium/webdriver/common/websocket_connection.rb[170-172]

### Suggested change
Update the log string to include a recommended value, e.g.:
- `... Raise WebSocket.max_frame_size=#{MAX_FRAME_SIZE} (or higher).`
- Or `... Set WebSocket.max_frame_size to >= #{MAX_FRAME_SIZE}.`
Also remove the trailing `=` if no assignment example is included.

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

---

Results up to commit c85c861

🐞 Bugs (1) 📘 Rule violations (1) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. No cross-binding note for MAX_FRAME_SIZE 📘 Rule violation ≡ Correctness

Description

This PR changes user-visible DevTools/BiDi WebSocket behavior by raising the effective frame size
ceiling and closing the connection when a frame is dropped, but the changed code does not document
any comparison/alignment with other Selenium language bindings or an intentional divergence. This
risks inconsistent behavior across bindings without an auditable rationale.

Code

rb/lib/selenium/webdriver/common/websocket_connection.rb[R38-41]

+      # websocket-ruby defaults to a 20MB limit and silently drops larger
+      # frames, which can stall the listener. CDP payloads (e.g. large data:
+      # URLs) can exceed that, so raise the ceiling for our connections.
+      MAX_FRAME_SIZE = 100 * 1024 * 1024 # 100MB

Evidence

The checklist requires cross-language comparison/documentation when changing user-visible binding
behavior (PR Compliance ID 389265). The PR introduces a new MAX_FRAME_SIZE and applies it, and
adds logic to close the connection when an oversized frame is detected, but the surrounding comments
do not reference any cross-binding comparison or intentional divergence rationale.

Rule 389265: Compare cross-language bindings when changing user-visible behavior
rb/lib/selenium/webdriver/common/websocket_connection.rb[38-41]
rb/lib/selenium/webdriver/common/websocket_connection.rb[167-178]

Agent prompt

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

## Issue description
A user-visible behavior change was introduced in the Ruby binding (increasing the effective WebSocket frame size ceiling and closing the connection when an oversized frame is detected), but there is no nearby documentation indicating which other Selenium binding(s) were compared or why Ruby should differ.

## Issue Context
PR Compliance requires cross-language comparison evidence for user-visible behavior changes. Adding a short comment (or a doc/changelog note) referencing the binding(s) checked and the expected alignment/divergence is sufficient.

## Fix Focus Areas
- rb/lib/selenium/webdriver/common/websocket_connection.rb[38-41]
- rb/lib/selenium/webdriver/common/websocket_connection.rb[167-178]

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

---

2. send_cmd waits after close 🐞 Bug ☼ Reliability

Description

When frame_dropped? closes the socket and sets @closing, any send_cmd already waiting for its
response will still block until RESPONSE_WAIT_TIMEOUT and then raise a generic TimeoutError,
obscuring the actual connection failure. This can add a ~30s delay to failures triggered by
oversized/invalid frames.

Code

rb/lib/selenium/webdriver/common/websocket_connection.rb[R167-173]

+      def frame_dropped?
+        return false unless incoming_frame.error?
+
+        WebDriver.logger.error("WebSocket frame dropped (#{incoming_frame.error}): exceeds max_frame_size " \
+                               "(#{WebSocket.max_frame_size} bytes). Raise WebSocket.max_frame_size=", id: :ws)
+        close_socket
+        true

Evidence

The PR change makes frame_dropped? close the socket (via close_socket), and close_socket sets
@closing = true. However, send_cmd waits solely for a matching message ID to appear and does not
consult @closing, so if the socket is closed before the response arrives, the wait can only end by
timing out (Wait raises Error::TimeoutError).

rb/lib/selenium/webdriver/common/websocket_connection.rb[100-114]
rb/lib/selenium/webdriver/common/websocket_connection.rb[144-174]
rb/lib/selenium/webdriver/common/wait.rb[51-77]

Agent prompt

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

### Issue description
`frame_dropped?` now closes the socket and sets `@closing`, but `send_cmd` continues to wait for a response via `Wait#until` without checking connection/closing state. If the listener has been torn down (e.g., due to an oversized frame), the response can never arrive, so callers wait the full timeout and get a generic `Error::TimeoutError`.

### Issue Context
This PR correctly prevents the previous hot-spin by closing the websocket when `incoming_frame.error?` is set. However, callers already blocked inside `send_cmd` can still experience a long, misleading timeout.

### Fix Focus Areas
- rb/lib/selenium/webdriver/common/websocket_connection.rb[100-114]
- rb/lib/selenium/webdriver/common/websocket_connection.rb[144-174]

### Suggested fix
In `send_cmd`, make the wait condition fail fast when the connection is closing/closed. For example:
- Check `@closing` inside the `wait.until` block and raise a clearer error (e.g., `Error::WebDriverError, "WebSocket closed while waiting for response"`) when true.
- Optionally also guard with `socket.closed?` if available.
This ensures dropped-frame shutdowns surface promptly rather than timing out.

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

---

[qodo-code-review]

Code review by qodo was updated up to the latest commit 510ee4e

[qodo-code-review]

Code review by qodo was updated up to the latest commit c85c861

[qodo-code-review]

Code review by qodo was updated up to the latest commit f23041f

[Chandan25sharma]

Two complementary fixes:

apply_frame_size_limit — raises WebSocket.max_frame_size to 100 MB inside WebSocketConnection#initialize, not at load time, and only when the current limit is lower. Non-devtools users or anyone who already configured a larger value are unaffected. websocket-ruby exposes max_frame_size as a global-only setting; there is no per-instance option.

frame_dropped? — checks incoming_frame.error? (per-instance flag, not should_raise) after each read. When set, logs an actionable error and closes the connection cleanly instead of leaving a dead listener spinning on a stale buffer.

should_raise was left untouched — using incoming_frame.error? is the per-instance equivalent and avoids affecting unrelated connections.

Tested the fix locally. The two-part approach is correct:

apply_frame_size_limit in initialize prevents the hang for the common case (DevTools heap snapshots, large screenshots, etc.) without affecting users who already set a higher limit
frame_dropped? catches the remaining edge case cleanly — logs an actionable error and closes the connection instead of leaving the listener spinning at 100% CPU
The choice to use incoming_frame.error? over should_raise is the right call — should_raise is global and would affect all connections, error? is per-instance.

Unit tests cover both paths: the limit bump (raises when lower, leaves alone when higher) and the frame_dropped? shutdown. Builds cleanly on #17286.