[py] Fix type annotation error and raise clearer error message

[Paresh-0007]

User description

🔗 Related Issues

Partially addresses #15697

💥 What does this PR do?

Fixes specific mypy type annotation errors in:

• selenium/webdriver/common/virtual_authenticator.py
  • Now raises a KeyError if rpId is missing in Credential.from_dict, ensuring rp_id is always a string.
• selenium/webdriver/remote/errorhandler.py
  • Added type checks before calling .get on possibly-None values, so .get is only used on dictionaries.

🔧 Implementation Notes

• Used isinstance(value, dict) before using .get to resolve mypy union-attr errors.
• Updated from_dict to require rpId and raise a clear error if missing, as discussed in project guidelines.

💡 Additional Considerations

• This is my first PR to Selenium—happy to make any changes or improvements!
• No new tests or docs needed as this is a type annotation/mypy error fix only.
• Will again work on same issue of got time from my college schedule but i'm feeling proud by contributing to this repo. Thanks !

🔄 Types of changes

• Cleanup (formatting, renaming)

---

PR Type

Other

---

Description

• Fix mypy type annotation errors in virtual authenticator

• Add type checks before dictionary operations in error handler

• Ensure required rpId field validation with clear error message

---

Diagram Walkthrough

flowchart LR
  A["Type annotation errors"] --> B["Add type checks"]
  B --> C["Fix virtual_authenticator.py"]
  B --> D["Fix errorhandler.py"]
  C --> E["Validate rpId field"]
  D --> F["Check dict before .get()"]

Loading

File Walkthrough

Relevant files

Bug fix

virtual_authenticator.py Validate required rpId field in Credential.from_dict          py/selenium/webdriver/common/virtual_authenticator.py Replace optional rpId retrieval with required field validation Raise KeyError if rpId is missing from credential data Ensure rp_id is always a string type +3/-1      errorhandler.py Add type checks before dictionary operations                          py/selenium/webdriver/remote/errorhandler.py Add isinstance checks before calling .get() on potentially None values Prevent union-attr mypy errors by validating dict type Handle message extraction safely with type validation +7/-2

---

[CLAassistant]

All committers have signed the CLA.

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Backward Compatibility Changing rpId from optional to required and raising KeyError may break existing callers that relied on missing rpId. Verify callers and upstream data always include rpId, or consider a more descriptive exception type. if "rpId" not in data: raise KeyError("Missing required field 'rpId' in credential data.") rp_id = data["rpId"] private_key = urlsafe_b64decode(f"{data['privateKey']}==") Message Handling Logic When a non-dict non-str message is encountered, message is set to None, potentially losing error context. Validate this behavior against expected WebDriver responses to ensure no regression in error messages. if not isinstance(message, str): value = message if isinstance(message, dict): message = message.get("message") else: message = None else:

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Prevent invalid value reassignment Avoid overwriting value with a non-dict message, which can lead to later .get() accesses failing. Only reassign value when message is a dict; otherwise keep value intact. py/selenium/webdriver/remote/errorhandler.py [174-179] if not isinstance(message, str): - value = message if isinstance(message, dict): + value = message message = message.get("message") else: message = None Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that value should only be reassigned if message is a dictionary, preventing potential AttributeError exceptions on subsequent .get() calls on value.	Medium
	Enforce rpId string type Validate the type of rpId before use to avoid propagating non-string values, which can break downstream logic and violate the rp_id: str contract. Coerce to string or raise a clear error if the value is not a string. py/selenium/webdriver/common/virtual_authenticator.py [183-185] if "rpId" not in data: raise KeyError("Missing required field 'rpId' in credential data.") -rp_id = data["rpId"] +rp_id_raw = data["rpId"] +if not isinstance(rp_id_raw, str): + raise TypeError("Field 'rpId' must be a string.") +rp_id = rp_id_raw Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that rpId should be a string and adds a type check, which improves the code's robustness against malformed input data.	Low
Update

[Paresh-0007]

@olleolleolle hello sir
its my first contribution in selenium , please ignore any mistakes if found and also let me know if u have any feedback on me.

[Paresh-0007]

will try to do more better !

[navin772]

Should we directly use data["rpId"] instead of raising an error since it is a required field and is expected to be always present?

[Paresh-0007]

Thank you for the suggestion! I added the explicit check to provide a clearer error message if rpId is missing, but I agree that directly accessing data["rpId"] would automatically raise a KeyError if it’s absent. I’m happy to update this to remove the manual check and rely on the built-in exception for missing keys, if that’s preferred for consistency with the rest of the codebase.

[navin772]

Sure

[navin772]

This instance check is not required since we are taking care of it below. It won't raise any mypy type errors.

[Paresh-0007]

I’ll remove the redundant isinstance check here since the code already ensures the right type, and it shouldn’t affect mypy or runtime behavior.

[navin772]

We can replace with a better and shorter check:

		message = message.get("message") if message is not None else None

[Paresh-0007]

Great suggestion! I’ll update this to use the shorter one-liner as you proposed. It’s definitely more concise and readable.

[cgoldberg]

Those aren't equivalent. You would want:

message = message.get("message") if isinstance(message, dict) else None