-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
Description
Description
After adding PR #16174 to Selenium 4.36.0, our tests are failing with the following stacktrace:
File ".../base.py", line 1149, in save_last_security_key
credentials = self.selenium.driver.get_credentials()
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File ".../python3.12/site-packages/selenium/webdriver/common/virtual_authenticator.py", line 206, in wrapper
return func(self, *args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File ".../python3.12/site-packages/selenium/webdriver/common/virtual_authenticator.py", line 220, in wrapper
return func(self, *args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File ".../python3.12/site-packages/selenium/webdriver/remote/webdriver.py", line 1531, in get_credentials
return [Credential.from_dict(credential) for credential in credential_data["value"]]
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File ".../python3.12/site-packages/selenium/webdriver/common/virtual_authenticator.py", line 184, in from_dict
rp_id = data["rpId"]
The problem is, that the pull request claims that
No new tests or docs needed as this is a type annotation/mypy error fix only.
However, when the rpId
is missing, the code now throws an error instead of setting it to None
in the resulting Credential
. This causes a breaking change.
The rpId
field can be missing as when missing, it should default to the current domain.
I therefore propose to revert the mentioned pull request. There are other options to fix this, e. g. setting the rpId
to the current domain inside Selenium, but this fix would be now harder to implement and even harder to correctly test to not cause issues in other codebases.
Reproducible Code
It is hard to provide reproducible code for me as this causes in issue in tests for our own application.
ℹ️ Last known working version: 4.35.0