You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
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"notindata:
raiseKeyError("Missing required field 'rpId' in credential data.")
rp_id=data["rpId"]
private_key=urlsafe_b64decode(f"{data['privateKey']}==")
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.
✅ Prevent invalid value reassignmentSuggestion Impact:The commit changed the code to only call message.get("message") when message is a dict (using a conditional expression), implicitly avoiding issues when message is not a dict. This aligns with the suggestion’s intent to prevent invalid accesses.
code diff:
if not isinstance(message, str):
value = message
- if isinstance(message, dict):- message = message.get("message")- else:- message = None+ message = message.get("message") if isinstance(message, dict) else None
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.
if not isinstance(message, str):
- value = message
if isinstance(message, dict):
+ value = message
message = message.get("message")
else:
message = None
[Suggestion processed]
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.
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.
cgoldberg
changed the title
Fix type annotation errors in virtual_authenticator.py and errorhandl…
[py] Fix type annotation error and raise clearer error message
Aug 14, 2025
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
🔗 Related Issues
Partially addresses #15697
💥 What does this PR do?
Fixes specific mypy type annotation errors in:
selenium/webdriver/common/virtual_authenticator.pyrpIdis missing inCredential.from_dict, ensuringrp_idis always a string.selenium/webdriver/remote/errorhandler.py.geton possibly-None values, so.getis only used on dictionaries.🔧 Implementation Notes
isinstance(value, dict)before using.getto resolve mypy union-attr errors.from_dictto requirerpIdand raise a clear error if missing, as discussed in project guidelines.💡 Additional Considerations
🔄 Types of changes
PR Type
Other
Description
Fix mypy type annotation errors in virtual authenticator
Add type checks before dictionary operations in error handler
Ensure required
rpIdfield validation with clear error messageDiagram Walkthrough
File Walkthrough
virtual_authenticator.py
Validate required rpId field in Credential.from_dictpy/selenium/webdriver/common/virtual_authenticator.py
rpIdretrieval with required field validationrpIdis missing from credential datarp_idis always a string typeerrorhandler.py
Add type checks before dictionary operationspy/selenium/webdriver/remote/errorhandler.py