[🚀 Feature] [py]: Implement BiDi add/remove_request handler

[jkim2492]

User description

Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Implementation of add_request_handler and remove_request_handler as described in #13993. These methods directly require BiDi objects

• network.AddIntercept
• network.BeforeRequestSent
• network.ContinueRequest
• network.RemoveRequest
• browsingContext.Navigate

and other added objects are dependencies of objects listed above.

Motivation and Context

#13993

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

• Implemented BiDi network interception and request handling, including adding and removing request handlers.
• Introduced base classes for BiDi objects with JSON serialization and deserialization capabilities.
• Added browsing context navigation classes and network management for remote WebDriver.
• Integrated network management into WebDriver, providing a network property for accessing network features.
• Added tests to verify the functionality of BiDi network request handlers, including multiple intercepts and their removal.

---

Changes walkthrough 📝

	Relevant files
Enhancement	bidi.py Define base classes for BiDi objects                                          py/selenium/webdriver/common/bidi/bidi.py Introduced base classes for BiDi objects. Implemented JSON serialization and deserialization methods. Added BidiEvent and BidiCommand classes. +59/-0    browsing_context.py Add browsing context navigation classes                                    py/selenium/webdriver/common/bidi/browsing_context.py Added Navigate and NavigateParameters classes. Defined BrowsingContext and ReadinessState types. +42/-0    network.py Implement network interception and request handling            py/selenium/webdriver/common/bidi/network.py Implemented classes for network interception and request continuation. Added support for URL patterns and intercept phases. Introduced Network class for managing network operations. +213/-0  script.py Extend script module with stack trace classes                        py/selenium/webdriver/common/bidi/script.py Added StackFrame and StackTrace classes. Extended existing script functionalities with BiDi objects. +14/-0    network.py Implement network management for remote WebDriver                py/selenium/webdriver/remote/network.py Implemented network management for remote WebDriver. Added methods for adding and removing request handlers. Introduced context management for network operations. +125/-0  webdriver.py Integrate network management into WebDriver                            py/selenium/webdriver/remote/webdriver.py Integrated network management into WebDriver. Added network property for accessing network features. +9/-0
Tests	bidi_network_tests.py Add tests for BiDi network request handlers                            py/test/selenium/webdriver/common/bidi_network_tests.py Added tests for BiDi network request handlers. Verified multiple intercepts and their removal. +57/-0

---

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

[CLAassistant]

All committers have signed the CLA.

[codiumai-pr-agent-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

🧪 PR contains tests

🔒 Security concerns Network Interception: The network request interception functionality could potentially be used to modify sensitive data or inject malicious content if not properly secured. The request handler implementation should validate and sanitize modified requests to prevent request tampering or injection attacks.

⚡ Recommended focus areas for review Error Handling The network request handling code lacks proper error handling for failed requests or invalid responses. Should validate responses and handle exceptions. Resource Management Network listeners and intercepts may not be properly cleaned up if an error occurs during execution. Should ensure cleanup in error cases. Type Safety Several type hints use Optional without proper null checks in the code. This could lead to runtime errors if null values are passed.

[codiumai-pr-agent-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible bug	Create a copy of dictionary keys before iteration to prevent modification-during-iteration errors Use a copy of the intercepts keys when iterating to prevent potential runtime errors from modifying the dictionary during iteration. py/selenium/webdriver/remote/network.py [119-121] async def clear_intercepts(self): - for intercept in self.intercepts: + for intercept in list(self.intercepts.keys()): await self.remove_intercept(intercept) Apply this suggestion Suggestion importance[1-10]: 9 Why: This fixes a potential runtime error that could occur when modifying a dictionary while iterating over it, which is a common source of bugs in Python. This is a critical improvement for code reliability.	9
Possible issue	Add error handling for WebSocket connection failures to prevent crashes and improve reliability Add error handling for when the WebSocket connection fails or is interrupted during network operations. This will prevent potential crashes and provide better error feedback. py/selenium/webdriver/remote/network.py [47-54] -async with open_cdp(ws_url) as conn: - self.conn = conn - self.bidi_network = network.Network(self.conn) - async with trio.open_nursery() as nursery: - self.nursery = nursery - yield +try: + async with open_cdp(ws_url) as conn: + self.conn = conn + self.bidi_network = network.Network(self.conn) + async with trio.open_nursery() as nursery: + self.nursery = nursery + yield +except (ConnectionError, trio.BrokenResourceError) as e: + raise RuntimeError("Failed to establish or maintain WebSocket connection") from e Apply this suggestion Suggestion importance[1-10]: 8 Why: Adding error handling for WebSocket connection failures is crucial for preventing application crashes and providing meaningful error messages. This is particularly important for network operations that depend on stable connections.	8
Best practice	Implement proper resource cleanup to prevent resource leaks and ensure clean shutdown Ensure proper cleanup of resources by implementing an async cleanup method to handle WebSocket connections and listeners when they're no longer needed. py/selenium/webdriver/remote/network.py [119-121] async def clear_intercepts(self): - for intercept in self.intercepts: + for intercept in list(self.intercepts.keys()): await self.remove_intercept(intercept) + self.conn = None + self.bidi_network = None + self.nursery = None Apply this suggestion Suggestion importance[1-10]: 7 Why: Proper cleanup of WebSocket connections and other resources is important to prevent memory leaks and ensure system resources are released properly. This is a significant improvement for long-running applications.	7
	Add type annotations to collection initializations for better code clarity and type safety Initialize collections in init with explicit types to prevent potential type-related issues and improve code clarity. py/selenium/webdriver/remote/network.py [35-38] def __init__(self, driver): self.driver = driver - self.listeners = {} - self.intercepts = defaultdict(lambda: {"event_name": None, "handlers": []}) + self.listeners: dict[str, any] = {} + self.intercepts: defaultdict[str, dict] = defaultdict(lambda: {"event_name": None, "handlers": []}) Apply this suggestion Suggestion importance[1-10]: 3 Why: While adding type hints can improve code clarity and maintainability, this is a relatively minor enhancement that doesn't affect functionality. The existing code is already clear and working correctly.	3

💡 Need additional feedback ? start a PR chat

[shbenzer]

@AutomatedTester What do you think?

[AutomatedTester]

Is this not working against Firefox and Edge?

[jkim2492]

It was working with edge. I've pushed a commit to remove xfail_edge.

As for firefox, it fails with "unsupported operation" at network.continueRequest. I see mentions of network.continueRequest at https://wiki.mozilla.org/Firefox/Meeting/9-July-2024 , but I recall experiencing a similar problem with firefox when testing #14345.

[AutomatedTester]

I really like this implementation and is showing a great direction.

Things that I think we need to think about (and I don't think it should block this) is how can we simplify the migration for folks using Classic to BiDi.

[jkim2492]

@AutomatedTester

Things that I think we need to think about (and I don't think it should block this) is how can we simplify the migration for folks using Classic to BiDi.

Yup. I agree.

[jkim2492]

@AutomatedTester

I also wanted to point out a potential naming issue for BiDi objects.

Our implementations of BiDi objects (including this PR) include class <module name> inside the respective modules to hold function definitions

• Example for "Script".
• Example from this PR

However, some BiDi modules contain types that have the same name as the module which creates a naming conflict.

• Spec for browsingContext.Browsingcontext
• Implementation for browsingContext.Browsingcontext

In the particular case of Browsingcontext = str, we can workaround this issue by substituting references to Browsingcontext with str, but this is not consistent, and also not applicable for more complex types.