-
Notifications
You must be signed in to change notification settings - Fork 572
fix UnknownCommandError #868
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| # TODO: Remove the fallback | ||
| return self.mark_extension_absence(ext_name).execute(Command.GET_CURRENT_ACTIVITY)['value'] | ||
| except WebDriverException as e: | ||
| if isinstance(e, UnknownMethodException) or self.is_missing_command(e.msg): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could it be the server throws anything else other than UnknownMethodException in case of missing method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UnknownMethodException is only raised by
python-client/appium/webdriver/webdriver.py
Lines 505 to 517 in a369b6f
| def assert_extension_exists(self, ext_name: str) -> 'WebDriver': | |
| """ | |
| Verifies if the given extension is not present in the list of absent extensions | |
| for the given driver instance. | |
| This API is designed for private usage. | |
| :param ext_name: extension name | |
| :return: self instance for chaining | |
| :raise UnknownMethodException: If the extension has been marked as absent once | |
| """ | |
| if ext_name in self._absent_extensions: | |
| raise UnknownMethodException() | |
| return self |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This must be a bug in the Python client itself. I believe it does not properly map the exception raised by the server because this works as expected with Java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, then, i guess in selenium client side..? Quickly checked with ruby client. it also actually handled as Selenium::WebDriver::Error::UnknownCommandError. I'll take a look this weekend
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries, I have already prepared a fix: #869
Thanks for pointing this out, I did not know it's so broken. Seems like the actual error handling code in selenium client has not been updated since ages and only supports responses in JWP format
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, gotcha. Then this pr is no longer needed
| def mark_extension_absence(self: T, ext_name: str) -> T: | ||
| ... | ||
|
|
||
| def is_missing_command(self: T, error_msg: Union[str, None]) -> bool: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
protocols must not contain method implementations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea, i noticed the implementation was in webdriver.py
for #865
Still initial implementation.