From 7ba8fe602b24f320e369bdabfdd37c2bae542724 Mon Sep 17 00:00:00 2001 From: matevip Date: Sun, 19 Apr 2026 17:10:27 +0800 Subject: [PATCH] feat(llm): track primary health + split BILLING / MODEL_NOT_FOUND from generic client errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Track the primary model health, not just fallback entries - NodeStreamingChatHelper accepts primaryProviderId via a new 5-arg constructor; AgentGraphBuilder passes ModelConfigEntity.getProvider() - Before the 5-retry primary loop, check healthTracker.isInCooldown(primaryProviderId): if true, log + broadcast "主模型暂时不可用（冷却中），直接尝试备选模型..." and short-circuit straight to the fallback chain. Prevents a degraded primary from burning 30+ seconds of backoff on every conversation turn. - recordPrimary(success/failure) now fires on every primary verdict — AUTH, BILLING, MODEL_NOT_FOUND, EMPTY_RESPONSE, generic UNKNOWN, and the explicit success path. Three consecutive failures push the primary provider into cooldown automatically. - Legacy 1/2/3-arg constructors leave primaryProviderId null; tracking silently disables for them so existing tests/wiring keep working. Split BILLING and MODEL_NOT_FOUND out of CLIENT_ERROR / AUTH_ERROR - BILLING (HTTP 402, "insufficient_quota", "credit balance is too low", "billing_hard_limit_reached", "quota exceeded"): payment failure on primary does not kill the call — a different provider may have credits. Skips same-model retries and heads to fallback chain. - MODEL_NOT_FOUND (HTTP 404, "Model not exist", "model_not_found", DashScope "[InvalidParameter] url error"): unknown model id will not start working on retry. Was previously misclassified as CLIENT_ERROR and terminated the whole call; now routes to fallback so a different provider can attempt with its default model. - classifyError ordering matters: BILLING / MODEL_NOT_FOUND are matched BEFORE the generic 400 / Bad Request branch, otherwise they would be swallowed by CLIENT_ERROR. Tests - ErrorClassificationTest: 11 tests, covers multi-vendor error phrasing for both new types + regression checks that 401 / 429 / 400 still classify as before - NodeStreamingChatHelperFallbackChainTest: +2 tests verifying primaryProviderId persistence on the new constructor and null on legacy ones - 181 tests pass (was 168 + 13 new) --- .../vip/mate/agent/AgentGraphBuilder.java | 8 +- .../agent/graph/NodeStreamingChatHelper.java | 152 ++++++++++++++---- .../agent/graph/ErrorClassificationTest.java | 111 +++++++++++++ ...eStreamingChatHelperFallbackChainTest.java | 33 +++- 4 files changed, 270 insertions(+), 34 deletions(-) create mode 100644 mateclaw-server/src/test/java/vip/mate/agent/graph/ErrorClassificationTest.java diff --git a/mateclaw-server/src/main/java/vip/mate/agent/AgentGraphBuilder.java b/mateclaw-server/src/main/java/vip/mate/agent/AgentGraphBuilder.java index 209ad338..2bc54133 100644 --- a/mateclaw-server/src/main/java/vip/mate/agent/AgentGraphBuilder.java +++ b/mateclaw-server/src/main/java/vip/mate/agent/AgentGraphBuilder.java @@ -260,7 +260,9 @@ public class AgentGraphBuilder { String reasoningEffort, ModelConfigEntity primaryModelConfig) { try { List fallbackChain = buildFallbackChain(primaryModelConfig); - NodeStreamingChatHelper streamingHelper = new NodeStreamingChatHelper(streamTracker, fallbackChain, llmCacheMetricsAggregator, providerHealthTracker); + NodeStreamingChatHelper streamingHelper = new NodeStreamingChatHelper( + streamTracker, fallbackChain, llmCacheMetricsAggregator, providerHealthTracker, + primaryModelConfig != null ? primaryModelConfig.getProvider() : null); ToolExecutionExecutor executor = new ToolExecutionExecutor(toolSet, toolGuardService, approvalService, streamTracker, toolTimeoutProperties, toolResultStorage, toolConcurrencyRegistry); PlanGenerationNode planGenerationNode = new PlanGenerationNode(chatModel, planningService, streamingHelper, conversationWindowManager, toolSet); StepExecutionNode stepExecutionNode = new StepExecutionNode(chatModel, toolSet, executor, planningService, streamTracker, reasoningEffort, streamingHelper, conversationWindowManager); @@ -359,7 +361,9 @@ public class AgentGraphBuilder { String reasoningEffort, ModelConfigEntity primaryModelConfig) { try { List fallbackChain = buildFallbackChain(primaryModelConfig); - NodeStreamingChatHelper streamingHelper = new NodeStreamingChatHelper(streamTracker, fallbackChain, llmCacheMetricsAggregator, providerHealthTracker); + NodeStreamingChatHelper streamingHelper = new NodeStreamingChatHelper( + streamTracker, fallbackChain, llmCacheMetricsAggregator, providerHealthTracker, + primaryModelConfig != null ? primaryModelConfig.getProvider() : null); ToolExecutionExecutor executor = new ToolExecutionExecutor(toolSet, toolGuardService, approvalService, streamTracker, toolTimeoutProperties, toolResultStorage, toolConcurrencyRegistry); ReasoningNode reasoningNode = new ReasoningNode(chatModel, toolSet, reasoningEffort, streamingHelper, conversationWindowManager, streamTracker, 0, wikiContextService); ActionNode actionNode = new ActionNode(executor, streamTracker); diff --git a/mateclaw-server/src/main/java/vip/mate/agent/graph/NodeStreamingChatHelper.java b/mateclaw-server/src/main/java/vip/mate/agent/graph/NodeStreamingChatHelper.java index e0e3c175..34b6a5a1 100644 --- a/mateclaw-server/src/main/java/vip/mate/agent/graph/NodeStreamingChatHelper.java +++ b/mateclaw-server/src/main/java/vip/mate/agent/graph/NodeStreamingChatHelper.java @@ -61,48 +61,70 @@ public class NodeStreamingChatHelper { /** Optional per-provider health tracker; {@code null} in tests or when bean absent. */ private final vip.mate.llm.failover.ProviderHealthTracker healthTracker; + /** + * Provider id of the primary {@link ChatModel} this helper drives. Used + * by {@link #streamCallInternal} to consult / update {@link #healthTracker} + * for the primary too — if a provider's API key is revoked, primary + * cooldown lets us bypass the 5-retry stall on subsequent calls within + * the same conversation. Falls back to {@code null} when unknown + * (legacy callers, tests). + */ + private final String primaryProviderId; + public NodeStreamingChatHelper(ChatStreamTracker streamTracker) { - this(streamTracker, List.of(), null, null); + this(streamTracker, List.of(), null, null, null); } /** - * @deprecated use the list-based constructor with FallbackEntry — a single - * fallback cannot express the ordered multi-provider chain + * @deprecated use the full constructor — a single fallback cannot + * express the ordered multi-provider chain */ @Deprecated public NodeStreamingChatHelper(ChatStreamTracker streamTracker, ChatModel fallbackModel) { - this(streamTracker, wrap(fallbackModel), null, null); + this(streamTracker, wrap(fallbackModel), null, null, null); } /** - * @deprecated use the list-based constructor with FallbackEntry — a single - * fallback cannot express the ordered multi-provider chain + * @deprecated use the full constructor. */ @Deprecated public NodeStreamingChatHelper(ChatStreamTracker streamTracker, ChatModel fallbackModel, vip.mate.llm.cache.LlmCacheMetricsAggregator cacheMetrics) { - this(streamTracker, wrap(fallbackModel), cacheMetrics, null); + this(streamTracker, wrap(fallbackModel), cacheMetrics, null, null); } /** - * Full chain constructor without health tracker — primarily for tests and - * legacy wiring. Production callers should use the 4-arg variant so - * cooldown state is honored. + * Chain constructor without health tracker — primarily for tests and + * legacy wiring. Production callers should use the full constructor. */ public NodeStreamingChatHelper(ChatStreamTracker streamTracker, List fallbackChain, vip.mate.llm.cache.LlmCacheMetricsAggregator cacheMetrics) { - this(streamTracker, fallbackChain, cacheMetrics, null); + this(streamTracker, fallbackChain, cacheMetrics, null, null); + } + + /** + * Constructor with health tracker but unknown primary provider — used by + * tests where the helper isn't tied to a specific primary. Primary + * health tracking is disabled for instances built this way. + */ + public NodeStreamingChatHelper(ChatStreamTracker streamTracker, + List fallbackChain, + vip.mate.llm.cache.LlmCacheMetricsAggregator cacheMetrics, + vip.mate.llm.failover.ProviderHealthTracker healthTracker) { + this(streamTracker, fallbackChain, cacheMetrics, healthTracker, null); } public NodeStreamingChatHelper(ChatStreamTracker streamTracker, List fallbackChain, vip.mate.llm.cache.LlmCacheMetricsAggregator cacheMetrics, - vip.mate.llm.failover.ProviderHealthTracker healthTracker) { + vip.mate.llm.failover.ProviderHealthTracker healthTracker, + String primaryProviderId) { this.streamTracker = streamTracker; this.fallbackChain = fallbackChain == null ? List.of() : List.copyOf(fallbackChain); this.cacheMetrics = cacheMetrics; this.healthTracker = healthTracker; + this.primaryProviderId = primaryProviderId; } private static List wrap(ChatModel m) { @@ -111,6 +133,17 @@ public class NodeStreamingChatHelper { return m == null ? List.of() : List.of(new vip.mate.llm.failover.FallbackEntry("__legacy__", m)); } + /** + * Record a single primary-model outcome to the health tracker. No-op when + * either the tracker bean isn't wired or the primary's providerId is + * unknown (e.g., tests, legacy callers built without the full constructor). + */ + private void recordPrimary(boolean success) { + if (healthTracker == null || primaryProviderId == null) return; + if (success) healthTracker.recordSuccess(primaryProviderId); + else healthTracker.recordFailure(primaryProviderId); + } + /** * 流式调用 LLM 并实时广播增量内容 * @@ -202,24 +235,38 @@ public class NodeStreamingChatHelper { || msg.contains("thinking block")) { return ErrorType.THINKING_BLOCK_ERROR; } + // BILLING — payment / quota exhausted. Distinct from AUTH because + // a different provider may have credits, so we should fall back instead of + // terminating the call. Both OpenAI ("insufficient_quota") and Anthropic + // ("credit balance is too low") use these phrases in 402-class responses. + if (msg.contains("402") || msg.contains("insufficient_quota") + || msg.contains("credit balance is too low") + || msg.contains("billing_error") || msg.contains("billing_hard_limit_reached") + || msg.contains("You exceeded your current quota") + || msg.contains("quota exceeded") || msg.contains("Quota exceeded")) { + return ErrorType.BILLING; + } + // MODEL_NOT_FOUND — provider rejects the requested model id. + // Includes DashScope's "[InvalidParameter] url error, please check url" + // (https://help.aliyun.com/zh/model-studio/error-code#error-url) which despite + // the wording is the provider rejecting an unknown/unsupported model id on + // the native protocol. Splitting this out from CLIENT_ERROR lets us hand off + // to the fallback chain instead of terminating — a different provider may + // recognize the model name (or have an equivalent default). + if (msg.contains("Model not exist") + || msg.contains("model_not_found") + || msg.contains("Model not found") + || msg.contains("does not exist") + || msg.contains("[InvalidParameter]") + || msg.contains("InvalidParameter") + || msg.contains("url error")) { + return ErrorType.MODEL_NOT_FOUND; + } // Client errors (400 Bad Request — unsupported format, invalid params, etc.) — NOT retryable if (msg.contains("400") || msg.contains("Bad Request") || msg.contains("invalid_request_error") || msg.contains("unsupported")) { return ErrorType.CLIENT_ERROR; } - // DashScope-specific "model name does not map to a valid endpoint" — reported as - // "[InvalidParameter] url error, please check url" (see - // https://help.aliyun.com/zh/model-studio/error-code#error-url). Despite the wording - // it's not a URL issue — it's the provider rejecting an unknown/unsupported model id - // on the native protocol. Treat as client error so we do NOT retry. - if (msg.contains("[InvalidParameter]") - || msg.contains("InvalidParameter") - || msg.contains("url error") - || msg.contains("Model not exist") - || msg.contains("model_not_found") - || msg.contains("Model not found")) { - return ErrorType.CLIENT_ERROR; - } // Server errors if (msg.contains("500") || msg.contains("502") || msg.contains("503") || msg.contains("504") || msg.contains("APITimeoutError") || msg.contains("APIConnectionError") @@ -253,19 +300,47 @@ public class NodeStreamingChatHelper { throw new CancellationException("Stream stopped by user"); } + // if the primary's provider is in cooldown (3+ recent + // consecutive failures within the cooldown window), skip the 5-retry + // primary loop entirely and head straight to the fallback chain. + // Without this short-circuit a degraded primary forces every LLM + // call in the conversation to wait through the full backoff. + boolean primarySkipped = primaryProviderId != null + && healthTracker != null + && healthTracker.isInCooldown(primaryProviderId); + if (primarySkipped) { + log.warn("[{}] Primary provider={} is in cooldown — skipping straight to fallback chain", + phase, primaryProviderId); + if (broadcast) { + broadcastDelta(conversationId, "warning", + buildDeltaJson("主模型暂时不可用（冷却中），直接尝试备选模型...")); + } + } + // 主模型重试循环 StreamResult lastResult = null; - for (int attempt = 0; attempt <= MAX_RETRIES; attempt++) { + if (!primarySkipped) for (int attempt = 0; attempt <= MAX_RETRIES; attempt++) { lastResult = doStreamCall(chatModel, prompt, conversationId, phase, broadcast, attempt); if (lastResult != null) { // PTL: 不重试，直接返回给上层 Node 处理 if (lastResult.errorType() == ErrorType.PROMPT_TOO_LONG) { return lastResult; } - // AUTH: 不重试 + // AUTH: 不重试 — 但要记账（auth 不会自愈，连续 N 次后冷却避免每轮都撞） if (lastResult.errorType() == ErrorType.AUTH_ERROR) { + recordPrimary(false); return lastResult; } + // BILLING / MODEL_NOT_FOUND — provider-side hard failures + // that won't change on retry. Skip to fallback chain (a different + // provider may have credits, or the model name may be valid there). + if (lastResult.errorType() == ErrorType.BILLING + || lastResult.errorType() == ErrorType.MODEL_NOT_FOUND) { + log.warn("[{}] Primary error={} — skipping same-model retries, handing off to fallback chain", + phase, lastResult.errorType()); + recordPrimary(false); + break; + } // CLIENT_ERROR (400 Bad Request): 不重试（参数/格式错误重试也不会变） if (lastResult.errorType() == ErrorType.CLIENT_ERROR) { return lastResult; @@ -284,20 +359,27 @@ public class NodeStreamingChatHelper { // productive; a different provider has a better chance of succeeding. if (lastResult.errorType() == ErrorType.EMPTY_RESPONSE) { log.warn("[{}] Primary returned empty response — skipping same-model retries, handing off to fallback chain", phase); + recordPrimary(false); break; } // 成功 if (lastResult.errorMessage() == null || lastResult.errorType() == ErrorType.NONE) { + recordPrimary(true); return lastResult; } // Any other non-null errored result with a classified type that doStreamCall // chose NOT to retry (i.e. UNKNOWN, or RATE_LIMIT/SERVER_ERROR past MAX_RETRIES) // must exit — otherwise we silently spin through attempts and waste seconds // per turn on unrecoverable errors like DashScope's "url error" / unknown model. + recordPrimary(false); return lastResult; } // lastResult == null 表示需要重试 } + // If we exhausted the retry loop without a verdict, primary effectively failed. + if (!primarySkipped && lastResult != null && lastResult.errorType() != ErrorType.NONE) { + recordPrimary(false); + } // Primary exhausted retries — walk the fallback chain in priority order. // Each fallback gets a single shot (no retry); first successful result wins. @@ -850,6 +932,22 @@ public class NodeStreamingChatHelper { * rejection that comes back as HTTP 200 with empty body. */ EMPTY_RESPONSE, + /** + * payment / billing failure (HTTP 402, "insufficient_quota", + * "credit balance is too low", etc.). Distinct from {@link #AUTH_ERROR} + * because the right response is to switch provider (a different + * provider may have credits) rather than just terminate. Skips same-model + * retries and falls through to the fallback chain. + */ + BILLING, + /** + * requested model id not recognized by the provider + * (HTTP 404, "Model not exist", "model_not_found", DashScope's + * "url error"). Same handling as {@link #BILLING} — heads straight + * to the fallback chain instead of looping retries against a model + * that does not exist. + */ + MODEL_NOT_FOUND, /** 其他未知错误 */ UNKNOWN } diff --git a/mateclaw-server/src/test/java/vip/mate/agent/graph/ErrorClassificationTest.java b/mateclaw-server/src/test/java/vip/mate/agent/graph/ErrorClassificationTest.java new file mode 100644 index 00000000..58ead87e --- /dev/null +++ b/mateclaw-server/src/test/java/vip/mate/agent/graph/ErrorClassificationTest.java @@ -0,0 +1,111 @@ +package vip.mate.agent.graph; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.lang.reflect.Method; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * classification tests for the new error types + * ({@link NodeStreamingChatHelper.ErrorType#BILLING}, + * {@link NodeStreamingChatHelper.ErrorType#MODEL_NOT_FOUND}). + * + *

These two are split out from {@code AUTH_ERROR} / {@code CLIENT_ERROR} + * because the right action is to switch provider, not to terminate. + * Mis-classifying a billing error as auth would break the whole call chain.

+ */ +class ErrorClassificationTest { + + private static NodeStreamingChatHelper.ErrorType classify(Throwable t) throws Exception { + Method m = NodeStreamingChatHelper.class.getDeclaredMethod("classifyError", Throwable.class); + m.setAccessible(true); + return (NodeStreamingChatHelper.ErrorType) m.invoke(null, t); + } + + // ===== BILLING ===== + + @Test + @DisplayName("HTTP 402 → BILLING") + void status402IsBilling() throws Exception { + assertEquals(NodeStreamingChatHelper.ErrorType.BILLING, + classify(new RuntimeException("402 Payment Required"))); + } + + @Test + @DisplayName("OpenAI 'insufficient_quota' → BILLING") + void openaiQuotaIsBilling() throws Exception { + assertEquals(NodeStreamingChatHelper.ErrorType.BILLING, + classify(new RuntimeException("Error code: insufficient_quota — please check your plan"))); + } + + @Test + @DisplayName("Anthropic 'credit balance is too low' → BILLING") + void anthropicCreditIsBilling() throws Exception { + assertEquals(NodeStreamingChatHelper.ErrorType.BILLING, + classify(new RuntimeException("Your credit balance is too low to access the API"))); + } + + @Test + @DisplayName("'You exceeded your current quota' → BILLING") + void quotaExceededIsBilling() throws Exception { + assertEquals(NodeStreamingChatHelper.ErrorType.BILLING, + classify(new RuntimeException("You exceeded your current quota, please check your plan"))); + } + + // ===== MODEL_NOT_FOUND ===== + + @Test + @DisplayName("'Model not exist' → MODEL_NOT_FOUND") + void modelNotExistIsModelNotFound() throws Exception { + assertEquals(NodeStreamingChatHelper.ErrorType.MODEL_NOT_FOUND, + classify(new RuntimeException("Model not exist: gpt-99"))); + } + + @Test + @DisplayName("'model_not_found' → MODEL_NOT_FOUND") + void modelNotFoundCodeIsModelNotFound() throws Exception { + assertEquals(NodeStreamingChatHelper.ErrorType.MODEL_NOT_FOUND, + classify(new RuntimeException("Error: model_not_found"))); + } + + @Test + @DisplayName("DashScope '[InvalidParameter] url error' → MODEL_NOT_FOUND (not CLIENT_ERROR)") + void dashscopeInvalidParameterIsModelNotFound() throws Exception { + // Despite the wording, DashScope returns this when the model id is unknown + // — the right action is to try a fallback provider, not terminate as 400. + assertEquals(NodeStreamingChatHelper.ErrorType.MODEL_NOT_FOUND, + classify(new RuntimeException("[InvalidParameter] url error, please check url"))); + } + + @Test + @DisplayName("Anthropic 'model does not exist' → MODEL_NOT_FOUND") + void anthropicDoesNotExistIsModelNotFound() throws Exception { + assertEquals(NodeStreamingChatHelper.ErrorType.MODEL_NOT_FOUND, + classify(new RuntimeException("model claude-99 does not exist"))); + } + + // ===== Regression: existing classifications still work ===== + + @Test + @DisplayName("HTTP 401 still classifies as AUTH_ERROR (not billing)") + void status401StillAuth() throws Exception { + assertEquals(NodeStreamingChatHelper.ErrorType.AUTH_ERROR, + classify(new RuntimeException("401 Unauthorized: Invalid API Key"))); + } + + @Test + @DisplayName("HTTP 429 still classifies as RATE_LIMIT") + void status429StillRateLimit() throws Exception { + assertEquals(NodeStreamingChatHelper.ErrorType.RATE_LIMIT, + classify(new RuntimeException("429 Too Many Requests"))); + } + + @Test + @DisplayName("Plain 400 Bad Request still classifies as CLIENT_ERROR") + void status400StillClientError() throws Exception { + assertEquals(NodeStreamingChatHelper.ErrorType.CLIENT_ERROR, + classify(new RuntimeException("400 Bad Request: malformed JSON"))); + } +} diff --git a/mateclaw-server/src/test/java/vip/mate/agent/graph/NodeStreamingChatHelperFallbackChainTest.java b/mateclaw-server/src/test/java/vip/mate/agent/graph/NodeStreamingChatHelperFallbackChainTest.java index 0f6f4296..1765f33b 100644 --- a/mateclaw-server/src/test/java/vip/mate/agent/graph/NodeStreamingChatHelperFallbackChainTest.java +++ b/mateclaw-server/src/test/java/vip/mate/agent/graph/NodeStreamingChatHelperFallbackChainTest.java @@ -88,12 +88,35 @@ class NodeStreamingChatHelperFallbackChainTest { } @Test - @DisplayName("EMPTY_RESPONSE error type exists (fallback trigger)") - void emptyResponseErrorTypeExists() { - // Compile-time safety net: the enum constant the streaming pipeline relies on + @DisplayName("EMPTY_RESPONSE / BILLING / MODEL_NOT_FOUND error types exist ") + void fallbackTriggerErrorTypesExist() { + // Compile-time safety net: these enum constants the streaming pipeline relies on // must not be renamed or removed without breaking the fallback contract. - NodeStreamingChatHelper.ErrorType t = NodeStreamingChatHelper.ErrorType.EMPTY_RESPONSE; - assertNotNull(t); + assertNotNull(NodeStreamingChatHelper.ErrorType.EMPTY_RESPONSE); + assertNotNull(NodeStreamingChatHelper.ErrorType.BILLING); + assertNotNull(NodeStreamingChatHelper.ErrorType.MODEL_NOT_FOUND); + } + + @Test + @DisplayName("primary providerId is stored when supplied via the full constructor") + void primaryProviderIdStored() throws Exception { + NodeStreamingChatHelper helper = new NodeStreamingChatHelper( + streamTracker, List.of(), null, null, "openai"); + + Field f = NodeStreamingChatHelper.class.getDeclaredField("primaryProviderId"); + f.setAccessible(true); + assertEquals("openai", f.get(helper), + "primary provider id must be retained for health tracking"); + } + + @Test + @DisplayName("legacy constructors leave primaryProviderId null (tracking disabled)") + void primaryProviderIdNullForLegacyConstructors() throws Exception { + NodeStreamingChatHelper helper = new NodeStreamingChatHelper(streamTracker); + Field f = NodeStreamingChatHelper.class.getDeclaredField("primaryProviderId"); + f.setAccessible(true); + assertNull(f.get(helper), + "legacy constructors must leave primaryProviderId unset so tracking is silently disabled"); } @SuppressWarnings("unchecked")