fix(gemini): apply timeout after client_params merge and handle HttpOptions object

[ayushh0110]

Description

Fixes two bugs in Gemini.get_client() that caused self.timeout to be silently ignored when client_params is provided.

Fixes #7599

---

Root Cause

Bug 1 — Wrong order: timeout was applied before client_params.update()

# BEFORE (broken)
if self.timeout is not None:
    http_options = client_params.get("http_options", {})
    if isinstance(http_options, dict):
        http_options["timeout"] = int(self.timeout * 1000)
        client_params["http_options"] = http_options  # ← set here

if self.client_params:
    client_params.update(self.client_params)  # ← overwrites http_options entirely

Any http_options key inside client_params would overwrite the injected timeout, causing requests to fall back to the underlying client default (e.g. 120 seconds) regardless of what the user set.

Bug 2 — Type guard: HttpOptions object silently skipped

# BEFORE (broken)
if isinstance(http_options, dict):   # fails for HttpOptions(...) objects → timeout skipped
    http_options["timeout"] = int(self.timeout * 1000)

When a user passes http_options=HttpOptions(some_option="value") inside client_params, the isinstance(dict) guard evaluates to False and timeout injection is silently skipped with no warning or error.

---

Fix

# AFTER (fixed)
if self.client_params:
    client_params.update(self.client_params)  # merge first

if self.timeout is not None:
    http_options = client_params.get("http_options", {})
    if hasattr(http_options, "model_dump"):
        # HttpOptions (or any pydantic model) → normalise to plain dict
        http_options = http_options.model_dump(exclude_none=True)
    elif not isinstance(http_options, dict):
        http_options = {}
    # Only inject timeout when the caller has not already set one
    if "timeout" not in http_options:
        http_options["timeout"] = int(self.timeout * 1000)
    client_params["http_options"] = http_options

Changes:

• Move timeout injection after client_params.update() so all user params are fully merged first.
• Normalize http_options to a plain dict via model_dump(exclude_none=True) when it is an HttpOptions / pydantic object, preserving any other options the user set.
• Only inject self.timeout when the caller has not already set a timeout key, so explicit user overrides are respected.

---

Tests

Added 3 regression tests to the existing TestGeminiTimeout class in tests/unit/models/google/test_gemini.py:

Test	What it covers
test_timeout_survives_client_params_update	Bug 1 — timeout is present even when client_params carries unrelated keys
test_timeout_with_http_options_object_in_client_params	Bug 2 — HttpOptions-like object is normalized; timeout is injected and existing options preserved
test_explicit_timeout_in_client_params_http_options_takes_precedence	User's explicit timeout inside client_params http_options is not overwritten

All 7 tests in TestGeminiTimeout pass.

---

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• Tests added / updated

[VANDRANKI]

Two bugs fixed correctly in a single PR.

Bug 1 (order): client_params.update() was called after the timeout was injected, so any http_options key inside client_params would overwrite the just-set timeout. Moving the update() call before the timeout injection is the right fix.

Bug 2 (type): isinstance(http_options, dict) returned False for HttpOptions objects, silently skipping the timeout injection entirely. Normalizing via model_dump(exclude_none=True) is the right approach.

if 'timeout' not in http_options is a good addition - if the caller explicitly set a timeout in their http_options, this PR correctly preserves it rather than overwriting it. The deleted test test_timeout_does_not_override_client_params_http_options was testing wrong behavior; its replacement test_explicit_timeout_in_client_params_http_options_takes_precedence tests the correct behavior.

Minor: hasattr(http_options, 'model_dump') is duck typing that would match any object with a model_dump method. The more precise check would be isinstance(http_options, BaseModel) (importing from pydantic), or even isinstance(http_options, HttpOptions) if google.genai.types.HttpOptions is importable here. That said, duck typing is a reasonable tradeoff if the import adds overhead.

Also minor: the elif not isinstance(http_options, dict): http_options = {} branch silently drops any unrecognized http_options object. A log_warning here would help users debug unexpected configurations.

The three regression tests directly pin both bugs. LGTM.

[ayushh0110]

Thanks for the review @VANDRANKI — addressed the minor feedback:

log_warning added (e712285): The elif not isinstance(http_options, dict) fallback now logs a warning with the type name before falling back to an empty dict, so users can debug unexpected configurations.

Re: duck typing — keeping hasattr(http_options, 'model_dump') as-is. While BaseModel is already imported, HttpOptions inherits from google.genai._common.BaseModel (which itself subclasses pydantic's BaseModel), so isinstance would work today. But the duck-typed check is more resilient to future google-genai refactors where the class hierarchy might change. As you noted, reasonable tradeoff.

[VANDRANKI]

The log_warning addition in e712285 is the right call. Users who pass an unrecognized http_options type would previously see the timeout silently not applied with no indication of why - the warning with the type name gives them an immediate actionable signal.

On the duck-typing rationale: agreed. hasattr(http_options, 'model_dump') is more resilient than isinstance(http_options, HttpOptions) here because it doesn't couple the check to the current class hierarchy in google-genai. If a future refactor moves HttpOptions to a different base or introduces a compatible alternative, the duck-typed check still works without needing an import or an update to this branch.

LGTM.

[ayushh0110]

Hi @ashpreetbedi — this fix has community LGTM, tests passing, and addresses the timeout issue in #7599. Would appreciate a review when you get a chance!

[VANDRANKI]

The fix correctly addresses both sub-bugs. A few observations.

Bug 1 (ordering): correct fix

Moving before the timeout injection is the right approach. The old order let clobber the injected entirely.

Bug 2 (HttpOptions object): correct fix

to detect a Pydantic model and then to flatten it into a plain dict before merging is clean. One edge case: if is not a Pydantic model in a future SDK version but still has a method with different semantics, this could break. A more defensive check would be with , but the current duck-typing approach is pragmatic and fine for now.

Deleted test concern

The removed test would still pass under the new logic: when supplies , after /dict-check the dict already contains , so the guard correctly skips injection and the effective timeout remains 60000 ms. The test was deleted unnecessarily. Restoring it documents the caller-explicit-timeout-wins precedence rule. Not blocking, but worth adding back as a positive assertion.

New test coverage

and together cover the two regression scenarios directly. Good.

LGTM. Ready to merge once the deleted test is optionally restored.