[py] fix mypy errors with types and type hints by iampopovich · Pull Request #16827 · SeleniumHQ/selenium

iampopovich · 2026-01-01T22:44:52Z

User description

🔗 Related Issues

relates to #15697

💥 What does this PR do?

These changes help prevent runtime errors by providing clear error messages when required modules or configuration are missing. Additionally, there are minor enhancements to type annotations and imports.

Dependency and configuration validation:

Added explicit checks to ensure the devtools module is loaded before handling events or connecting sessions in cdp.py, raising a RuntimeError if not. [1] [2]
In remote_connection.py, added a check to ensure _client_config is initialized before making a request, raising a RuntimeError otherwise.
In remote/webdriver.py, added checks in start_devtools and _start_bidi to ensure the CDP version is present in capabilities, the CDP module is loaded, and that command_executor is a RemoteConnection instance, raising clear exceptions if any are missing. [1] [2]

Codebase clarity and maintainability:

Added a type annotation and import for the service attribute in LocalWebDriver, clarifying that it should be set by subclasses.

🔧 Implementation Notes

This pull request introduces several defensive programming improvements to the Selenium WebDriver codebase, primarily focused on validating dependencies and configuration before proceeding with key operations. Optional arguments were not changed to not to break backward compatibility.

💡 Additional Considerations

🔄 Types of changes

Cleanup (formatting, renaming)

PR Type

Bug fix, Enhancement

Description

Add runtime validation checks for CDP and BiDi module dependencies
Validate command_executor type and ClientConfig initialization before use
Add explicit error messages for missing CDP version in capabilities
Add Service type annotation to LocalWebDriver class

Diagram Walkthrough

flowchart LR
  A["WebDriver Methods"] -->|validate| B["CDP/BiDi Support"]
  C["RemoteConnection"] -->|check| D["ClientConfig"]
  E["CdpBase/CdpConnection"] -->|verify| F["devtools Module"]
  G["LocalWebDriver"] -->|annotate| H["Service Type"]

File Walkthrough

Relevant files

Error handling

cdp.py `Add devtools module validation in CDP methods` py/selenium/webdriver/common/bidi/cdp.py Added runtime check in `_handle_event()` to ensure devtools module is loaded before accessing util Added runtime check in `connect_session()` to ensure devtools module is loaded before accessing target Both checks raise RuntimeError with descriptive message if devtools is None	+4/-0
remote_connection.py `Add ClientConfig initialization validation` py/selenium/webdriver/remote/remote_connection.py Added runtime check in `_request()` method to validate _client_config is initialized Raises RuntimeError with clear message if ClientConfig is None before accessing keep_alive	+2/-0
webdriver.py `Add comprehensive validation for CDP and BiDi support` py/selenium/webdriver/remote/webdriver.py Added null check for cdp_version in `start_devtools()` with descriptive WebDriverException Added validation that cdp module is loaded before calling import_devtools() Added type check to ensure command_executor is RemoteConnection instance for CDP support Added type check to ensure command_executor is RemoteConnection instance for BiDi support Improved code formatting with blank lines for readability	+13/-1

Enhancement

webdriver.py `Add Service type annotation to LocalWebDriver` py/selenium/webdriver/common/webdriver.py Added import for Service class from selenium.webdriver.common.service Added type annotation for service attribute in LocalWebDriver class Clarifies that service should be set by subclasses before calling super().__init__()	+3/-0

…iver

…bilities

selenium/webdriver/remote/webdriver.py:1048: error: Item "None" of Module | None has no attribute "import_devtools" [union-attr]

selenium/webdriver/common/webdriver.py:42: error: "LocalWebDriver" has no attribute "service" [attr-defined]

…ection selenium/webdriver/common/bidi/cdp.py:303: error: Item "None" of "Any | None" has no attribute "util" [union-attr] selenium/webdriver/common/bidi/cdp.py:427: error: Item "None" of "Any | None" has no attribute "target" [union-attr]

…tion selenium/webdriver/remote/remote_connection.py:419: error: Item "None" of "ClientConfig | None" has no attribute "keep_alive" [union-attr]

qodo-code-review · 2026-01-02T01:21:02Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Detailed error text: Newly added exceptions include internal implementation details (e.g., module/component names and required call sequences) that may be considered too specific for user-facing error messages under this policy. Referred Code if cdp is None: raise WebDriverException("CDP module not loaded") self._devtools = cdp.import_devtools(version) if self._websocket_connection: return self._devtools, self._websocket_connection if self.caps["browserName"].lower() == "firefox": raise RuntimeError("CDP support for Firefox has been removed. Please switch to WebDriver BiDi.") if not isinstance(self.command_executor, RemoteConnection): raise WebDriverException("command_executor must be a RemoteConnection instance for CDP support") Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

