[py]Fix: Timeout env variable GLOBAL_DEFAULT_TIMEOUT has no effect

[XueSongTap]

User description

🔗 Related Issues

Fixes #15604

💥 What does this PR do?

This PR fixes an issue where the GLOBAL_DEFAULT_TIMEOUT environment variable was being ignored when setting timeout values in Selenium's Python bindings. The current implementation always uses a hardcoded timeout value (120 seconds) in the remote connection classes, making the environment variable ineffective.

🔧 Implementation Notes

The implementation has been simplified to properly prioritize timeout sources:

1. Use GLOBAL_DEFAULT_TIMEOUT if it's set in the environment
2. Otherwise, use the explicitly provided timeout parameter
3. Fall back to socket.getdefaulttimeout() if neither is available

This same logic was also applied to the reset_timeout() method to ensure consistent behavior throughout the client lifecycle.

💡 Additional Considerations

This fix restores expected behavior without changing the API. Users who set the GLOBAL_DEFAULT_TIMEOUT environment variable will now see it properly respected.

🔄 Types of changes

• Bug fix (backwards compatible)

---

PR Type

Bug fix

---

Description

• Fixes the GLOBAL_DEFAULT_TIMEOUT environment variable being ignored.

• Adjusts timeout logic to prioritize environment variable correctly.

• Ensures consistent behavior in reset_timeout method.

---

Changes walkthrough 📝

	Relevant files
Bug fix	client_config.py Fix timeout logic to respect environment variable                py/selenium/webdriver/remote/client_config.py Updated timeout logic to prioritize GLOBAL_DEFAULT_TIMEOUT. Modified reset_timeout to respect GLOBAL_DEFAULT_TIMEOUT. Simplified conditional checks for timeout assignment. +8/-8

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[CLAassistant]

All committers have signed the CLA.

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Inconsistent Property The reset_timeout method sets self._timeout but the init method sets self.timeout. This inconsistency could lead to unexpected behavior as they appear to be modifying different attributes. self._timeout = ( float(os.getenv("GLOBAL_DEFAULT_TIMEOUT")) if os.getenv("GLOBAL_DEFAULT_TIMEOUT") is not None else socket.getdefaulttimeout() )

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Avoid redundant environment lookups The code calls os.getenv("GLOBAL_DEFAULT_TIMEOUT") twice, which is inefficient and could lead to inconsistent results if the environment variable changes between calls. Store the result in a variable and reuse it. py/selenium/webdriver/remote/client_config.py [107-111] +global_timeout = os.getenv("GLOBAL_DEFAULT_TIMEOUT") self.timeout = ( - float(os.getenv("GLOBAL_DEFAULT_TIMEOUT")) - if os.getenv("GLOBAL_DEFAULT_TIMEOUT") is not None + float(global_timeout) + if global_timeout is not None else (timeout if timeout is not None else socket.getdefaulttimeout()) ) Apply this suggestion Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a performance and reliability issue where the same environment variable is queried twice. Storing the result in a variable improves efficiency and ensures consistency if the environment variable were to change between calls.	Medium
Update

[cgoldberg]

There are 2 scenarios I can think of that we should also consider (and have tests for):

• if the environment variable is a number less than zero (i.e. GLOBAL_DEFAULT_TIMEOUT=-10.0), it will use this value, which will lead to an error.

• if the environment variable can't be converted to a float (i.e. GLOBAL_DEFAULT_TIMEOUT=bad), it doesn't raise a helpful error.

I think in both of these circumstances, we should just raise a ValueError with a useful message. Something like: ValueError: GLOBAL_DEFAULT_TIMEOUT must be a number >= 0

Also.. in the tests, you might want to mock os.environ rather than modifying it directly. It probably doesn't matter, because you are always setting it back to the original value in a finally block, but this is a nice pattern we use in other tests. Here is an example:

selenium/py/test/selenium/webdriver/chrome/chrome_service_tests.py

Line 130 in f457f15

with patch.dict("os.environ", {"SE_CHROMEDRIVER": "/foo/bar"}):

[cgoldberg]

@XueSongTap Thanks for the PR!... this looks really good.

Please see my review comments, and also don't forget to sign the CLA as mentioned here:

#15628 (comment)

Also, take a look at the code suggestions that the AI bot left in earlier comments.

[XueSongTap]

Hi @barancev, thank you for your helpful review!

You're absolutely right that reading the environment variable at module/class construction time makes it static and insensitive to any later changes to GLOBAL_DEFAULT_TIMEOUT.

To address this, I'm planning to refactor the timeout logic in the following way:

• Instead of evaluating the timeout once in __init__, I will expose it via a @property method, something like:

@property
def timeout(self):
    env_value = os.getenv("GLOBAL_DEFAULT_TIMEOUT")
    if env_value is not None:
        return float(env_value)
    if self._explicit_timeout is not None:
        return self._explicit_timeout
    return socket.getdefaulttimeout()

• The constructor will keep _explicit_timeout to capture any user-specified value.
• Calling reset_timeout() will simply clear _explicit_timeout, allowing fallback to either the env variable or socket default.

This way, changes to GLOBAL_DEFAULT_TIMEOUT at runtime will be respected dynamically, which should make the behavior more predictable and flexible.

Would this approach align with your expectations? Let me know what you think before I proceed with the changes.

Thanks again!

[titusfortner]

Thanks for the PR, but let's wait on deciding that this is really what we want to do in ##15604

[cgoldberg]

@XueSongTap We have decided to remove the use of the GLOBAL_DEFAULT_TIMEOUT environment variable (see discussion in #15604). Can you do that in your branch? We want to completely remove any reference to it, and keep the other logic as you have it.