From edab153fc5741b915f935a6873b38667fbacfe8a Mon Sep 17 00:00:00 2001 From: umaru Date: Sun, 24 May 2026 21:06:21 +0800 Subject: [PATCH] =?UTF-8?q?feat(failure-rules):=20=E9=94=99=E8=AF=AF?= =?UTF-8?q?=E7=B1=BB=E5=9E=8B=E6=9E=9A=E4=B8=BE=E5=BC=BA=E6=A0=A1=E9=AA=8C?= =?UTF-8?q?=20+=20=E5=A4=9A=E9=80=89=20UI?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 把上游失败规则的 error_types 字段从自由 CSV 升级为封闭枚举： - 新增 src/lib/constants/failover-error-types.ts 作为 FailoverErrorType 的运行时单一来源（数组 + zod enum + 类型守卫），让前后端共享。 - 三处 admin route schema（全局 POST、单条 PUT、scoped POST）改用 z.array(failoverErrorTypeSchema)，拼错会直接 400。 - 服务层 assertValidRuleMatch 增加 enum 校验作为第二防线；ruleMatchesEvidence 命中含未知值的历史规则时以 per-rule 去重的方式 log.warn 一次，让运维通过日志感知到沉默失效。 - 前端 Editor 把 CSV Input 替换为 FailoverErrorTypeMultiSelect（基于 shadcn Popover + Checkbox + Badge 拼装），全选/清空、未知遗留值用红色 Badge + tooltip 提示删除；列表展示同样区分已知 label 与未知字符串。 - i18n 同步：英中文新增 SelectAll / ClearAll / RemoveAria / UnknownTooltip，Placeholder 改为选择式文案。 - 测试：服务层新增"拒绝未知 errorType / 历史含未知值的规则仍可匹配合法 evidence"两条；editor 测试将 MultiSelect mock 为按钮列表，断言数据契约。 --- .../upstream-failure-rules/[id]/route.ts | 3 +- .../api/admin/upstream-failure-rules/route.ts | 3 +- .../upstreams/[id]/failure-rules/route.ts | 3 +- .../failover-error-type-multi-select.tsx | 194 ++++++++++++++++++ .../admin/upstream-failure-rules-editor.tsx | 78 +++++-- src/lib/constants/failover-error-types.ts | 22 ++ src/lib/services/upstream-failure-rules.ts | 39 +++- src/messages/en.json | 6 +- src/messages/zh-CN.json | 6 +- .../upstream-failure-rules-editor.test.tsx | 53 ++++- .../services/upstream-failure-rules.test.ts | 41 ++++ 11 files changed, 424 insertions(+), 24 deletions(-) create mode 100644 src/components/admin/failover-error-type-multi-select.tsx create mode 100644 src/lib/constants/failover-error-types.ts diff --git a/src/app/api/admin/upstream-failure-rules/[id]/route.ts b/src/app/api/admin/upstream-failure-rules/[id]/route.ts index 242cac22..396f6ac7 100644 --- a/src/app/api/admin/upstream-failure-rules/[id]/route.ts +++ b/src/app/api/admin/upstream-failure-rules/[id]/route.ts @@ -8,6 +8,7 @@ import { parseFailureRuleMatch, updateFailureRule, } from "@/lib/services/upstream-failure-rules"; +import { failoverErrorTypeSchema } from "@/lib/constants/failover-error-types"; import { createLogger } from "@/lib/utils/logger"; import { z } from "zod"; @@ -17,7 +18,7 @@ type RouteContext = { params: Promise<{ id: string }> }; const failureRuleMatchSchema = z.object({ status_codes: z.array(z.number().int().min(100).max(599)).nullable().optional(), - error_types: z.array(z.string().trim().min(1)).nullable().optional(), + error_types: z.array(failoverErrorTypeSchema).nullable().optional(), body_pattern: z.string().nullable().optional(), header_name: z.string().nullable().optional(), header_pattern: z.string().nullable().optional(), diff --git a/src/app/api/admin/upstream-failure-rules/route.ts b/src/app/api/admin/upstream-failure-rules/route.ts index af6d5d92..2d557dd9 100644 --- a/src/app/api/admin/upstream-failure-rules/route.ts +++ b/src/app/api/admin/upstream-failure-rules/route.ts @@ -8,6 +8,7 @@ import { listFailureRules, parseFailureRuleMatch, } from "@/lib/services/upstream-failure-rules"; +import { failoverErrorTypeSchema } from "@/lib/constants/failover-error-types"; import { createLogger } from "@/lib/utils/logger"; import { z } from "zod"; @@ -15,7 +16,7 @@ const log = createLogger("admin-upstream-failure-rules"); const failureRuleMatchSchema = z.object({ status_codes: z.array(z.number().int().min(100).max(599)).nullable().optional(), - error_types: z.array(z.string().trim().min(1)).nullable().optional(), + error_types: z.array(failoverErrorTypeSchema).nullable().optional(), body_pattern: z.string().nullable().optional(), header_name: z.string().nullable().optional(), header_pattern: z.string().nullable().optional(), diff --git a/src/app/api/admin/upstreams/[id]/failure-rules/route.ts b/src/app/api/admin/upstreams/[id]/failure-rules/route.ts index 59c75355..b69b17f8 100644 --- a/src/app/api/admin/upstreams/[id]/failure-rules/route.ts +++ b/src/app/api/admin/upstreams/[id]/failure-rules/route.ts @@ -8,6 +8,7 @@ import { listFailureRules, parseFailureRuleMatch, } from "@/lib/services/upstream-failure-rules"; +import { failoverErrorTypeSchema } from "@/lib/constants/failover-error-types"; import { createLogger } from "@/lib/utils/logger"; import { z } from "zod"; @@ -17,7 +18,7 @@ type RouteContext = { params: Promise<{ id: string }> }; const failureRuleMatchSchema = z.object({ status_codes: z.array(z.number().int().min(100).max(599)).nullable().optional(), - error_types: z.array(z.string().trim().min(1)).nullable().optional(), + error_types: z.array(failoverErrorTypeSchema).nullable().optional(), body_pattern: z.string().nullable().optional(), header_name: z.string().nullable().optional(), header_pattern: z.string().nullable().optional(), diff --git a/src/components/admin/failover-error-type-multi-select.tsx b/src/components/admin/failover-error-type-multi-select.tsx new file mode 100644 index 00000000..245bb924 --- /dev/null +++ b/src/components/admin/failover-error-type-multi-select.tsx @@ -0,0 +1,194 @@ +"use client"; + +import { useMemo, useState } from "react"; +import { ChevronDown, X } from "lucide-react"; +import { Badge } from "@/components/ui/badge"; +import { Button } from "@/components/ui/button"; +import { Checkbox } from "@/components/ui/checkbox"; +import { Popover, PopoverContent, PopoverTrigger } from "@/components/ui/popover"; +import { + FAILOVER_ERROR_TYPES, + isKnownFailoverErrorType, +} from "@/lib/constants/failover-error-types"; +import { cn } from "@/lib/utils"; +import type { FailoverErrorType } from "@/types/api"; + +export interface FailoverErrorTypeMultiSelectProps { + value: string[]; + onChange: (next: string[]) => void; + getLabel: (type: FailoverErrorType) => string; + placeholder: string; + selectAllLabel: string; + clearAllLabel: string; + unknownTooltip: string; + removeAriaLabel: string; + className?: string; +} + +export function FailoverErrorTypeMultiSelect({ + value, + onChange, + getLabel, + placeholder, + selectAllLabel, + clearAllLabel, + unknownTooltip, + removeAriaLabel, + className, +}: FailoverErrorTypeMultiSelectProps) { + const [open, setOpen] = useState(false); + + const selectedSet = useMemo(() => new Set(value), [value]); + const knownSelected = useMemo( + () => FAILOVER_ERROR_TYPES.filter((type) => selectedSet.has(type)), + [selectedSet] + ); + const unknownSelected = useMemo( + () => value.filter((entry) => !isKnownFailoverErrorType(entry)), + [value] + ); + const allKnownSelected = knownSelected.length === FAILOVER_ERROR_TYPES.length; + + const toggleType = (type: FailoverErrorType, checked: boolean) => { + if (checked) { + if (selectedSet.has(type)) return; + onChange([...value, type]); + } else { + onChange(value.filter((entry) => entry !== type)); + } + }; + + const removeEntry = (entry: string) => { + onChange(value.filter((existing) => existing !== entry)); + }; + + const selectAllKnown = () => { + const merged = [...unknownSelected, ...FAILOVER_ERROR_TYPES]; + onChange(merged); + }; + + const clearAll = () => { + onChange([]); + }; + + return ( +
+ + + + + +
+ +
+
+ {FAILOVER_ERROR_TYPES.map((type) => { + const checked = selectedSet.has(type); + const inputId = `failover-error-type-${type}`; + return ( + + ); + })} +
+ + +
+ ); +} diff --git a/src/components/admin/upstream-failure-rules-editor.tsx b/src/components/admin/upstream-failure-rules-editor.tsx index cea5c9ea..47817aec 100644 --- a/src/components/admin/upstream-failure-rules-editor.tsx +++ b/src/components/admin/upstream-failure-rules-editor.tsx @@ -1,6 +1,6 @@ "use client"; -import { useMemo, useState } from "react"; +import { useCallback, useMemo, useState, type ReactNode } from "react"; import { Loader2, Plus, Search, Trash2 } from "lucide-react"; import { useTranslations } from "next-intl"; import { toast } from "sonner"; @@ -10,6 +10,8 @@ import { Checkbox } from "@/components/ui/checkbox"; import { Input } from "@/components/ui/input"; import { Switch } from "@/components/ui/switch"; import { Textarea } from "@/components/ui/textarea"; +import { FailoverErrorTypeMultiSelect } from "@/components/admin/failover-error-type-multi-select"; +import { isKnownFailoverErrorType } from "@/lib/constants/failover-error-types"; import { useCreateGlobalUpstreamFailureRule, useCreateUpstreamFailureRule, @@ -41,16 +43,17 @@ function parseStatusCodes(value: string): number[] | null { function buildMatch(input: { statusCodes: string; - errorTypes: string; + errorTypes: string[]; bodyPattern: string; headerName: string; headerPattern: string; }): UpstreamFailureRuleMatch { const normalizedHeaderName = input.headerName.trim(); const normalizedHeaderPattern = input.headerPattern.trim(); + const knownErrorTypes = input.errorTypes.filter(isKnownFailoverErrorType); return { status_codes: parseStatusCodes(input.statusCodes), - error_types: parseCsvList(input.errorTypes) as FailoverErrorType[], + error_types: knownErrorTypes.length ? knownErrorTypes : null, body_pattern: input.bodyPattern.trim() || null, header_name: normalizedHeaderName && normalizedHeaderPattern ? normalizedHeaderName : null, header_pattern: @@ -70,12 +73,41 @@ function hasRuleCondition(match: UpstreamFailureRuleMatch): boolean { interface RuleConditionDetail { key: string; label: string; - value: string; + value: ReactNode; +} + +function renderErrorTypeChips( + errorTypes: readonly string[], + getErrorTypeLabel: (type: FailoverErrorType) => string, + unknownTooltip: string +): ReactNode { + return ( +
+ {errorTypes.map((entry) => + isKnownFailoverErrorType(entry) ? ( + + {getErrorTypeLabel(entry)} + + ) : ( + + {entry} + + ) + )} +
+ ); } function getRuleConditionDetails( match: UpstreamFailureRuleMatch, - t: ReturnType + t: ReturnType, + getErrorTypeLabel: (type: FailoverErrorType) => string, + unknownTooltip: string ): RuleConditionDetail[] { const details: RuleConditionDetail[] = []; if (match.status_codes?.length) { @@ -89,7 +121,7 @@ function getRuleConditionDetails( details.push({ key: "error_types", label: t("failureRuleErrorTypes"), - value: match.error_types.join(", "), + value: renderErrorTypeChips(match.error_types, getErrorTypeLabel, unknownTooltip), }); } if (match.body_pattern) { @@ -235,6 +267,12 @@ export function UpstreamFailureRulesEditor({ scope = "upstream", }: UpstreamFailureRulesEditorProps) { const t = useTranslations("upstreams"); + const tErrorType = useTranslations("requestLogs.retryErrorType"); + const getErrorTypeLabel = useCallback( + (type: FailoverErrorType) => tErrorType(type), + [tErrorType] + ); + const unknownTooltip = t("failureRuleErrorTypeUnknownTooltip"); const localRulesQuery = useUpstreamFailureRules( upstreamId, scope === "upstream" && Boolean(upstreamId) @@ -248,7 +286,7 @@ export function UpstreamFailureRulesEditor({ const [name, setName] = useState(""); const [enabled, setEnabled] = useState(true); const [statusCodes, setStatusCodes] = useState(""); - const [errorTypes, setErrorTypes] = useState(""); + const [errorTypes, setErrorTypes] = useState([]); const [bodyPattern, setBodyPattern] = useState(""); const [headerName, setHeaderName] = useState(""); const [headerPattern, setHeaderPattern] = useState(""); @@ -290,7 +328,7 @@ export function UpstreamFailureRulesEditor({ setName(""); setEnabled(true); setStatusCodes(""); - setErrorTypes(""); + setErrorTypes([]); setBodyPattern(""); setHeaderName(""); setHeaderPattern(""); @@ -343,7 +381,12 @@ export function UpstreamFailureRulesEditor({ {filteredRules.length ? ( filteredRules.map((rule) => { - const conditionDetails = getRuleConditionDetails(rule.match, t); + const conditionDetails = getRuleConditionDetails( + rule.match, + t, + getErrorTypeLabel, + unknownTooltip + ); return (
{detail.label}
-
+
{detail.value}
@@ -477,10 +526,15 @@ export function UpstreamFailureRulesEditor({ onChange={(event) => setStatusCodes(event.target.value)} placeholder={t("failureRuleStatusCodesPlaceholder")} /> - setErrorTypes(event.target.value)} + onChange={setErrorTypes} + getLabel={getErrorTypeLabel} placeholder={t("failureRuleErrorTypesPlaceholder")} + selectAllLabel={t("failureRuleErrorTypesSelectAll")} + clearAllLabel={t("failureRuleErrorTypesClearAll")} + unknownTooltip={unknownTooltip} + removeAriaLabel={t("failureRuleErrorTypesRemoveAria")} /> !isKnownFailoverErrorType(value)); + if (unknown.length) { + throw new InvalidFailureRuleError( + `Unknown error types: ${unknown.join(", ")}. Allowed values: ${FAILOVER_ERROR_TYPES.join(", ")}` + ); + } + } + if (match.bodyPattern?.trim()) { try { new RegExp(match.bodyPattern); @@ -129,6 +142,24 @@ function testRegex(pattern: string, value: string | null | undefined): boolean { } } +const warnedUnknownErrorTypeRuleIds = new Set(); + +function warnUnknownErrorTypesOnce(rule: UpstreamFailureRule, unknown: string[]): void { + if (!unknown.length || warnedUnknownErrorTypeRuleIds.has(rule.id)) { + return; + } + warnedUnknownErrorTypeRuleIds.add(rule.id); + log.warn( + { + ruleId: rule.id, + ruleName: rule.name, + unknown, + allowed: FAILOVER_ERROR_TYPES, + }, + "upstream failure rule contains unknown error types and will never match those values; edit the rule to remove or correct them" + ); +} + function ruleMatchesEvidence(rule: UpstreamFailureRule, evidence: FailureEvidence): boolean { const match = rule.match as UpstreamFailureRuleMatch; @@ -136,8 +167,12 @@ function ruleMatchesEvidence(rule: UpstreamFailureRule, evidence: FailureEvidenc return false; } - if (match.errorTypes?.length && !match.errorTypes.includes(evidence.errorType ?? "")) { - return false; + if (match.errorTypes?.length) { + const unknown = match.errorTypes.filter((value) => !isKnownFailoverErrorType(value)); + warnUnknownErrorTypesOnce(rule, unknown); + if (!match.errorTypes.includes(evidence.errorType ?? "")) { + return false; + } } if (match.bodyPattern) { diff --git a/src/messages/en.json b/src/messages/en.json index 6fbdb403..4a9626c8 100644 --- a/src/messages/en.json +++ b/src/messages/en.json @@ -954,7 +954,11 @@ "failureRuleEnabled": "Enabled", "failureRuleCreateEnabled": "Create as enabled", "failureRuleStatusCodesPlaceholder": "HTTP statuses, e.g. 400,429", - "failureRuleErrorTypesPlaceholder": "Error types, e.g. http_4xx,timeout", + "failureRuleErrorTypesPlaceholder": "Select error types to match", + "failureRuleErrorTypesSelectAll": "Select all", + "failureRuleErrorTypesClearAll": "Clear all", + "failureRuleErrorTypesRemoveAria": "Remove this error type", + "failureRuleErrorTypeUnknownTooltip": "This error type is no longer in the supported enum and will never match any upstream event. Remove or replace it.", "failureRuleBodyPatternPlaceholder": "Response body regex", "failureRuleHeaderNamePlaceholder": "Header name", "failureRuleHeaderPatternPlaceholder": "Header regex", diff --git a/src/messages/zh-CN.json b/src/messages/zh-CN.json index 053e652f..c479f015 100644 --- a/src/messages/zh-CN.json +++ b/src/messages/zh-CN.json @@ -959,7 +959,11 @@ "failureRuleEnabled": "启用", "failureRuleCreateEnabled": "创建后立即启用", "failureRuleStatusCodesPlaceholder": "HTTP 状态码，例如 400,429", - "failureRuleErrorTypesPlaceholder": "错误类型，例如 http_4xx,timeout", + "failureRuleErrorTypesPlaceholder": "选择需要匹配的错误类型", + "failureRuleErrorTypesSelectAll": "全选", + "failureRuleErrorTypesClearAll": "清空", + "failureRuleErrorTypesRemoveAria": "移除该错误类型", + "failureRuleErrorTypeUnknownTooltip": "该错误类型不在当前支持的枚举内，已不会被任何上游事件命中，请删除或替换。", "failureRuleBodyPatternPlaceholder": "响应体正则", "failureRuleHeaderNamePlaceholder": "响应头名称", "failureRuleHeaderPatternPlaceholder": "响应头正则", diff --git a/tests/components/upstream-failure-rules-editor.test.tsx b/tests/components/upstream-failure-rules-editor.test.tsx index 7600dee6..f1ac3cd8 100644 --- a/tests/components/upstream-failure-rules-editor.test.tsx +++ b/tests/components/upstream-failure-rules-editor.test.tsx @@ -3,6 +3,50 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { UpstreamFailureRulesEditor } from "@/components/admin/upstream-failure-rules-editor"; import type { UpstreamFailureRule } from "@/types/api"; +vi.mock("@/components/admin/failover-error-type-multi-select", () => ({ + FailoverErrorTypeMultiSelect: ({ + value, + onChange, + }: { + value: string[]; + onChange: (next: string[]) => void; + }) => ( +
+ {[ + "timeout", + "first_byte_timeout", + "upstream_no_content_stream", + "stream_idle_timeout", + "stream_error", + "http_5xx", + "http_4xx", + "http_429", + "connection_error", + "circuit_open", + "concurrency_full", + ].map((type) => ( + + ))} +
    + {value.map((entry) => ( +
  • + {entry} +
    • + ))} +
+
+ ), +})); + const { mockToastError, mockToastSuccess } = vi.hoisted(() => ({ mockToastError: vi.fn(), mockToastSuccess: vi.fn(), @@ -259,9 +303,8 @@ describe("UpstreamFailureRulesEditor", () => { fireEvent.change(screen.getByPlaceholderText("failureRuleStatusCodesPlaceholder"), { target: { value: " 429, 200, 99, abc " }, }); - fireEvent.change(screen.getByPlaceholderText("failureRuleErrorTypesPlaceholder"), { - target: { value: "timeout, upstream_error" }, - }); + fireEvent.click(screen.getByLabelText("add-timeout")); + fireEvent.click(screen.getByLabelText("add-http_5xx")); fireEvent.change(screen.getByPlaceholderText("failureRuleBodyPatternPlaceholder"), { target: { value: "rate limit" }, }); @@ -281,7 +324,7 @@ describe("UpstreamFailureRulesEditor", () => { enabled: true, match: { status_codes: [429, 200], - error_types: ["timeout", "upstream_error"], + error_types: ["timeout", "http_5xx"], body_pattern: "rate limit", header_name: "retry-after", header_pattern: "\\d+", @@ -314,7 +357,7 @@ describe("UpstreamFailureRulesEditor", () => { enabled: true, match: { status_codes: [500, 503], - error_types: [], + error_types: null, body_pattern: null, header_name: null, header_pattern: null, diff --git a/tests/unit/services/upstream-failure-rules.test.ts b/tests/unit/services/upstream-failure-rules.test.ts index 5561f899..b2d39f43 100644 --- a/tests/unit/services/upstream-failure-rules.test.ts +++ b/tests/unit/services/upstream-failure-rules.test.ts @@ -424,4 +424,45 @@ describe("upstream-failure-rules", () => { }) ).rejects.toBeInstanceOf(InvalidFailureRuleError); }); + + it("rejects rules containing unknown FailoverErrorType values", async () => { + await expect( + createFailureRule({ + name: "Unknown error type", + match: { errorTypes: ["http_500", "timeout"] }, + }) + ).rejects.toThrow(/Unknown error types: http_500/); + + await expect( + updateFailureRule("rule-1", { + match: { errorTypes: ["definitely_not_a_real_type"] }, + }) + ).rejects.toBeInstanceOf(InvalidFailureRuleError); + }); + + it("tolerates legacy rules with unknown errorTypes when matching evidence", async () => { + mockFindFirst.mockResolvedValueOnce({ + id: "upstream-1", + failureRuleConfig: { useGlobalRules: true }, + }); + mockFindMany.mockResolvedValueOnce([ + makeRule({ + id: "legacy-rule", + match: { + statusCodes: null, + errorTypes: ["http_500", "timeout"], + bodyPattern: null, + headerName: null, + headerPattern: null, + }, + }), + ]); + + const result = await matchFailureRule({ + upstreamId: "upstream-1", + errorType: "timeout", + }); + + expect(result?.id).toBe("legacy-rule"); + }); });