[py] Add low-level sync API to use DevTools

[p0deje]

User description

Initial implementation of #13975. This provides a low-level API that:

1. establishes WebSocket connection to the driver;
2. use the connection to send CDP commands and receive results;
3. use the connection to subscribe to events;

Once this is merged, I'll work on the subsequent PRs that provide high-level API backed by CDP for now (network interception, log events, basic authentication, etc).

The implementation can also be used to send BiDi commands and subscribe to events once BiDi is implemented in Python (assuming we follow the same pattern as for CDP).

---

PR Type

Enhancement, Tests

---

Description

• Added start_devtools method in webdriver.py to establish a WebSocket connection and interact with DevTools.
• Implemented WebSocketConnection class to manage WebSocket connections, send and receive messages, and handle events.
• Added tests for DevTools WebSocket connection and console message logging.
• Added websocket-client dependency to requirements.txt.
• Enhanced event_class decorator to add event_class attribute to classes.

---

Changes walkthrough 📝

	Relevant files
Enhancement	generate.py Add `event_class` attribute to event classes.                        py/generate.py Added event_class attribute to classes decorated with event_class decorator. +1/-0      webdriver.py Add `start_devtools` method for WebSocket connection to DevTools. py/selenium/webdriver/remote/webdriver.py Imported WebSocketConnection. Added start_devtools method to establish WebSocket connection and interact with DevTools. Initialized _websocket_connection attribute. +29/-0    websocket_connection.py Implement `WebSocketConnection` class for WebSocket communication. py/selenium/webdriver/remote/websocket_connection.py Created WebSocketConnection class to manage WebSocket connections. Implemented methods for sending and receiving messages, handling events, and managing connection state. +107/-0
Tests	devtools_tests.py Add tests for DevTools WebSocket connection and console logging. py/test/selenium/webdriver/common/devtools_tests.py Added tests for DevTools WebSocket connection and console message logging. +38/-0
Dependencies	requirements.txt Add `websocket-client` dependency.                                              py/requirements.txt Added websocket-client dependency. +1/-0

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[codiumai-pr-agent-pro]

PR Description updated to latest commit (929f205)

• Copy walkthrough table to "Files Changed" Tab

[codiumai-pr-agent-pro]

PR Review 🔍

⏱️ Estimated effort to review [1-5]	3, because the PR involves multiple components including a new WebSocket connection class, modifications to the WebDriver class, and the addition of tests. The complexity of the WebSocket operations and the integration with existing systems increase the review effort.
🧪 Relevant tests	Yes
⚡ Possible issues	Thread Safety: The WebSocketConnection class uses threads and shared state without visible locks or thread-safe data structures, which might lead to race conditions or data corruption in a multi-threaded environment.
	Error Handling: The start_devtools method in webdriver.py and several methods in WebSocketConnection lack comprehensive error handling, which might lead to unhandled exceptions during WebSocket operations.
	Resource Leak: The WebSocketConnection class does not appear to handle exceptions during WebSocket setup, which could lead to resource leaks if the connection is not properly closed on errors.
🔒 Security concerns	No

[p0deje]

I'm not sure about the naming here, any suggestions are welcome.

[diemol]

Java calls this getDevTools(), I am not sure what is the idiomatic fit for Python.

[nir-tal-talkspace]

to start a devtools session in Java -> devTools.createSession();

[p0deje]

This method both gets DevTools (loads the proper version) and creates session, then returns both the loaded devtools module and the established connection. start_devtools is the best possible name I came up with for these 2 operations. We could of course split them into 2 separate methods...

[codiumai-pr-agent-pro]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible issue	Add exception handling for WebSocket connection initialization Add exception handling for the WebSocket connection to manage potential connection errors and ensure the application can handle such scenarios gracefully. py/selenium/webdriver/remote/websocket_connection.py [81-83] -self._ws = WebSocketApp(self.url, on_open=on_open, on_message=on_message, on_error=on_error) -self._ws_thread = Thread(target=run_socket) -self._ws_thread.start() +try: + self._ws = WebSocketApp(self.url, on_open=on_open, on_message=on_message, on_error=on_error) + self._ws_thread = Thread(target=run_socket) + self._ws_thread.start() +except Exception as e: + logger.error(f"Failed to start WebSocket connection: {e}") + self._started = False Suggestion importance[1-10]: 8 Why: Adding exception handling around the WebSocket connection initialization is crucial to manage potential errors gracefully. This suggestion correctly identifies a significant potential issue and provides a practical improvement to enhance the robustness of the connection setup.	8
	Add a timeout to the WebSocket thread join method to prevent indefinite hanging To ensure the WebSocket thread is properly terminated, add a timeout to the join method in the close function, which will prevent the application from hanging indefinitely. py/selenium/webdriver/remote/websocket_connection.py [29] -self._ws_thread.join() +self._ws_thread.join(timeout=10) Suggestion importance[1-10]: 7 Why: Adding a timeout to the join method of the WebSocket thread is a practical suggestion to prevent the application from hanging if the thread does not terminate. This change improves the reliability of the close method, ensuring that resources are properly cleaned up even in error scenarios.	7
