mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-12 23:12:23 -06:00
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 9c2e809b26)
This commit is contained in:
@@ -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")
|
||||
|
||||
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user