SeleniumHQ / selenium

A browser automation framework and ecosystem.
https://selenium.dev
Apache License 2.0
30.7k stars 8.19k forks source link

[šŸš€ Feature]: `Service` shutting down via `__del__` is unintuitive (as `Remote` only consumes the service url) #14464

Open GideonBear opened 2 months ago

GideonBear commented 2 months ago

Feature and motivation

I recently faced a weird bug where returning from a function would cause the browser to close and Selenium to complain they couldn't reach the driver. Turned out, my Service was garbage collected (and quitting it via __del__) after I had used its service_url.

from selenium import webdriver
from selenium.webdriver.chrome import service

def get_driver():
    webdriver_service = service.Service("operadriver")
    webdriver_service.start()

    options = webdriver.ChromeOptions()
    options.add_experimental_option('w3c', True)

    driver = webdriver.Remote(webdriver_service.service_url, options=options)

    return driver

driver = get_driver()
driver.get("https://example.com")

While in hindsight it is expected behavior based on the code (I did not pass webdriver_service, I passed a string I got from it), it's not very weird (especially if you don't know the internals of how webdrivers work) to overlook the .service_url and assume that Remote would set webdriver_service as an instance attribute, keeping it from being garbage collected. This behavior is in my opinion very unintuitive. A possible solution would be to only allow passing the whole Service into Remote instead of just the service_url (and setting it as an instance attribute), but this would be backwards-incompatible. Maybe add another method for passing the whole Service (or overload the original method), and encourage using that (for example by raising a warning when the service_url parameter is accessed) instead of the service_url? I don't have a good proposal, but I thought I would submit an issue anyway, since it cost me a lot of time to debug.

After writing this, it came to mind that using Service and Remote directly might not have been intended anyway. The readme of webdriver-manager told me to do it this way, probably because Opera was deprecated. I'll submit it anyway, but if this is the case, it's probably a lot less important.

Usage example

Not applicable

github-actions[bot] commented 2 months ago

@GideonBear, 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!