selenium icon indicating copy to clipboard operation
selenium copied to clipboard

[🐛 Bug]: [Python] WebDriver constructor must not assume that BaseOptions instance has _ignore_local_proxy property

Open mykola-mokhnach opened this issue 3 years ago • 8 comments

What happened?

I'm talking about https://github.com/SeleniumHQ/selenium/blob/27ff5866dc1f50e4a01733fb6b0f3e734a72363b/py/selenium/webdriver/remote/webdriver.py#L256

The typing info says options: Union[BaseOptions, List[BaseOptions]] = None, while _ignore_local_proxy property is only defined for ArgOptions class. This means any code that inherits Options from BaseOptions rather than from ArgOptions would fail with missing attribute error (which is also happening for the Appium Python client)

How can we reproduce the issue?

Pass any custom class inherited from BaseOptions rather than from ArgOptions to the WebDriver constructor

Relevant log output

if options:
                capabilities = options.to_capabilities()
>               _ignore_local_proxy = options._ignore_local_proxy
E               AttributeError: 'UiAutomator2Options' object has no attribute '_ignore_local_proxy'


### Operating System

macOS

### Selenium version

Python 3.9

### What are the browser(s) and version(s) where you see this issue?

Appium

### What are the browser driver(s) and version(s) where you see this issue?

Any

### Are you using Selenium Grid?

_No response_

mykola-mokhnach avatar Jun 09 '22 19:06 mykola-mokhnach

@mykola-mokhnach, thank you for creating this issue. We will troubleshoot it as soon as we can.


Info for maintainers

Triage this issue by using labels.

If information is missing, add a helpful comment and then I-issue-template label.

If the issue is a question, add the I-question label.

If the issue is valid but there is no time to troubleshoot it, consider adding the help wanted label.

If the issue requires changes or fixes from an external project (e.g., ChromeDriver, GeckoDriver, MSEdgeDriver, W3C), add the applicable G-* label, and it will provide the correct link and auto-close the issue.

After troubleshooting the issue, please add the R-awaiting answer label.

Thank you!

github-actions[bot] avatar Jun 09 '22 19:06 github-actions[bot]

I assume such issue could be easily caught with mypy automatically

mykola-mokhnach avatar Jun 09 '22 19:06 mykola-mokhnach

It could but we have a lot of work to get that going, types at the moment are quite frankly very messy, any PRs introducing improved types would be greatly appreciated. tox -e mypy exists but its extremely broken and not enforced in CI at all, it's a mammoth task to get everything typed etc

symonk avatar Jun 09 '22 19:06 symonk

This issue is stale because it has been open 280 days with no activity. Remove stale label or comment or this will be closed in 14 days.

github-actions[bot] avatar Mar 17 '23 10:03 github-actions[bot]

This issue is stale because it has been open 280 days with no activity. Remove stale label or comment or this will be closed in 14 days.

github-actions[bot] avatar Dec 22 '23 20:12 github-actions[bot]

I think the fix for this should have been just moving it from ArgOptions to BaseOptions? At any rate, I finally have a PR to at least deprecate this parameter entirely: https://github.com/SeleniumHQ/selenium/pull/13286

titusfortner avatar Dec 22 '23 22:12 titusfortner

Thanks @titusfortner for taking care of it. This also means we'd need to adjust Appium Python client after the next Selenium release. FYI @KazuCocoa

mykola-mokhnach avatar Dec 22 '23 23:12 mykola-mokhnach

@mykola-mokhnach would https://github.com/SeleniumHQ/selenium/pull/13926 be enough while we merge @titusfortner's PR?

diemol avatar May 10 '24 14:05 diemol