From fbe31b9885efdb8aeb407e8fa25d386bf4236d62 Mon Sep 17 00:00:00 2001 From: Patrick Buckley Date: Mon, 6 Jul 2026 20:56:04 -0700 Subject: [PATCH] fix(models): surface client-construction failures as factory misconfig SDK client construction can fail on environment problems the config never sees (e.g. httpx resolving a certifi CA path deleted by a venv rebuild). Those escaped as bare exceptions and turned every workstream open/create into an opaque 500; re-type them as ValueError in ModelRegistry.get_client so routes answer 503 with the message and the alias. (cherry picked from commit 9c2e809b2605356f8e3de04073eebdf88714ed65) --- tests/test_model_registry.py | 32 ++++++++++++++++++++++++++++++++ turnstone/core/model_registry.py | 22 +++++++++++++++++++--- 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/tests/test_model_registry.py b/tests/test_model_registry.py index 785ecbb8..255a12c1 100644 --- a/tests/test_model_registry.py +++ b/tests/test_model_registry.py @@ -167,6 +167,38 @@ class TestModelRegistry: with pytest.raises(ValueError, match="Unknown model alias"): reg.get_client("nonexistent") + def test_client_construction_failure_is_value_error(self) -> None: + # Environment failures inside SDK construction (e.g. httpx raising + # FileNotFoundError for a CA bundle deleted by a venv rebuild) must + # surface as ValueError so routes answer 503-with-message instead + # of an opaque 500. + reg = self._make_registry() + with ( + patch( + "turnstone.core.model_registry.create_client", + side_effect=FileNotFoundError(2, "No such file or directory"), + ), + pytest.raises(ValueError, match="'default'.*FileNotFoundError") as excinfo, + ): + reg.get_client("default") + assert isinstance(excinfo.value.__cause__, FileNotFoundError) + # Nothing half-constructed may be cached — a later call with a + # repaired environment must construct for real. + assert "default" not in reg._clients + + def test_client_construction_value_error_passes_through(self) -> None: + # create_client's own misconfig ValueErrors already carry + # remediation text and must not be double-wrapped. + reg = self._make_registry() + with ( + patch( + "turnstone.core.model_registry.create_client", + side_effect=ValueError("anthropic-compatible requires base_url"), + ), + pytest.raises(ValueError, match="^anthropic-compatible requires base_url$"), + ): + reg.get_client("default") + def test_shutdown(self) -> None: reg = self._make_registry() reg.get_client("default") diff --git a/turnstone/core/model_registry.py b/turnstone/core/model_registry.py index d7ab7f41..3e1fcaf6 100644 --- a/turnstone/core/model_registry.py +++ b/turnstone/core/model_registry.py @@ -145,9 +145,25 @@ class ModelRegistry: raise ValueError(f"Unknown model alias: {alias}") if alias not in self._clients: cfg = self._models[alias] - self._clients[alias] = create_client( - cfg.provider, base_url=cfg.base_url, api_key=cfg.api_key - ) + try: + self._clients[alias] = create_client( + cfg.provider, base_url=cfg.base_url, api_key=cfg.api_key + ) + except ValueError: + # create_client's own misconfig errors already carry + # remediation text — pass through untouched. + raise + except Exception as exc: + # SDK construction can fail on environment problems the + # config never sees — e.g. httpx resolving a CA-bundle + # path that a venv rebuild deleted (FileNotFoundError). + # Routes map ValueError to a 503 with the message; + # anything else surfaces as an opaque 500, so re-type + # here where the alias is known. + raise ValueError( + f"failed to construct {cfg.provider} client for model " + f"alias {alias!r}: {type(exc).__name__}: {exc}" + ) from exc return self._clients[alias] def get_provider(self, alias: str) -> LLMProvider: