mirror of
https://gitee.com/mateos/mateclaw.git
synced 2026-09-13 03:13:41 +08:00
fix(failover): probe URL construction + permissive 4xx/5xx handling
Two real bugs the user restart surfaced — both turned healthy providers into HARD-removed false positives. Bug #1 — URL duplication OpenAiCompatibleListModelsProbe always concatenated /v1/models, so providers whose Base URL already includes the version segment got the wrong URL: LMStudio http://localhost:1234/v1 → /v1/v1/models → 404 ZhipuAI .../api/paas/v4 → /v4/v1/models → 404 Fix: detect a trailing /vN suffix and append /models instead. Six unit tests in OpenAiCompatibleListModelsProbeTest lock the rule down. Bug #2 — 404 false positives Kimi for Coding API does not expose /v1/models even though chat works fine, so the probe correctly received a 404 and incorrectly HARD-removed the provider from the pool. Other vendors will hit the same — listing is not a universal contract. Fix: classify HTTP responses semantically. 401 / 403 → HARD remove (real auth failure) 404 / 405 / 410 → fail-open (endpoint missing, server may be alive) other 4xx / 5xx → fail-open (probe inconclusive — let chat decide) network errors → fail (unreachable) This is the same philosophy as ChatGPTOAuthStatusProbe: when we cannot cheaply confirm health, we do not proactively penalize the provider. Same logic applied to Anthropic + DashScope probes for consistency. Net effect on the user deployment after restart: - kimi-code stays in pool (404 → fail-open) → primary path works again - lmstudio + zhipu-cn also stay in pool (URL bug fixed) - dashscope + ollama unchanged (real 200 OK) Tests: 6 new for resolveModelsPath. The 2 unrelated WikiRawMaterialDedupTest failures pre-date this commit and live in ba86bea.
This commit is contained in:
matevip
parent
4700d0312d
commit
527a67374d
@ -6,6 +6,8 @@ import org.springframework.http.MediaType;
|
||||
import org.springframework.http.client.JdkClientHttpRequestFactory;
|
||||
import org.springframework.stereotype.Component;
|
||||
import org.springframework.util.StringUtils;
|
||||
import org.springframework.web.client.HttpClientErrorException;
|
||||
import org.springframework.web.client.HttpServerErrorException;
|
||||
import org.springframework.web.client.RestClient;
|
||||
import vip.mate.llm.failover.ProbeResult;
|
||||
import vip.mate.llm.failover.ProviderProbeStrategy;
|
||||
@ -53,9 +55,22 @@ public class AnthropicListModelsProbe implements ProviderProbeStrategy {
|
||||
|
||||
String body = client.get().uri("/v1/models").retrieve().body(String.class);
|
||||
long latency = System.currentTimeMillis() - start;
|
||||
if (body == null || body.isBlank()) {
|
||||
return ProbeResult.fail(latency, "empty body from /v1/models");
|
||||
// Empty body from a 200 — uncommon but not necessarily fatal.
|
||||
return ProbeResult.ok(latency);
|
||||
} catch (HttpClientErrorException e) {
|
||||
long latency = System.currentTimeMillis() - start;
|
||||
int status = e.getStatusCode().value();
|
||||
// Real auth failure → HARD remove. Anything else (404 endpoint missing,
|
||||
// 429 probe rate-limited, etc.) is fail-open: the chat path is authoritative.
|
||||
if (status == 401 || status == 403) {
|
||||
return ProbeResult.fail(latency, "auth failed (" + status + ")");
|
||||
}
|
||||
log.info("[Probe] anthropic /v1/models returned {} — fail-open (in-pool)", status);
|
||||
return ProbeResult.ok(latency);
|
||||
} catch (HttpServerErrorException e) {
|
||||
long latency = System.currentTimeMillis() - start;
|
||||
log.info("[Probe] anthropic /v1/models returned {} — fail-open (5xx may be transient)",
|
||||
e.getStatusCode().value());
|
||||
return ProbeResult.ok(latency);
|
||||
} catch (Exception e) {
|
||||
long latency = System.currentTimeMillis() - start;
|
||||
|
||||
@ -6,6 +6,8 @@ import org.springframework.http.MediaType;
|
||||
import org.springframework.http.client.JdkClientHttpRequestFactory;
|
||||
import org.springframework.stereotype.Component;
|
||||
import org.springframework.util.StringUtils;
|
||||
import org.springframework.web.client.HttpClientErrorException;
|
||||
import org.springframework.web.client.HttpServerErrorException;
|
||||
import org.springframework.web.client.RestClient;
|
||||
import vip.mate.llm.failover.ProbeResult;
|
||||
import vip.mate.llm.failover.ProviderProbeStrategy;
|
||||
@ -52,9 +54,20 @@ public class DashScopeListModelsProbe implements ProviderProbeStrategy {
|
||||
|
||||
String body = client.get().uri("/v1/models").retrieve().body(String.class);
|
||||
long latency = System.currentTimeMillis() - start;
|
||||
if (body == null || body.isBlank()) {
|
||||
return ProbeResult.fail(latency, "empty body from /v1/models");
|
||||
return ProbeResult.ok(latency);
|
||||
} catch (HttpClientErrorException e) {
|
||||
long latency = System.currentTimeMillis() - start;
|
||||
int status = e.getStatusCode().value();
|
||||
// Real auth failure → HARD remove. Other 4xx are inconclusive: fail-open.
|
||||
if (status == 401 || status == 403) {
|
||||
return ProbeResult.fail(latency, "auth failed (" + status + ")");
|
||||
}
|
||||
log.info("[Probe] dashscope /v1/models returned {} — fail-open (in-pool)", status);
|
||||
return ProbeResult.ok(latency);
|
||||
} catch (HttpServerErrorException e) {
|
||||
long latency = System.currentTimeMillis() - start;
|
||||
log.info("[Probe] dashscope /v1/models returned {} — fail-open (5xx may be transient)",
|
||||
e.getStatusCode().value());
|
||||
return ProbeResult.ok(latency);
|
||||
} catch (Exception e) {
|
||||
long latency = System.currentTimeMillis() - start;
|
||||
|
||||
@ -6,6 +6,8 @@ import org.springframework.http.MediaType;
|
||||
import org.springframework.http.client.JdkClientHttpRequestFactory;
|
||||
import org.springframework.stereotype.Component;
|
||||
import org.springframework.util.StringUtils;
|
||||
import org.springframework.web.client.HttpClientErrorException;
|
||||
import org.springframework.web.client.HttpServerErrorException;
|
||||
import org.springframework.web.client.RestClient;
|
||||
import vip.mate.llm.failover.ProbeResult;
|
||||
import vip.mate.llm.failover.ProviderProbeStrategy;
|
||||
@ -14,14 +16,31 @@ import vip.mate.llm.model.ModelProviderEntity;
|
||||
|
||||
import java.net.http.HttpClient;
|
||||
import java.time.Duration;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
/**
|
||||
* Probes an OpenAI-compatible provider by calling its {@code GET /v1/models}
|
||||
* endpoint — free (zero token cost) and authenticates the API key in one
|
||||
* round-trip. A 200 response with body present is sufficient to confirm
|
||||
* reachability + auth; we don't try to parse the model list because
|
||||
* different providers (Kimi / DeepSeek / Moonshot / OpenRouter) have minor
|
||||
* schema differences that aren't relevant to a liveness check.
|
||||
* Probes an OpenAI-compatible provider by listing its models.
|
||||
*
|
||||
* <p>Two non-trivial things this implementation handles:</p>
|
||||
*
|
||||
* <ol>
|
||||
* <li><b>Path construction.</b> Different vendors set Base URL to different
|
||||
* depths: OpenAI/DeepSeek/Kimi point at the API root
|
||||
* ({@code https://api.openai.com}) while LMStudio / ZhipuAI bake the
|
||||
* version segment in ({@code http://localhost:1234/v1},
|
||||
* {@code https://open.bigmodel.cn/api/paas/v4}). We append {@code /models}
|
||||
* when the URL already ends with a {@code /vN} segment, otherwise
|
||||
* {@code /v1/models}. Without this we'd hit {@code /v1/v1/models} on
|
||||
* LMStudio and {@code /v4/v1/models} on Zhipu — both 404.</li>
|
||||
*
|
||||
* <li><b>Permissive 4xx/5xx handling.</b> Not every OpenAI-compatible
|
||||
* vendor implements {@code /models}. Kimi for Coding returns a
|
||||
* structured 404 here even though chat works fine. So we treat
|
||||
* {@code 404 / 405 / 410} as <i>fail-open</i> (probe inconclusive,
|
||||
* leave the provider in the pool — first chat call will be authoritative)
|
||||
* rather than HARD removing. Only auth (401/403) and connection-level
|
||||
* failures (DNS / refused / timeout) are treated as definitive negatives.</li>
|
||||
* </ol>
|
||||
*/
|
||||
@Slf4j
|
||||
@Component
|
||||
@ -30,6 +49,9 @@ public class OpenAiCompatibleListModelsProbe implements ProviderProbeStrategy {
|
||||
/** Conservative HTTP timeout — keeps a stalled provider from holding up the parallel batch. */
|
||||
private static final Duration TIMEOUT = Duration.ofSeconds(5);
|
||||
|
||||
/** Matches a trailing {@code /v1}, {@code /v2}, ..., {@code /v99} segment on the base URL. */
|
||||
private static final Pattern VERSION_SUFFIX = Pattern.compile("/v\\d{1,2}$");
|
||||
|
||||
@Override
|
||||
public ModelProtocol supportedProtocol() {
|
||||
return ModelProtocol.OPENAI_COMPATIBLE;
|
||||
@ -40,16 +62,18 @@ public class OpenAiCompatibleListModelsProbe implements ProviderProbeStrategy {
|
||||
if (provider == null || !StringUtils.hasText(provider.getBaseUrl())) {
|
||||
return ProbeResult.fail(0, "base URL not configured");
|
||||
}
|
||||
String baseUrl = stripTrailingSlash(provider.getBaseUrl().trim());
|
||||
String modelsPath = resolveModelsPath(baseUrl);
|
||||
long start = System.currentTimeMillis();
|
||||
try {
|
||||
HttpClient httpClient = HttpClient.newBuilder().connectTimeout(TIMEOUT).build();
|
||||
RestClient client = RestClient.builder()
|
||||
.baseUrl(normalizeBaseUrl(provider.getBaseUrl()))
|
||||
.baseUrl(baseUrl)
|
||||
.requestFactory(new JdkClientHttpRequestFactory(httpClient))
|
||||
.defaultHeader(HttpHeaders.ACCEPT, MediaType.APPLICATION_JSON_VALUE)
|
||||
.build();
|
||||
|
||||
RestClient.RequestHeadersSpec<?> spec = client.get().uri("/v1/models");
|
||||
RestClient.RequestHeadersSpec<?> spec = client.get().uri(modelsPath);
|
||||
String apiKey = provider.getApiKey();
|
||||
if (StringUtils.hasText(apiKey)) {
|
||||
spec = spec.header(HttpHeaders.AUTHORIZATION, "Bearer " + apiKey.trim());
|
||||
@ -58,18 +82,60 @@ public class OpenAiCompatibleListModelsProbe implements ProviderProbeStrategy {
|
||||
String body = spec.retrieve().body(String.class);
|
||||
long latency = System.currentTimeMillis() - start;
|
||||
if (body == null || body.isBlank()) {
|
||||
return ProbeResult.fail(latency, "empty body from /v1/models");
|
||||
// Empty body from a 200 — unusual but not necessarily fatal; treat as ok.
|
||||
log.debug("[Probe] {} {} returned empty body — treating as ok",
|
||||
provider.getProviderId(), modelsPath);
|
||||
return ProbeResult.ok(latency);
|
||||
}
|
||||
return ProbeResult.ok(latency);
|
||||
} catch (HttpClientErrorException e) {
|
||||
long latency = System.currentTimeMillis() - start;
|
||||
int status = e.getStatusCode().value();
|
||||
// 401/403 = real auth failure → HARD remove
|
||||
if (status == 401 || status == 403) {
|
||||
log.debug("[Probe] {} {} auth failed: {}", provider.getProviderId(), modelsPath, status);
|
||||
return ProbeResult.fail(latency, "auth failed (" + status + ")");
|
||||
}
|
||||
// 404/405/410 = endpoint not implemented — many providers (e.g. Kimi for Coding)
|
||||
// don't expose /v1/models even though chat works. Stay in pool, let the chat
|
||||
// path be authoritative; the worst case is one wasted first request.
|
||||
if (status == 404 || status == 405 || status == 410) {
|
||||
log.info("[Probe] {} {} returned {} — endpoint not implemented; treating as fail-open (in-pool)",
|
||||
provider.getProviderId(), modelsPath, status);
|
||||
return ProbeResult.ok(latency);
|
||||
}
|
||||
// Other 4xx (e.g., 400 invalid request, 429 rate limit on the probe itself):
|
||||
// ambiguous — fail-open too, since these don't tell us about chat capability.
|
||||
log.info("[Probe] {} {} returned {} — fail-open (inconclusive)",
|
||||
provider.getProviderId(), modelsPath, status);
|
||||
return ProbeResult.ok(latency);
|
||||
} catch (HttpServerErrorException e) {
|
||||
long latency = System.currentTimeMillis() - start;
|
||||
// 5xx — the provider's listing infrastructure is flaky but chat may still work.
|
||||
// Fail-open and let runtime cooldown handle it if chat also fails.
|
||||
log.info("[Probe] {} {} returned {} — fail-open (5xx may be transient)",
|
||||
provider.getProviderId(), modelsPath, e.getStatusCode().value());
|
||||
return ProbeResult.ok(latency);
|
||||
} catch (Exception e) {
|
||||
long latency = System.currentTimeMillis() - start;
|
||||
log.debug("[Probe] {} /v1/models failed: {}", provider.getProviderId(), e.getMessage());
|
||||
log.debug("[Probe] {} {} failed: {}", provider.getProviderId(), modelsPath, e.getMessage());
|
||||
return ProbeResult.fail(latency, shortMessage(e));
|
||||
}
|
||||
}
|
||||
|
||||
private static String normalizeBaseUrl(String url) {
|
||||
// Strip trailing slash for clean URL composition; /v1/models then concatenates correctly.
|
||||
/**
|
||||
* Pick the right path to append. If the base URL already ends in a {@code /vN}
|
||||
* version segment, append only {@code /models}. Otherwise append {@code /v1/models}.
|
||||
* Package-private so the unit test can exercise it directly.
|
||||
*/
|
||||
static String resolveModelsPath(String baseUrl) {
|
||||
if (baseUrl != null && VERSION_SUFFIX.matcher(baseUrl).find()) {
|
||||
return "/models";
|
||||
}
|
||||
return "/v1/models";
|
||||
}
|
||||
|
||||
private static String stripTrailingSlash(String url) {
|
||||
return url.endsWith("/") ? url.substring(0, url.length() - 1) : url;
|
||||
}
|
||||
|
||||
|
||||
@ -0,0 +1,69 @@
|
||||
package vip.mate.llm.failover.probe;
|
||||
|
||||
import org.junit.jupiter.api.DisplayName;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
|
||||
/**
|
||||
* Locks down the URL-resolution rule for {@link OpenAiCompatibleListModelsProbe}:
|
||||
*
|
||||
* <ul>
|
||||
* <li>Vendors that point at the API root (OpenAI / Kimi / DeepSeek) get
|
||||
* {@code /v1/models} appended.</li>
|
||||
* <li>Vendors that include a {@code /vN} segment in their Base URL
|
||||
* (LMStudio's {@code /v1}, ZhipuAI's {@code /v4}, etc.) get only
|
||||
* {@code /models} appended — preventing the {@code /v1/v1/models} or
|
||||
* {@code /v4/v1/models} 404s the original implementation produced.</li>
|
||||
* </ul>
|
||||
*/
|
||||
class OpenAiCompatibleListModelsProbeTest {
|
||||
|
||||
@Test
|
||||
@DisplayName("API-root base URL → append /v1/models (OpenAI / DeepSeek / Kimi)")
|
||||
void apiRootBaseGetsV1Models() {
|
||||
assertEquals("/v1/models",
|
||||
OpenAiCompatibleListModelsProbe.resolveModelsPath("https://api.openai.com"));
|
||||
assertEquals("/v1/models",
|
||||
OpenAiCompatibleListModelsProbe.resolveModelsPath("https://api.deepseek.com"));
|
||||
assertEquals("/v1/models",
|
||||
OpenAiCompatibleListModelsProbe.resolveModelsPath("https://api.moonshot.cn"));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Base URL ends in /v1 → append only /models (LMStudio)")
|
||||
void v1SuffixGetsOnlyModels() {
|
||||
assertEquals("/models",
|
||||
OpenAiCompatibleListModelsProbe.resolveModelsPath("http://localhost:1234/v1"));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Base URL ends in /v4 → append only /models (ZhipuAI)")
|
||||
void v4SuffixGetsOnlyModels() {
|
||||
assertEquals("/models",
|
||||
OpenAiCompatibleListModelsProbe.resolveModelsPath("https://open.bigmodel.cn/api/paas/v4"));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Base URL ends in /v2 (hypothetical) → append only /models")
|
||||
void otherVersionSuffixGetsOnlyModels() {
|
||||
assertEquals("/models",
|
||||
OpenAiCompatibleListModelsProbe.resolveModelsPath("https://example.com/api/v2"));
|
||||
assertEquals("/models",
|
||||
OpenAiCompatibleListModelsProbe.resolveModelsPath("https://example.com/v3"));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Base URL contains /vN mid-path but doesn't end with it → append /v1/models")
|
||||
void midPathVersionDoesNotMatch() {
|
||||
assertEquals("/v1/models",
|
||||
OpenAiCompatibleListModelsProbe.resolveModelsPath("https://api.example.com/v1/proxy"));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Edge: null / blank base URL falls back to /v1/models (caller validates emptiness separately)")
|
||||
void nullOrBlankBase() {
|
||||
assertEquals("/v1/models", OpenAiCompatibleListModelsProbe.resolveModelsPath(null));
|
||||
assertEquals("/v1/models", OpenAiCompatibleListModelsProbe.resolveModelsPath(""));
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user