Maintainability	Encapsulate global variables within the class to avoid potential side effects To avoid potential issues with global variables, consider encapsulating the devtools and cdp variables within a class or instance. This will help prevent unintended side effects and improve code maintainability. py/selenium/webdriver/remote/webdriver.py [1024-1029] -global devtools if self._websocket_connection: - return devtools, self._websocket_connection + return self.devtools, self._websocket_connection else: - global cdp - import_cdp() + self.import_cdp() Suggestion importance[1-10]: 7 Why: The suggestion to encapsulate global variables like devtools and cdp within a class is valid for improving maintainability and avoiding side effects. However, the provided 'improved_code' does not fully address the necessary changes, such as initializing these variables within the class constructor or managing their scope correctly.	7
Enhancement	Make timeout and interval parameters configurable for the _wait_until method Instead of using a fixed timeout and interval for _wait_until, consider making these parameters configurable to allow for more flexibility and adaptability in different environments. py/selenium/webdriver/remote/websocket_connection.py [11-12] -_response_wait_timeout = 30 -_response_wait_interval = 0.1 +def __init__(self, url, response_wait_timeout=30, response_wait_interval=0.1): + self._response_wait_timeout = response_wait_timeout + self._response_wait_interval = response_wait_interval Suggestion importance[1-10]: 6 Why: Making the timeout and interval parameters configurable in the _wait_until method is a good enhancement for flexibility. However, the impact of this change is moderate as it primarily affects internal behavior and does not directly influence the API's external usability or functionality.	6

[p0deje]

A general comment that applies to this whole PR - do we want to have typing properly implemented? I'm not familiar with the Python typing system so I did what was the easiest for me. I can re-iterate and add types if that's preferred.

[diemol]

I would invest in typing for the BiDi implementation. Not much in the CDP one.

[p0deje]

Types are already available in CDP, the question is should I add types to the WebSocketConnection class.

[codiumai-pr-agent-pro]

CI Failure Feedback 🧐

(Checks updated until commit 3efca1b)

Action: Python / Lint

Failed stage: Test with tox [❌]

Failure summary: The action failed due to linting errors detected by flake8. The specific issues are: selenium/webdriver/remote/websocket_connection.py:47:38: Undefined name _response_wait_timeout (F821). selenium/webdriver/remote/websocket_connection.py:78:19: Undefined name InternalError (F821). test/selenium/webdriver/common/devtools_tests.py:19:1: Imported but unused selenium.webdriver.common.by.By (F401). test/selenium/webdriver/common/devtools_tests.py:20:1: Imported but unused selenium.webdriver.common.log.Log (F401). test/selenium/webdriver/common/devtools_tests.py:21:1: Imported but unused selenium.webdriver.support.expected_conditions as EC (F401).

Relevant error logs: 1: ##[group]Operating System 2: Ubuntu ... 205: linting-ci: freeze /home/runner/work/selenium/selenium/py> python -m pip freeze --all 206: linting-ci: black==24.1.1,charset-normalizer==3.3.2,click==8.1.7,docformatter==1.7.5,flake8==6.1.0,flake8-typing-imports==1.14.0,isort==5.13.2,mccabe==0.7.0,mypy-extensions==1.0.0,packaging==24.0,pathspec==0.12.1,pip==24.0,platformdirs==4.2.2,pycodestyle==2.11.1,pyflakes==3.1.0,setuptools==69.5.1,tomli==2.0.1,typing_extensions==4.11.0,untokenize==0.1.1,wheel==0.43.0 207: linting-ci: commands[0] /home/runner/work/selenium/selenium/py> isort --check-only --diff selenium/ test/ conftest.py 208: linting-ci: commands[1] /home/runner/work/selenium/selenium/py> black --check --diff selenium/ test/ conftest.py -l 120 209: All done! ✨ 🍰 ✨ 210: 217 files would be left unchanged. 211: linting-ci: commands[2] /home/runner/work/selenium/selenium/py> flake8 selenium/ test/ --min-python-version=3.8 212: selenium/webdriver/remote/websocket_connection.py:47:38: F821 undefined name '_response_wait_timeout' 213: selenium/webdriver/remote/websocket_connection.py:78:19: F821 undefined name 'InternalError' 214: test/selenium/webdriver/common/devtools_tests.py:19:1: F401 'selenium.webdriver.common.by.By' imported but unused 215: test/selenium/webdriver/common/devtools_tests.py:20:1: F401 'selenium.webdriver.common.log.Log' imported but unused 216: test/selenium/webdriver/common/devtools_tests.py:21:1: F401 'selenium.webdriver.support.expected_conditions as EC' imported but unused 217: linting-ci: exit 1 (1.57 seconds) /home/runner/work/selenium/selenium/py> flake8 selenium/ test/ --min-python-version=3.8 pid=2616 218: linting-ci: FAIL code 1 (7.41=setup[3.77]+cmd[0.44,1.63,1.57] seconds) 219: evaluation failed :( (7.48 seconds) 220: ##[error]Process completed with exit code 1.

---

✨ CI feedback usage guide:

---

The CI feedback tool (/checks) automatically triggers when a PR has a failed check.
The tool analyzes the failed checks and provides several feedbacks:

• Failed stage
• Failed test name
• Failure summary
• Relevant error logs

In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR:

/checks "https://github.com/{repo_name}/actions/runs/{run_number}/job/{job_number}"

where {repo_name} is the name of the repository, {run_number} is the run number of the failed check, and {job_number} is the job number of the failed check.

Configuration options

• enable_auto_checks_feedback - if set to true, the tool will automatically provide feedback when a check is failed. Default is true.
• excluded_checks_list - a list of checks to exclude from the feedback, for example: ["check1", "check2"]. Default is an empty list.
• enable_help_text - if set to true, the tool will provide a help message with the feedback. Default is true.
• persistent_comment - if set to true, the tool will overwrite a previous checks comment with the new feedback. Default is true.
• final_update_message - if persistent_comment is true and updating a previous checks message, the tool will also create a new message: "Persistent checks updated to latest commit". Default is true.

See more information about the checks tool in the docs.