fix(client): handle is_closed as method in OpenAI SDK
The openai SDK's SyncAPIClient.is_closed is a method, not a property. getattr(client, 'is_closed', False) returned the bound method object, which is always truthy — causing _is_openai_client_closed() to report all clients as closed and triggering unnecessary client recreation (~100-200ms TCP+TLS overhead per API call). Fix: check if is_closed is callable and call it, otherwise treat as bool. Fixes #4377 Co-authored-by: Bartok9 <Bartok9@users.noreply.github.com>
This commit is contained in:
25
run_agent.py
25
run_agent.py
@ -3486,14 +3486,33 @@ class AIAgent:
|
||||
|
||||
@staticmethod
|
||||
def _is_openai_client_closed(client: Any) -> bool:
|
||||
"""Check if an OpenAI client is closed.
|
||||
|
||||
Handles both property and method forms of is_closed:
|
||||
- httpx.Client.is_closed is a bool property
|
||||
- openai.OpenAI.is_closed is a method returning bool
|
||||
|
||||
Prior bug: getattr(client, "is_closed", False) returned the bound method,
|
||||
which is always truthy, causing unnecessary client recreation on every call.
|
||||
"""
|
||||
from unittest.mock import Mock
|
||||
|
||||
if isinstance(client, Mock):
|
||||
return False
|
||||
if bool(getattr(client, "is_closed", False)):
|
||||
return True
|
||||
|
||||
is_closed_attr = getattr(client, "is_closed", None)
|
||||
if is_closed_attr is not None:
|
||||
# Handle method (openai SDK) vs property (httpx)
|
||||
if callable(is_closed_attr):
|
||||
if is_closed_attr():
|
||||
return True
|
||||
elif bool(is_closed_attr):
|
||||
return True
|
||||
|
||||
http_client = getattr(client, "_client", None)
|
||||
return bool(getattr(http_client, "is_closed", False))
|
||||
if http_client is not None:
|
||||
return bool(getattr(http_client, "is_closed", False))
|
||||
return False
|
||||
|
||||
def _create_openai_client(self, client_kwargs: dict, *, reason: str, shared: bool) -> Any:
|
||||
if self.provider == "copilot-acp" or str(client_kwargs.get("base_url", "")).startswith("acp://copilot"):
|
||||
|
||||
@ -2741,6 +2741,46 @@ def test_is_openai_client_closed_honors_custom_client_flag():
|
||||
assert AIAgent._is_openai_client_closed(SimpleNamespace(is_closed=False)) is False
|
||||
|
||||
|
||||
def test_is_openai_client_closed_handles_method_form():
|
||||
"""Fix for issue #4377: is_closed as method (openai SDK) vs property (httpx).
|
||||
|
||||
The openai SDK's is_closed is a method, not a property. Prior to this fix,
|
||||
getattr(client, "is_closed", False) returned the bound method object, which
|
||||
is always truthy, causing the function to incorrectly report all clients as
|
||||
closed and triggering unnecessary client recreation on every API call.
|
||||
"""
|
||||
|
||||
class MethodFormClient:
|
||||
"""Mimics openai.OpenAI where is_closed() is a method."""
|
||||
|
||||
def __init__(self, closed: bool):
|
||||
self._closed = closed
|
||||
|
||||
def is_closed(self) -> bool:
|
||||
return self._closed
|
||||
|
||||
# Method returning False - client is open
|
||||
open_client = MethodFormClient(closed=False)
|
||||
assert AIAgent._is_openai_client_closed(open_client) is False
|
||||
|
||||
# Method returning True - client is closed
|
||||
closed_client = MethodFormClient(closed=True)
|
||||
assert AIAgent._is_openai_client_closed(closed_client) is True
|
||||
|
||||
|
||||
def test_is_openai_client_closed_falls_back_to_http_client():
|
||||
"""Verify fallback to _client.is_closed when top-level is_closed is None."""
|
||||
|
||||
class ClientWithHttpClient:
|
||||
is_closed = None # No top-level is_closed
|
||||
|
||||
def __init__(self, http_closed: bool):
|
||||
self._client = SimpleNamespace(is_closed=http_closed)
|
||||
|
||||
assert AIAgent._is_openai_client_closed(ClientWithHttpClient(http_closed=False)) is False
|
||||
assert AIAgent._is_openai_client_closed(ClientWithHttpClient(http_closed=True)) is True
|
||||
|
||||
|
||||
class TestAnthropicBaseUrlPassthrough:
|
||||
"""Bug fix: base_url was filtered with 'anthropic in base_url', blocking proxies."""
|
||||
|
||||
|
||||
Reference in New Issue
Block a user