From 7552061ae9a594c8990b21121b0aeaf38466dd85 Mon Sep 17 00:00:00 2001 From: Gadi Evron Date: Sun, 13 Sep 2026 12:16:20 +0300 Subject: [PATCH 1/2] =?UTF-8?q?fix(llm):=20the=20google=20adapter's=20real?= =?UTF-8?q?-transport=20guard=20=E2=80=94=20and=20the=20catch-gap=20it=20f?= =?UTF-8?q?ound=20live=20(#576=20follow-up=204b)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #576 review flagged the blind spot: the suite's factories INJECT httpx errors, so nothing could detect a future google-genai default-transport flip (the anthropic/openai 3.x SDKs already moved to httpx2; the genai sync client subclasses httpx.Client today and references httpx2 in the same module — the drift direction is live). The guard (3 tests): the adapter driven through the SDK's ACTUAL default transport — a real genai.Client against an unroutable endpoint (hermetic: a dummy key, no injected client, max_retries=0) on both the complete() and validate() ladders, plus the accept-then-close shape (a listener that resets mid-handshake — the #576 review's second case). The tests are structured to go RED and name the missing class the day a transport flip or an unwrapped exception shape arrives. THE GUARD FOUND A REAL GAP ON ITS FIRST RUN: a mid-connection reset surfaces as httpx.ReadError (or a raw httpcore error on the stream-read path) — a class the adapter's six-name catch list DID NOT include (ConnectError/ConnectTimeout/ReadTimeout/WriteTimeout/PoolTimeout/TimeoutException — ReadError and RemoteProtocolError were never listed). A real network blip mid-scan crashed untyped instead of mapping to LLMConnectionError. The fix: the clause becomes httpx.TransportError (the single base of every transport error in the httpx family — strictly wider than the old six, never narrower) + the httpcore bases for the raw path, on BOTH ladders. The identity split is documented in-line: httpx re-exports its OWN hierarchy (httpx.ReadError is NOT httpcore.ReadError), so both families can arrive; the guard tests keep the mapping honest across a backend flip. Evidence: pytest tests/test_llm_google_adapter.py — 11 passed (the conn-refused guard, the validate-path guard, the half-open guard — RED before the fix, GREEN after); the full Python suite — 4074 passed, 0 FAIL. --- .../tests/test_llm_google_adapter.py | 95 +++++++++++++++++++ .../utilities/llm/providers/google.py | 12 ++- 2 files changed, 105 insertions(+), 2 deletions(-) diff --git a/libs/openant-core/tests/test_llm_google_adapter.py b/libs/openant-core/tests/test_llm_google_adapter.py index 3864bc40..90231249 100644 --- a/libs/openant-core/tests/test_llm_google_adapter.py +++ b/libs/openant-core/tests/test_llm_google_adapter.py @@ -143,3 +143,98 @@ def test_tool_use_only_candidate_is_valid_not_empty(): prompt_token_count=1, candidates_token_count=1, total_token_count=2)) result = _response_to_unified(resp) assert result.content and result.stop_reason == "tool_use" + + +# --------------------------------------------------------------------------- +# The real-transport guard (#576 review follow-up 4b): the suite's factories +# INJECT httpx errors, so nothing detects a future google-genai default- +# transport flip (the anthropic/openai 3.x SDKs already moved to httpx2; the +# genai sync client subclasses httpx.Client TODAY and references httpx2 in +# the same module — the drift direction is live). These tests drive the +# adapter through the SDK's ACTUAL default transport: a REAL genai.Client +# against an unroutable endpoint, no injected client, no factory stubs. A +# transport flip breaks the exception CLASS IDENTITY — the raised error no +# longer matches the adapter's `except httpx.*` clauses — and these tests +# go RED the day the default transport's error classes diverge. +# --------------------------------------------------------------------------- + +def _real_transport_adapter(): + """A GoogleAdapter around a REAL SDK client: hermetic (a dummy key, an + unroutable endpoint via the SDK's own HttpOptions, zero network beyond + the refused connection), max_retries=0 so the failure is immediate.""" + from utilities.llm.providers.google import GoogleAdapter + return GoogleAdapter(api_key="test-key", + base_url="http://127.0.0.1:9", max_retries=0) + + +def test_real_transport_connection_refused_maps_typed(): + """The conn-refused class through the SDK's DEFAULT transport (the #576 + probe shape, made permanent): a real genai.Client raises whatever its + real transport raises; the adapter must map it to LLMConnectionError. + RED on a transport flip (the error no longer matches the httpx clauses).""" + from utilities.llm import Message, TextBlock + from utilities.llm.adapter import LLMConnectionError + adapter = _real_transport_adapter() + with pytest.raises(LLMConnectionError): + adapter.complete(model="gemini-2.5-pro", system=None, + messages=[Message(role="user", + content=[TextBlock("hi")])], + max_tokens=8) + + +def test_real_transport_validate_maps_typed(): + """The same guard on the validate() path (its own except ladder).""" + from utilities.llm.adapter import LLMConnectionError + adapter = _real_transport_adapter() + with pytest.raises(LLMConnectionError): + adapter.validate(model="gemini-2.5-pro") + + +def test_real_transport_half_open_connection_maps_typed(): + """The accept-then-close case (the #576 review's second shape): a + listener that accepts and immediately closes. If the SDK's default + transport raises a class the adapter's catch list MISSES (e.g. + httpx.RemoteProtocolError / httpx.ReadError), the error escapes the + LLMConnectionError mapping — this test goes RED, naming the missing + clause. Today's conn-refused-only list is asserted live, not assumed.""" + import socket + import threading + from utilities.llm import Message, TextBlock + from utilities.llm.adapter import LLMError + + listener = socket.socket() + listener.bind(("127.0.0.1", 0)) + listener.listen(4) + port = listener.getsockname()[1] + + def accept_and_close(): + for _ in range(8): # the SDK retries 0; cover races anyway + try: + conn, _ = listener.accept() + conn.close() + except OSError: + return + + t = threading.Thread(target=accept_and_close, daemon=True) + t.start() + try: + from utilities.llm.providers.google import GoogleAdapter + adapter = GoogleAdapter(api_key="test-key", + base_url=f"http://127.0.0.1:{port}", + max_retries=0) + try: + adapter.complete(model="gemini-2.5-pro", system=None, + messages=[Message(role="user", + content=[TextBlock("hi")])], + max_tokens=8) + except LLMError: + pass # typed mapping held — the guard's pass condition + # Anything NOT an LLMError escapes untyped: pytest.raises nothing, + # so the raw exception propagates and THIS test fails with the + # un-typed error's own traceback — naming exactly which transport + # class the adapter's catch list is missing. + finally: + listener.close() + t.join(timeout=2) + finally: + listener.close() diff --git a/libs/openant-core/utilities/llm/providers/google.py b/libs/openant-core/utilities/llm/providers/google.py index 657d8eb4..4718c28e 100644 --- a/libs/openant-core/utilities/llm/providers/google.py +++ b/libs/openant-core/utilities/llm/providers/google.py @@ -58,6 +58,7 @@ import threading from typing import Any, Optional +import httpcore import httpx from google import genai from google.genai import errors as genai_errors @@ -303,7 +304,14 @@ def complete( raise LLMResponseError(redact_secrets(str(exc))) from redacted_cause_from(exc) except genai_errors.APIError as exc: raise LLMResponseError(redact_secrets(str(exc))) from redacted_cause_from(exc) - except (httpx.ConnectError, httpx.ConnectTimeout, httpx.ReadTimeout, httpx.WriteTimeout, httpx.PoolTimeout, httpx.TimeoutException) as exc: + # 4b (the real-transport guard's catch): the SDK does NOT wrap + # mid-connection transport failures (a reset mid-read surfaces as a + # raw httpcore.exceptions.TransportError subclass — ReadError/RemoteProtocolError + # — escaping its APIError family); proven live by the accept-then- + # close guard test. httpx re-exports its own subclasses of these, but + # the identity can differ across transport backends, so catch the + # httpcore base class — every httpx transport error inherits it. + except (httpx.TransportError, httpcore.NetworkError, httpcore.ProtocolError) as exc: raise LLMConnectionError(redact_secrets(str(exc))) from redacted_cause_from(exc) return _response_to_unified(response) @@ -332,7 +340,7 @@ def validate(self, model: str) -> None: raise LLMResponseError(redact_secrets(str(exc))) from redacted_cause_from(exc) except genai_errors.APIError as exc: raise LLMResponseError(redact_secrets(str(exc))) from redacted_cause_from(exc) - except (httpx.ConnectError, httpx.ConnectTimeout, httpx.ReadTimeout, httpx.WriteTimeout, httpx.PoolTimeout, httpx.TimeoutException) as exc: + except (httpx.TransportError, httpcore.NetworkError, httpcore.ProtocolError) as exc: raise LLMConnectionError(redact_secrets(str(exc))) from redacted_cause_from(exc) From 04d1d26e685bfbb892c2fdc7653f220cb50e7846 Mon Sep 17 00:00:00 2001 From: Gadi Evron Date: Sun, 13 Sep 2026 12:54:14 +0300 Subject: [PATCH 2/2] =?UTF-8?q?fix(llm):=20the=20#596=20review=20folds=20?= =?UTF-8?q?=E2=80=94=20the=20comment=20corrected=20(no=20nonexistent=20cla?= =?UTF-8?q?ss;=20the=20real=20chaining=20hierarchy),=20httpcore.TimeoutExc?= =?UTF-8?q?eption=20added,=20the=20dependency=20declared,=20the=20docstrin?= =?UTF-8?q?g=20updated?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reviewers' corrections: (1) the in-line rationale named a nonexistent httpcore.exceptions.TransportError and claimed every httpx transport error inherits the httpcore base — false (httpx chains via raise-from into its OWN hierarchy; neither family inherits the other; httpcore's roots are NetworkError/ProtocolError/TimeoutException). The comment now describes the real mechanism: httpx.TransportError is the load-bearing base (strictly wider than the old six); the httpcore bases are belt-and-braces for an un-wrapped raw exception. (2) httpcore.TimeoutException added to both clauses (the third root, absent — a raw httpcore timeout would have escaped untyped on the claimed raw path). (3) httpcore declared in pyproject.toml (it was transitive-only; the adapter now imports it directly — a module-level ImportError on the google provider would be far worse than the network blip this fixes). (4) the module docstring's stale two-name failure surface updated to the TransportError family. --- libs/openant-core/pyproject.toml | 1 + .../utilities/llm/providers/google.py | 28 +++++++++++++------ 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/libs/openant-core/pyproject.toml b/libs/openant-core/pyproject.toml index 27567f21..75c63a81 100644 --- a/libs/openant-core/pyproject.toml +++ b/libs/openant-core/pyproject.toml @@ -20,6 +20,7 @@ dependencies = [ "python-dotenv>=1.0.0", "pydantic>=2.0.0", "httpx>=0.24.0", + "httpcore>=1.0.0", # 4b: the google adapter catches its bases directly (belt-and-braces under httpx) "PyYAML>=6.0", "requests>=2.31.0", "tree-sitter>=0.21.0", diff --git a/libs/openant-core/utilities/llm/providers/google.py b/libs/openant-core/utilities/llm/providers/google.py index 4718c28e..eced735b 100644 --- a/libs/openant-core/utilities/llm/providers/google.py +++ b/libs/openant-core/utilities/llm/providers/google.py @@ -304,14 +304,23 @@ def complete( raise LLMResponseError(redact_secrets(str(exc))) from redacted_cause_from(exc) except genai_errors.APIError as exc: raise LLMResponseError(redact_secrets(str(exc))) from redacted_cause_from(exc) - # 4b (the real-transport guard's catch): the SDK does NOT wrap - # mid-connection transport failures (a reset mid-read surfaces as a - # raw httpcore.exceptions.TransportError subclass — ReadError/RemoteProtocolError - # — escaping its APIError family); proven live by the accept-then- - # close guard test. httpx re-exports its own subclasses of these, but - # the identity can differ across transport backends, so catch the - # httpcore base class — every httpx transport error inherits it. - except (httpx.TransportError, httpcore.NetworkError, httpcore.ProtocolError) as exc: + # 4b (the real-transport guard's catch): the previous clause named + # six httpx classes individually and MISSED ReadError / + # RemoteProtocolError (a mid-connection reset escaped untyped — + # proven live by the accept-then-close guard test). + # httpx.TransportError is the single base of every transport error + # in the httpx family (connect, read, write, pool, remote-protocol, + # the timeout family) — strictly wider than the old list, never + # narrower. The httpcore bases are BELT-AND-BRACES for a raw + # httpcore exception arriving un-wrapped (httpx maps httpcore into + # its OWN hierarchy — httpx.ReadError is NOT httpcore.ReadError; + # the families chain via `raise ... from`, neither inherits the + # other). httpcore's roots are NetworkError / ProtocolError / + # TimeoutException (there is no httpcore.TransportError); all three + # are caught. A transport-backend flip (the httpx2 direction the + # anthropic/openai SDKs already took) changes which family arrives; + # this clause and the guard tests keep the mapping honest. + except (httpx.TransportError, httpcore.NetworkError, httpcore.ProtocolError, httpcore.TimeoutException) as exc: raise LLMConnectionError(redact_secrets(str(exc))) from redacted_cause_from(exc) return _response_to_unified(response) @@ -340,7 +349,8 @@ def validate(self, model: str) -> None: raise LLMResponseError(redact_secrets(str(exc))) from redacted_cause_from(exc) except genai_errors.APIError as exc: raise LLMResponseError(redact_secrets(str(exc))) from redacted_cause_from(exc) - except (httpx.TransportError, httpcore.NetworkError, httpcore.ProtocolError) as exc: + # The same transport clause as complete()'s (see its comment). + except (httpx.TransportError, httpcore.NetworkError, httpcore.ProtocolError, httpcore.TimeoutException) as exc: raise LLMConnectionError(redact_secrets(str(exc))) from redacted_cause_from(exc)