[rb] BiDi Network: add_request_handler, remove_request_handler, clear_request_handlers

[aguspe]

Description

This PR adds the following:

• The add, remove, and clear request handlers to the common Network class
• The continue_request method to the BiDi Network class
• Tests to cover all the methods

Tests executed locally:

Motivation and Context

Based on #13993 this PR follows the defined API methods implemented for the Ruby binding, now is possible to call driver.network.add_request_handler

This PR continues the work done in #14523

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

PR Type

Enhancement, Tests

---

Description

• Added continue_with_request method to the BiDi Network class for handling network requests.
• Implemented methods to add, remove, and clear request handlers in the common Network class.
• Updated type signatures to reflect new methods in both BiDi and common Network classes.
• Added integration tests to verify the functionality of request handler management and continue_with_request.

---

Changes walkthrough 📝

	Relevant files
Enhancement	network.rb Add continue_with_request method and fix event key              rb/lib/selenium/webdriver/bidi/network.rb Added continue_with_request method to handle network requests. Corrected the case for fetch_error in the EVENTS hash. +13/-1    network.rb Implement request handler management in Network class        rb/lib/selenium/webdriver/common/network.rb Introduced request handler management methods: add, remove, and clear. Added request_callbacks attribute for managing request handlers. +24/-1    network.rbs Update type signature for continue_with_request                    rb/sig/lib/selenium/webdriver/bidi/network.rbs Added type signature for continue_with_request. +2/-0      network.rbs Add type signatures for request handler methods                    rb/sig/lib/selenium/webdriver/common/network.rbs Added type signatures for request handler management methods. +8/-0
Tests	network_spec.rb Add integration test for continue_with_request                      rb/spec/integration/selenium/webdriver/bidi/network_spec.rb Added integration test for continue_with_request method. +14/-0    network_spec.rb Add tests for request handler management                                  rb/spec/integration/selenium/webdriver/network_spec.rb Added tests for adding, removing, and clearing request handlers. +27/-0

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Potential Bug The continue_with_request method accepts optional parameters but doesn't handle nil values properly. This could lead to sending nil values in the BiDi command. Error Handling The add_request_handler method doesn't handle potential errors from the BiDi commands or callbacks. Consider adding error handling. Type Definition The return type for remove_request_handler is specified as untyped but should match the implementation which returns nil

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible bug	Add guard clause to prevent potential nil reference errors when removing handlers Add validation to ensure request_id exists before attempting to remove a request handler to prevent potential nil errors. rb/lib/selenium/webdriver/common/network.rb [64-68] def remove_request_handler(id) - intercept = @request_callbacks[id] + return unless (intercept = @request_callbacks[id]) @network.remove_intercept(intercept['intercept']) @request_callbacks.delete(id) end Apply this suggestion Suggestion importance[1-10]: 8 Why: This is a critical defensive programming improvement that prevents potential runtime errors when attempting to remove non-existent handlers.	8
Enhancement	Remove nil parameters from command arguments to improve protocol communication efficiency Filter out nil values from the command arguments to avoid sending unnecessary parameters to the BiDi protocol. rb/lib/selenium/webdriver/bidi/network.rb [64-72] -@bidi.send_cmd( - 'network.continueWithRequest', +params = { request: args[:request_id], 'body' => args[:body], 'cookies' => args[:cookies], 'headers' => args[:headers], 'method' => args[:method], 'url' => args[:url] -) +}.compact +@bidi.send_cmd('network.continueWithRequest', **params) Apply this suggestion Suggestion importance[1-10]: 7 Why: The suggestion improves robustness by filtering out nil values before sending commands, preventing potential protocol issues and reducing unnecessary data transmission.	7
Best practice	Use unique identifiers for handler management to prevent potential key collisions Consider returning the handler ID as a UUID or other unique identifier instead of using the raw request_id to prevent potential collisions. rb/lib/selenium/webdriver/common/network.rb [52-62] def add_request_handler intercept = @network.add_intercept(phases: [BiDi::Network::PHASES[:before_request]]) request_id = @network.on(:before_request) do |event| request_id = event['requestId'] @network.continue_with_request(request_id: request_id) end - @request_callbacks[request_id] = intercept - request_id + handler_id = SecureRandom.uuid + @request_callbacks[handler_id] = intercept + handler_id end Apply this suggestion Suggestion importance[1-10]: 6 Why: Using UUIDs as handler IDs is a good practice to prevent potential collisions, though the current implementation using request_id might be sufficient for most use cases.	6

💡 Need additional feedback ? start a PR chat

[p0deje]

@aguspe This looks great, can we fix the CI and merge?

[aguspe]

@aguspe This looks great, can we fix the CI and merge?

Thank you @p0deje, I get my new laptop on monday and fix the CI on this PR

[aguspe]

@p0deje Now only the RBE checks are failing but this is due to a java test, I will merge it after the last PR executes and keep working on the Network BiDi for Ruby

  Stats over 2 runs: max = 37.0s, min = 36.5s, avg = 36.7s, dev = 0.2s
  /home/runner/.bazel/execroot/_main/bazel-out/k8-fastbuild/testlogs/java/test/org/openqa/selenium/ClickTest-firefox-beta/test.log
  /home/runner/.bazel/execroot/_main/bazel-out/k8-fastbuild/testlogs/java/test/org/openqa/selenium/ClickTest-firefox-beta/test_attempts/attempt_1.log

Executed 13 out of 2167 tests: 2166 tests pass and 1 fails remotely.