qodo-code-review · 2026-01-02T01:22:23Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	validate CDP version format Add validation to ensure `cdp_version` is a string containing a dot before splitting, and use `split(".", 1)` for safer parsing. py/selenium/webdriver/remote/webdriver.py [1038-1041] cdp_version = self.caps.get("se:cdpVersion") -if cdp_version is None: - raise WebDriverException("CDP version not found in capabilities") -version = cdp_version.split(".")[0] +if not isinstance(cdp_version, str) or "." not in cdp_version: + raise WebDriverException("Invalid or missing CDP version in capabilities") +version = cdp_version.split(".", 1)[0] Apply / Chat Suggestion importance[1-10]: 6 __ Why: This suggestion improves the robustness of the code by adding validation for the `cdp_version` format, preventing potential `AttributeError` exceptions from malformed capabilities.	Low
	Move browser check to fail-fast Move the Firefox browser check to the beginning of the `start_devtools` method to fail-fast and avoid unnecessary processing for unsupported configurations. py/selenium/webdriver/remote/webdriver.py [1033-1064] def start_devtools(self) -> tuple[Any, WebSocketConnection]: + if self.caps["browserName"].lower() == "firefox": + raise RuntimeError("CDP support for Firefox has been removed. Please switch to WebDriver BiDi.") + global cdp import_cdp() if self.caps.get("se:cdp"): ws_url = self.caps.get("se:cdp") cdp_version = self.caps.get("se:cdpVersion") if cdp_version is None: raise WebDriverException("CDP version not found in capabilities") version = cdp_version.split(".")[0] else: version, ws_url = self._get_cdp_details() if not ws_url: raise WebDriverException("Unable to find url to connect to from capabilities") if cdp is None: raise WebDriverException("CDP module not loaded") self._devtools = cdp.import_devtools(version) if self._websocket_connection: return self._devtools, self._websocket_connection - if self.caps["browserName"].lower() == "firefox": - raise RuntimeError("CDP support for Firefox has been removed. Please switch to WebDriver BiDi.") if not isinstance(self.command_executor, RemoteConnection): raise WebDriverException("command_executor must be a RemoteConnection instance for CDP support") self._websocket_connection = WebSocketConnection( ws_url, self.command_executor.client_config.websocket_timeout, self.command_executor.client_config.websocket_interval, ) return self._devtools, self._websocket_connection `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 5 __ Why: This is a good refactoring suggestion that implements the fail-fast principle, improving code structure and efficiency for an unsupported configuration.	Low
	prefer duck typing for executor Replace the `isinstance` check on `self.command_executor` with a `hasattr` check for the `client_config` attribute to better align with duck typing principles. py/selenium/webdriver/remote/webdriver.py [1056-1057] -if not isinstance(self.command_executor, RemoteConnection): - raise WebDriverException("command_executor must be a RemoteConnection instance for CDP support") +if not hasattr(self.command_executor, "client_config"): + raise WebDriverException("command_executor must support client_config for CDP support") Apply / Chat Suggestion importance[1-10]: 4 __ Why: The suggestion to use duck typing by checking for an attribute instead of a strict type check is a valid style and flexibility improvement, but it has a minor impact as the code currently works correctly.	Low
Learned best practice	Centralize repeated guard logic Factor the repeated `devtools is None` guard into a small helper (e.g., `_require_devtools_loaded(fn_name)`) and call it from both `_handle_event()` and `connect_session()` to avoid duplication and keep error messaging consistent. py/selenium/webdriver/common/bidi/cdp.py [302-305] global devtools -if devtools is None: - raise RuntimeError("CDP devtools module not loaded. Call import_devtools() first.") +_require_devtools_loaded("_handle_event") event = devtools.util.parse_json_event(data) Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Reduce duplication by centralizing repeated validation/guard logic (single-source helper) instead of duplicating the same runtime checks in multiple methods.	Low
More

Copilot

Pull request overview

This pull request enhances type safety and error handling in the Selenium Python WebDriver by adding defensive validation checks and type annotations to prevent runtime errors when required modules or configurations are missing.

Key Changes:

Added null checks for CDP version and module initialization before attempting operations
Added validation that command_executor is a RemoteConnection instance before accessing its properties
Added type annotation for the service attribute in LocalWebDriver

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
`py/selenium/webdriver/remote/webdriver.py`	Added null checks for CDP version in capabilities and validation that command_executor is RemoteConnection before using CDP/BiDi features
`py/selenium/webdriver/remote/remote_connection.py`	Added runtime check to ensure `_client_config` is initialized before making requests
`py/selenium/webdriver/common/webdriver.py`	Added type annotation and import for `service` attribute to clarify it should be set by subclasses
`py/selenium/webdriver/common/bidi/cdp.py`	Added checks to ensure devtools module is loaded before handling events or connecting sessions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-02T01:23:08Z

py/selenium/webdriver/remote/webdriver.py

+        if cdp is None:
+            raise WebDriverException("CDP module not loaded")


The check for whether the CDP module is loaded should occur immediately after calling import_cdp() on line 1035, before any processing that depends on the cdp module. Moving this check earlier would catch the issue sooner and prevent unnecessary processing of CDP version information when the module isn't available.

cgoldberg · 2026-01-02T02:09:27Z

I added some of these changes to #16828 along with fixing the rest of the other type errors and some other fixes. I'm going to close this PR, but thanks for the help!

iampopovich added 7 commits January 2, 2026 00:58

[py] Validate command_executor type for CDP and BiDi support in WebDr…

c669241

…iver

[py] Improve error handling for missing CDP version in WebDriver capa…

a3033c3

…bilities

[py] Add error handling for missing CDP module in WebDriver

3f712a4

selenium/webdriver/remote/webdriver.py:1048: error: Item "None" of Module | None has no attribute "import_devtools" [union-attr]

[py] Add Service type annotation to LocalWebDriver class

e916f4f

selenium/webdriver/common/webdriver.py:42: error: "LocalWebDriver" has no attribute "service" [attr-defined]

[py] Add runtime check for uninitialized ClientConfig in RemoteConnec…

457c061

…tion selenium/webdriver/remote/remote_connection.py:419: error: Item "None" of "ClientConfig | None" has no attribute "keep_alive" [union-attr]

Fix formatting issues in WebDriver class for improved readability

4bf6ed6

iampopovich assigned cgoldberg Jan 1, 2026

selenium-ci added C-py Python Bindings B-devtools Includes everything BiDi or Chrome DevTools related labels Jan 1, 2026

iampopovich mentioned this pull request Jan 1, 2026

[🐛 Bug]: [py] Many type annotation errors need fixing #15697

Closed

iampopovich marked this pull request as ready for review January 2, 2026 01:20

Copilot AI review requested due to automatic review settings January 2, 2026 01:20

Copilot started reviewing on behalf of iampopovich January 2, 2026 01:21 View session

qodo-code-review bot added the Review effort 2/5 label Jan 2, 2026

Copilot AI reviewed Jan 2, 2026

View reviewed changes

cgoldberg mentioned this pull request Jan 2, 2026

[py] Fix remaining mypy errors and enable CI for type checking #16828

Closed

cgoldberg closed this Jan 2, 2026

cgoldberg mentioned this pull request Jan 3, 2026

[py] Fix remaining mypy errors and enable CI for type checking #16837

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[py] fix mypy errors with types and type hints#16827

[py] fix mypy errors with types and type hints#16827
iampopovich wants to merge 7 commits intoSeleniumHQ:trunkfrom
iampopovich:chore/fix-mypy-errors-with-types

iampopovich commented Jan 1, 2026 •

edited by qodo-code-review bot

Loading

Uh oh!

qodo-code-review bot commented Jan 2, 2026

Uh oh!

qodo-code-review bot commented Jan 2, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 2, 2026

Uh oh!

cgoldberg commented Jan 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		if cdp is None:
		raise WebDriverException("CDP module not loaded")

Uh oh!

Conversation

iampopovich commented Jan 1, 2026 • edited by qodo-code-review bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User description

🔗 Related Issues

💥 What does this PR do?

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

PR Type

Description

Diagram Walkthrough

File Walkthrough

Uh oh!

qodo-code-review bot commented Jan 2, 2026

PR Compliance Guide 🔍

Uh oh!

qodo-code-review bot commented Jan 2, 2026

PR Code Suggestions ✨

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

cgoldberg commented Jan 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

iampopovich commented Jan 1, 2026 •

edited by qodo-code-review bot

Loading