Skip to content

Commit f596944

Browse files
authored
fix: make the provider connection test resilient to model retirement (#1035)
* fix: make the provider connection test resilient to model retirement Two independent hardenings against the #970 class of breakage (a hard-coded Gemini test model getting shut down by Google, which made testing a valid key fail with a 404): - Use Google's floating alias gemini-flash-latest for the Google/Vertex test model instead of a dated id, so a retirement repoints it for us. - Reframe the provider connection test around what an error actually proves: only a rejected key (401), missing permissions (403), or an unreachable endpoint are failures. Anything the provider returns after authenticating - a rate limit, or a missing/retired/unsupported model - still proves the credentials work, so it reports success. Previously this relied on matching the literal phrase 'not found' + 'model', which a differently-worded retirement/deprecation error slipped past. Unifies the auth/network/rate-limit classification (previously duplicated and divergent between connection_tester and credentials_service) into shared helpers. The individual-model test keeps model-not-found as a failure, since there a specific registered model really is broken. Adds classification tests with realistic provider error strings. * docs: add CHANGELOG entry for connection-test resilience fix
1 parent 4e96a27 commit f596944

5 files changed

Lines changed: 258 additions & 31 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3232
- `docker-compose.yml` now sources the SurrealDB credentials from `SURREAL_USER` / `SURREAL_PASSWORD` (applied to both the database server and the app), defaulting to `root:root` so the zero-config quick start is unchanged. Set them in a `.env` file to use your own credentials before exposing the instance; `.env.example` and the compose file note this (#946)
3333

3434
### Fixed
35+
- Testing a valid Google/Vertex credential no longer fails after Google retires a Gemini model. The connection test used a hard-coded model id that Google shuts down on a schedule (`gemini-2.0-flash`), so a valid key surfaced as a broken connection (#970). The Google/Vertex test now uses Google's floating `gemini-flash-latest` alias, and the provider connection test was reframed so only a rejected key, missing permissions, or an unreachable endpoint count as failures — a missing/retired/rate-limited model still reports the credentials as valid
3536
- API startup no longer crashes when SurrealDB isn't ready yet (e.g. docker-compose race on host reboot: `Temporary failure in name resolution`). The lifespan now polls a lightweight readiness probe with bounded exponential backoff (~50s budget, 5s per-probe timeout) before running migrations; migration errors themselves still fail fast (#708)
3637
- Markdown typography styles (`prose` classes) are active again: the Tailwind v4 migration left the old `tailwind.config.ts` (which loaded `@tailwindcss/typography`) silently ignored, so rendered markdown lost its typographic styling. The plugin and class-based dark mode are now configured in `globals.css`, and markdown rendering is centralized in a shared `MarkdownRenderer` component (#783)
3738
- Podcast generation no longer truncates on dense, long-form content (`LengthFinishReasonError` / `OUTPUT_PARSING_FAILURE`): episode profiles now support an optional `max_tokens` that is passed through to podcast_creator's outline/transcript generation, overriding its defaults — settable via the episode profile API (UI follow-up in #991) (#639)

api/credentials_service.py

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,7 @@ async def test_credential(credential_id: str) -> dict:
269269
_test_azure_connection,
270270
_test_ollama_connection,
271271
_test_openai_compatible_connection,
272+
classify_provider_test_error,
272273
)
273274

274275
provider = cred.provider.lower()
@@ -360,18 +361,10 @@ async def test_credential(credential_id: str) -> dict:
360361
}
361362

362363
error_msg = str(e)
363-
if "401" in error_msg or "unauthorized" in error_msg.lower():
364-
return {"provider": provider, "success": False, "message": "Invalid API key"}
365-
elif "403" in error_msg or "forbidden" in error_msg.lower():
366-
return {"provider": provider, "success": False, "message": "API key lacks required permissions"}
367-
elif "rate" in error_msg.lower() and "limit" in error_msg.lower():
368-
return {"provider": provider, "success": True, "message": "Rate limited - but connection works"}
369-
elif "not found" in error_msg.lower() and "model" in error_msg.lower():
370-
return {"provider": provider, "success": True, "message": "API key valid (test model not available)"}
371-
else:
364+
success, message = classify_provider_test_error(error_msg)
365+
if not success:
372366
logger.debug(f"Test connection error for credential {credential_id}: {e}")
373-
truncated = error_msg[:100] + "..." if len(error_msg) > 100 else error_msg
374-
return {"provider": provider, "success": False, "message": f"Error: {truncated}"}
367+
return {"provider": provider, "success": success, "message": message}
375368

376369

377370
async def discover_with_config(provider: str, config: dict) -> List[dict]:

open_notebook/ai/connection_tester.py

Lines changed: 109 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,18 @@ def _is_vertex_credentials_file_error(exc: Exception) -> bool:
4949

5050
# Test models for each provider - uses minimal/cheapest models for testing
5151
# Format: (model_name, model_type)
52+
#
53+
# Prefer a provider-maintained floating alias where one exists, so a model
54+
# retirement doesn't silently break the connection test (see #970: Google
55+
# hard-shuts-down Gemini model ids on a schedule). `gemini-flash-latest`
56+
# is Google's alias for the current stable Flash model and moves forward on
57+
# its own. The provider test also no longer treats a model-level failure as
58+
# a connection failure (see `_connection_failure_reason`), so even if an
59+
# alias ever breaks, the test still reports the credentials correctly.
5260
TEST_MODELS = {
5361
"openai": ("gpt-3.5-turbo", "language"),
5462
"anthropic": ("claude-3-haiku-20240307", "language"),
55-
"google": ("gemini-3.5-flash", "language"),
63+
"google": ("gemini-flash-latest", "language"),
5664
"groq": ("llama-3.1-8b-instant", "language"),
5765
"mistral": ("mistral-small-latest", "language"),
5866
"deepseek": ("deepseek-chat", "language"),
@@ -63,7 +71,7 @@ def _is_vertex_credentials_file_error(exc: Exception) -> bool:
6371
"deepgram": ("aura-2-thalia-en", "text_to_speech"),
6472
"ollama": (None, "language"), # Dynamic - will use first available model
6573
# Complex providers with additional configuration
66-
"vertex": ("gemini-3.5-flash", "language"), # Uses Google Vertex AI
74+
"vertex": ("gemini-flash-latest", "language"), # Uses Google Vertex AI
6775
"azure": ("gpt-35-turbo", "language"), # Azure OpenAI deployment name
6876
"openai_compatible": (None, "language"), # Dynamic - will use first available model
6977
"dashscope": ("qwen-plus", "language"),
@@ -281,26 +289,114 @@ def _get_test_audio() -> io.BytesIO:
281289
return _generate_test_wav()
282290

283291

284-
def _normalize_error_message(error_msg: str) -> Tuple[bool, str]:
285-
"""Normalize common error patterns into user-friendly messages."""
292+
def _connection_failure_reason(error_msg: str) -> Optional[str]:
293+
"""Classify whether an error means the provider is genuinely unreachable
294+
or the credentials are rejected.
295+
296+
Returns a user-facing failure message for the only errors that actually
297+
disprove a working provider connection — bad key (401), insufficient
298+
permissions (403), and network/timeout failures. Returns None for
299+
anything the provider itself returned *after* authenticating (a missing
300+
or retired model, an unsupported request, a rate limit): reaching the
301+
model layer at all proves the credentials and endpoint work, so those
302+
are not connection failures. This is what keeps a retired test model
303+
(see #970) from being misreported as a broken provider connection.
304+
"""
286305
lower = error_msg.lower()
287306

288307
if "401" in error_msg or "unauthorized" in lower:
289-
return False, "Invalid API key"
290-
elif "403" in error_msg or "forbidden" in lower:
291-
return False, "API key lacks required permissions"
292-
elif "rate" in lower and "limit" in lower:
308+
return "Invalid API key"
309+
if "403" in error_msg or "forbidden" in lower:
310+
return "API key lacks required permissions"
311+
if "timeout" in lower or "timed out" in lower:
312+
return "Connection timed out - check network/endpoint"
313+
if (
314+
"connection" in lower
315+
or "network" in lower
316+
or "getaddrinfo" in lower
317+
or "name resolution" in lower
318+
or "failed to establish" in lower
319+
):
320+
return "Connection error - check network/endpoint"
321+
return None
322+
323+
324+
def _is_rate_limit(error_msg: str) -> bool:
325+
"""True if the error is a throttling/quota response. Being rate-limited
326+
proves the request authenticated, so callers treat this as connection-OK.
327+
Covers the common phrasings across providers (429, quota, resource
328+
exhausted) rather than just the literal words "rate limit"."""
329+
lower = error_msg.lower()
330+
return (
331+
("rate" in lower and "limit" in lower)
332+
or "429" in error_msg
333+
or "quota" in lower
334+
or "resource has been exhausted" in lower
335+
or "resource exhausted" in lower
336+
)
337+
338+
339+
def _normalize_error_message(error_msg: str) -> Tuple[bool, str]:
340+
"""Normalize common error patterns into user-friendly messages.
341+
342+
Used by the *individual model* test, where the user is validating one
343+
specific registered model — so a missing model IS a failure (unlike the
344+
provider-level test, which only cares that the credentials work).
345+
"""
346+
reason = _connection_failure_reason(error_msg)
347+
if reason:
348+
return False, reason
349+
350+
if _is_rate_limit(error_msg):
293351
return True, "Rate limited - but connection works"
294-
elif "not found" in lower and "model" in lower:
352+
lower = error_msg.lower()
353+
if "not found" in lower and "model" in lower:
295354
return False, "Model not found on this provider"
296-
elif "connection" in lower or "network" in lower:
297-
return False, "Connection error - check network/endpoint"
298-
elif "timeout" in lower:
299-
return False, "Connection timed out - check network/endpoint"
300355

301356
return False, error_msg
302357

303358

359+
# Substrings that indicate the provider answered but the *test model* is
360+
# missing/retired/unsupported - proof the credentials and endpoint work.
361+
# Only consulted for fixed-endpoint API-key providers (URL-based providers
362+
# are tested via their own handlers), so a "not found" here is about the
363+
# model, never a user-supplied base URL.
364+
_MODEL_UNAVAILABLE_MARKERS = (
365+
"not found",
366+
"not supported",
367+
"does not exist",
368+
"deprecated",
369+
"unavailable",
370+
"no longer available",
371+
)
372+
373+
374+
def classify_provider_test_error(error_msg: str) -> Tuple[bool, str]:
375+
"""Classify a provider connection-test exception into (success, message).
376+
377+
The provider test only asks "do these credentials reach a working
378+
provider?" - so the sole real failures are a rejected key (401),
379+
insufficient permissions (403), and an unreachable endpoint. Anything
380+
the provider returned after authenticating - a rate limit, or a
381+
missing/retired/unsupported test model - still proves the connection
382+
works, so it's reported as success. This is the durable half of the
383+
#970 fix: even if the hard-coded test model is retired, a valid key is
384+
never misreported as a broken connection.
385+
"""
386+
reason = _connection_failure_reason(error_msg)
387+
if reason:
388+
return False, reason
389+
390+
if _is_rate_limit(error_msg):
391+
return True, "Rate limited - but connection works"
392+
lower = error_msg.lower()
393+
if any(marker in lower for marker in _MODEL_UNAVAILABLE_MARKERS):
394+
return True, "API key valid (test model unavailable)"
395+
396+
truncated = error_msg[:100] + "..." if len(error_msg) > 100 else error_msg
397+
return False, f"Error: {truncated}"
398+
399+
304400
async def test_individual_model(model) -> Tuple[bool, str]:
305401
"""
306402
Test a specific model configuration end-to-end by making a real API call.
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
"""
2+
Tests for connection-test error classification (open_notebook/ai/connection_tester.py).
3+
4+
Two semantics share one auth/network classifier:
5+
- The *provider* test asks only "do these credentials reach a working
6+
provider?" — so a missing/retired/unsupported test model is SUCCESS (the
7+
#970 durability fix): a hard-coded test model going away must never be
8+
misreported as a broken connection.
9+
- The *individual model* test validates one specific registered model, so a
10+
missing model there IS a failure.
11+
"""
12+
13+
import pytest
14+
15+
from open_notebook.ai.connection_tester import (
16+
_connection_failure_reason,
17+
_is_rate_limit,
18+
_normalize_error_message,
19+
classify_provider_test_error,
20+
)
21+
22+
# Realistic provider error strings.
23+
GOOGLE_RETIRED_MODEL_404 = (
24+
"404 models/gemini-2.0-flash is not found for API version v1beta, "
25+
"or is not supported for generateContent."
26+
)
27+
GOOGLE_DEPRECATED = "400 Model gemini-1.5-pro has been deprecated."
28+
GOOGLE_BAD_KEY_401 = "401 API key not valid. Please pass a valid API key."
29+
GOOGLE_PERM_403 = "403 Permission denied on resource project."
30+
GOOGLE_QUOTA_429 = "429 Resource has been exhausted (e.g. check quota)."
31+
DNS_FAILURE = "Connection error: [Errno -2] Name or service not known (getaddrinfo failed)"
32+
TIMEOUT = "Request timed out after 10s"
33+
34+
35+
class TestConnectionFailureReason:
36+
"""Only auth/permission/network are true connection failures."""
37+
38+
@pytest.mark.parametrize(
39+
"msg,expected_fragment",
40+
[
41+
(GOOGLE_BAD_KEY_401, "Invalid API key"),
42+
(GOOGLE_PERM_403, "lacks required permissions"),
43+
(DNS_FAILURE, "Connection error"),
44+
(TIMEOUT, "timed out"),
45+
],
46+
)
47+
def test_real_failures_return_a_reason(self, msg, expected_fragment):
48+
reason = _connection_failure_reason(msg)
49+
assert reason is not None
50+
assert expected_fragment in reason
51+
52+
@pytest.mark.parametrize(
53+
"msg",
54+
[GOOGLE_RETIRED_MODEL_404, GOOGLE_DEPRECATED, GOOGLE_QUOTA_429],
55+
)
56+
def test_provider_reached_returns_none(self, msg):
57+
# A model/quota problem came back FROM the provider — not a failure.
58+
assert _connection_failure_reason(msg) is None
59+
60+
61+
class TestRateLimitDetection:
62+
@pytest.mark.parametrize(
63+
"msg",
64+
[
65+
"429 Too Many Requests",
66+
"Resource has been exhausted (e.g. check quota).",
67+
"You have hit your rate limit",
68+
"quota exceeded for this project",
69+
],
70+
)
71+
def test_detects_throttling_phrasings(self, msg):
72+
assert _is_rate_limit(msg) is True
73+
74+
def test_plain_model_error_is_not_rate_limit(self):
75+
assert _is_rate_limit(GOOGLE_RETIRED_MODEL_404) is False
76+
77+
78+
class TestClassifyProviderTestError:
79+
"""Provider test: only bad creds / unreachable endpoint fail."""
80+
81+
def test_retired_test_model_is_success(self):
82+
# The core #970 regression: a shut-down test model must report the
83+
# key as valid, not the connection as broken.
84+
success, message = classify_provider_test_error(GOOGLE_RETIRED_MODEL_404)
85+
assert success is True
86+
assert "test model unavailable" in message
87+
88+
def test_deprecated_model_is_success(self):
89+
success, _ = classify_provider_test_error(GOOGLE_DEPRECATED)
90+
assert success is True
91+
92+
def test_rate_limit_is_success(self):
93+
success, message = classify_provider_test_error(GOOGLE_QUOTA_429)
94+
assert success is True
95+
assert "connection works" in message
96+
97+
@pytest.mark.parametrize(
98+
"msg,expected_fragment",
99+
[
100+
(GOOGLE_BAD_KEY_401, "Invalid API key"),
101+
(GOOGLE_PERM_403, "lacks required permissions"),
102+
(DNS_FAILURE, "Connection error"),
103+
(TIMEOUT, "timed out"),
104+
],
105+
)
106+
def test_auth_and_network_still_fail(self, msg, expected_fragment):
107+
success, message = classify_provider_test_error(msg)
108+
assert success is False
109+
assert expected_fragment in message
110+
111+
def test_unrecognized_error_stays_a_failure(self):
112+
# A construction/config error we can't attribute to the provider is
113+
# surfaced, not silently reported as success.
114+
success, message = classify_provider_test_error(
115+
"TypeError: create_language() missing required config 'base_url'"
116+
)
117+
assert success is False
118+
assert "Error:" in message
119+
120+
121+
class TestIndividualModelSemanticsDiffer:
122+
"""The SAME model-not-found string is failure for the individual-model
123+
test but success for the provider test — the intended difference."""
124+
125+
def test_model_not_found_is_failure_for_individual_test(self):
126+
success, message = _normalize_error_message(GOOGLE_RETIRED_MODEL_404)
127+
assert success is False
128+
assert "Model not found" in message
129+
130+
def test_same_string_is_success_for_provider_test(self):
131+
success, _ = classify_provider_test_error(GOOGLE_RETIRED_MODEL_404)
132+
assert success is True
133+
134+
def test_individual_test_shares_auth_classification(self):
135+
success, message = _normalize_error_message(GOOGLE_BAD_KEY_401)
136+
assert success is False
137+
assert "Invalid API key" in message

tests/test_credentials_api.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -269,15 +269,15 @@ def test_classify_matrix(self):
269269
assert classify_model_type("scribe_v1", "elevenlabs") == "speech_to_text"
270270
assert classify_model_type("eleven_multilingual_v2", "elevenlabs") == "text_to_speech"
271271

272-
def test_google_and_vertex_use_current_test_model(self):
273-
# Regression test for #970: the connection test used the retired
274-
# gemini-2.0-flash, so testing a valid Google AI key failed with 404.
275-
# gemini-3.5-flash is the current stable GA (gemini-2.5-flash is
276-
# already scheduled for shutdown), so pin the longer-lived one.
272+
def test_google_and_vertex_use_floating_alias(self):
273+
# Regression test for #970: the connection test used a hard-coded
274+
# Gemini id (gemini-2.0-flash) that Google later shut down, so a
275+
# valid key failed with 404. Use Google's floating alias, which the
276+
# provider repoints on each retirement, so it can't go stale.
277277
from open_notebook.ai.connection_tester import TEST_MODELS
278278

279-
assert TEST_MODELS["google"] == ("gemini-3.5-flash", "language")
280-
assert TEST_MODELS["vertex"] == ("gemini-3.5-flash", "language")
279+
assert TEST_MODELS["google"] == ("gemini-flash-latest", "language")
280+
assert TEST_MODELS["vertex"] == ("gemini-flash-latest", "language")
281281

282282

283283
if __name__ == "__main__":

0 commit comments

Comments
 (0)