From 86ba4f57a06c1194661ed3d97d8a7734c7d47ea5 Mon Sep 17 00:00:00 2001 From: JianFeeeee Date: Mon, 5 Oct 2026 10:27:04 +0800 Subject: [PATCH] =?UTF-8?q?fix(quota):=20=E5=8D=81=E4=B8=89=E4=B8=AA?= =?UTF-8?q?=E9=80=80=E9=81=BF=E4=B8=AD=E7=9A=84=E6=BA=90=E8=A2=AB=E6=B8=B2?= =?UTF-8?q?=E6=9F=93=E6=88=90=E3=80=8C=E6=9C=89=E6=95=B0=E6=8D=AE=E3=80=8D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 症状 生产 Quota 页面上 17 个源里 13 个显示「有数据」徽标 —— 而它们实际上什么都 没返回，只是刚刚被退避跳过。 ## 根因（三层，逐层都静默） **① 退避记录缺 `skipped` 标记** `due_sources` 与 `collectSource` 返回退避记录时没有 `skipped = true`，页面的 `filter(r => !r.skipped)` 就把它们当正常记录放进卡片。卡片渲染匹配已知字段， 一条只有 `reason` 的记录什么都匹配不上，于是落到兜底分支。 **② 兜底分支文案是「有数据」** 那是个断言 —— 它在说「探测有应答」。落到这里的记录其实是：一条退避通知、 一种我们不认识的错误形状、或供应商改了响应格式。把「没有证据」说成「有数据」， 十三个死上游就被渲染成健康的了。改成「未探测」。 **③ 退避与「没有余额接口」被混为一谈** 页脚原本统一说「未暴露余额接口」。这是对上游的另一种断言，而且对死上游是错的 —— 它有接口，只是暂时没探到。 ## 「只改了一处」这个坑 `render()` 改成按 `backing_off` 标记分组之后，两个调用点都必须传**记录**而不是 名字。我先改了刷新路径就部署，结果是页脚显示「未列出 13 个源：、、、……」—— 症状看起来像渲染 bug，实际是两个调用点与同一个函数之间的契约不一致。 快照路径那行 `.map(function(r){ return r.name; })` 是后来才补的。 判据 `TestQuotaRenderCallSitesPassRecordsNotNames` 现在要求两处都在。 ## 顺带修掉的两个真 bug **空 Lua 表在 Go 侧解码成 `{}` 而不是 `[]`** `plan.backing_off || []` 里 `{}` 是 truthy，空表会原样进入 `out.concat()` 并抛错， 整次刷新报失败。改用 `Array.isArray` —— 它与插件实际返回的类型匹配。 **分组靠文案匹配是个定时炸弹** 原先用 `reason.indexOf("backing off") === 0` 判断退避。理由字符串一旦本地化， 退避就会被并进「未暴露余额接口」。改用显式 `backing_off: true` 标记， 判据里也改成按标记匹配 —— 变异把文案改回英文立刻变红。 ## 判据 新增两条： - `TestQuotaBackoffRecordIsSkippedAndFlagged` —— 退避记录必须带 `skipped` 和 `backing_off` 标记，且必须有 reason（否则卡片是空的） - `TestQuotaRenderCallSitesPassRecordsNotNames` —— 两个调用点都不得把跳过列表 映射成名字 变异自证四次全部变红：去掉 `skipped`、去掉 `backing_off`、文案改回英文、 快照路径改回 map-to-name。 ## 生产实测（浏览器真实点击） ``` 第1轮 1694ms: 4 个源 · 0 个读取失败 13 个源正在退避（连续探测失败，等下次重试）：trae、qijiar、toter、… 第2轮 2488ms: 4 个源 · 0 个读取失败 13 个源正在退避（…）：trae、qijiar、toter、… ``` 页面现在只列 4 个真有数据的源，13 个退避中的归到页脚并说明原因。 Co-Authored-By: ModelRouter --- internal/lua/plugins/quota.lua | 73 ++++++++++++++++----- internal/lua/quota_backoff_test.go | 100 ++++++++++++++++++++++++++++- 2 files changed, 155 insertions(+), 18 deletions(-) diff --git a/internal/lua/plugins/quota.lua b/internal/lua/plugins/quota.lua index 7148279..191d62c 100644 --- a/internal/lua/plugins/quota.lua +++ b/internal/lua/plugins/quota.lua @@ -389,8 +389,8 @@ local function collectSource(src, budget) -- A streak is a host that has failed repeatedly. Skip it outright until its -- backoff expires, instead of re-paying three adapter timeouts every TTL. if backoffSeconds(seen, known.streak) then - return { name = src.name, host = host, skipped = true, - reason = "backing off after " .. tostring(known.streak) .. " failed probes" } + return { name = src.name, host = host, skipped = true, backing_off = true, + reason = "连续探测失败 " .. tostring(known.streak) .. " 次，退避中（等待下次重试）" } end if PROBE_MEMO_TTL > 0 and (os.time() - seen) > PROBE_MEMO_TTL then known = nil @@ -577,8 +577,12 @@ PLUGIN.due_sources = function(payload) local base = (s.base_url:gsub("/+$", "")) local known = HOST_MEMO[hostOf(base)] if type(known) == "table" and backoffSeconds(known.at or 0, known.streak) then - backing[#backing + 1] = { name = s.name, host = hostOf(base), - reason = "backing off after " .. tostring(known.streak) .. " failed probes" } + -- skipped matters: the page filters on it. Without the flag a backed-off + -- host reaches the card renderer, which has no field to match and falls + -- through to its "has data" default — so thirteen dead upstreams were + -- displayed as if they had answered. + backing[#backing + 1] = { name = s.name, host = hostOf(base), skipped = true, backing_off = true, + reason = "连续探测失败 " .. tostring(known.streak) .. " 次，退避中（等待下次重试）" } else due[#due + 1] = s end @@ -824,7 +828,14 @@ PLUGIN.ui = { } if(r.hard_limit!==undefined && r.hard_limit!==null) return {cls:"ok", pill:'按量计费'}; - return {cls:"", pill:'有数据'}; + // Nothing here matched. Say so. + // + // The old default was a neutral "有数据", which is a claim: it says the probe + // answered. A record that reached this point answered with no field the page + // knows — a back-off notice, an error shape we do not recognise, or a vendor + // that added a new response. Calling that "has data" is how thirteen dead + // upstreams ended up rendered as healthy. Absence of evidence is not evidence. + return {cls:"unknown", pill:'未探测'}; } function card(r){ var st=state(r); @@ -837,6 +848,13 @@ PLUGIN.ui = { h+='
'+escH(r.error)+'
'; return h; } + // A record with nothing but a reason has no balance, no month, no windows — + // the card renders as a bare header with an empty body. Say why instead. + if(r.reason && !r.error && (r.balance===undefined||r.balance===null) + && (r.hard_limit===undefined||r.hard_limit===null)){ + h+='
'+escH(r.reason)+'
'; + return h+''; + } h+=balLine(r); var rows=""; rows+=month(r); @@ -861,7 +879,10 @@ PLUGIN.ui = { var rs = (st && st.reports) || []; if(!rs.length) return false; var real = rs.filter(function(r){ return r && !r.skipped; }); - var skipped = rs.filter(function(r){ return r && r.skipped; }).map(function(r){ return r.name; }); + // Records, not names — same reason as the refresh path. Both call sites were + // mapping to names while render() was changed to group on the flag; fixing + // one and shipping it is what produced a footer with thirteen blank entries. + var skipped = rs.filter(function(r){ return r && r.skipped; }); render(real, skipped); var sub=document.getElementById("qa-sub"); if(sub && st.updated_at){ @@ -878,10 +899,22 @@ PLUGIN.ui = { host.innerHTML='
没有源提供可读的余额接口
'; } else { host.innerHTML=real.map(card).join(""); - if(skipped && skipped.length){ - host.innerHTML+='
未列出 '+skipped.length+ - ' 个源（未暴露余额接口）：'+escH(skipped.join("、"))+'
'; - } + if(skipped && skipped.length){ + // Two reasons a source has no card, and they are not interchangeable. + // "This upstream exposes no balance endpoint" and "this upstream is + // backing off after failing" are different facts; collapsing them into + // one list tells the operator their dead host simply has no dashboard. + var backing = skipped.filter(function(r){ return r.backing_off===true; }); + var silent = skipped.filter(function(r){ return r.backing_off!==true; }); + if(backing.length){ + host.innerHTML+='
'+backing.length+ + ' 个源正在退避（连续探测失败，等下次重试）：'+escH(backing.map(function(r){return r.name;}).join("、"))+'
'; + } + if(silent.length){ + host.innerHTML+='
未列出 '+silent.length+ + ' 个源（未暴露余额接口）：'+escH(silent.map(function(r){return r.name;}).join("、"))+'
'; + } + } } var sub=document.getElementById("qa-sub"); if(sub) sub.textContent = real.length+" 个源 · "+bad.length+" 个读取失败"; @@ -924,8 +957,12 @@ PLUGIN.ui = { // re-probed on every refresh forever. Measured before this split: fanout left // the memo at 0 entries and every refresh cost the full 12s. var plan = await pluginAPI.call("quota","due_sources",{sources:srcs}); - var due = (plan && plan.due) || []; - var out = (plan && plan.backing_off) || []; + // An EMPTY Lua table decodes as {} on the Go side, not [] — and {} is truthy, + // so `|| []` does not replace it. An empty object reaching out.concat() + // throws, and the whole refresh reports a failure. Array.isArray is the check + // that matches what the plugin actually returns. + var due = (plan && Array.isArray(plan.due)) ? plan.due : []; + var out = (plan && Array.isArray(plan.backing_off)) ? plan.backing_off : []; if (due.length) { out = out.concat(await pluginAPI.fanout("quota","collect_source",due.map(function(s){ @@ -946,8 +983,10 @@ PLUGIN.ui = { await pluginAPI.call("quota","memo_record",{results:(out||[]).map(function(r){ if(!r) return null; // A host that is merely BACKING OFF has not just failed; recording it as a - // failure would grow the streak on a probe that never happened. - if(r.reason && String(r.reason).indexOf("backing off")===0) return null; + // failure would grow the streak on a probe that never happened. Keyed on the + // flag rather than the wording, so translating the message cannot silently + // turn every skipped host into an ever-growing streak. + if(r.backing_off===true) return null; if(r.error) return {host:r.host}; if(r.skipped) return {host:r.host}; return {host:r.host, vendor:r.vendor}; @@ -959,7 +998,11 @@ PLUGIN.ui = { } var all=out||[]; var real=all.filter(function(r){return !r.skipped;}); - var skipped=all.filter(function(r){return r.skipped;}).map(function(r){return r.name;}); + // The records, not their names — render() groups skipped sources by the + // backing_off flag, which a bare name cannot carry. This call site was left + // mapping to names while the other was fixed, and the mismatch showed up as + // a footer listing thirteen unnamed sources. + var skipped=all.filter(function(r){return r.skipped;}); // real may already contain errors; collect_all keeps them for visibility. render(real, skipped); if(btn) btn.disabled=false; diff --git a/internal/lua/quota_backoff_test.go b/internal/lua/quota_backoff_test.go index c26d34c..dc252b4 100644 --- a/internal/lua/quota_backoff_test.go +++ b/internal/lua/quota_backoff_test.go @@ -27,9 +27,16 @@ func TestQuotaBackoffIsAppliedInSource(t *testing.T) { t.Error("quota.lua never consults backoffSeconds for a host with a streak — " + "the backoff is computed but not applied") } - if !strings.Contains(src, `reason = "backing off after "`) { - t.Error("the skip path does not say why it skipped; a silently skipped host " + - "looks exactly like a host that simply has no balance endpoint") + // The reason is required, but matched on the FLAG rather than the wording: + // the message is localised, and grouping by text would merge "backing off" + // into "exposes no balance endpoint" the moment someone translates it. + if strings.Count(src, "reason = \"连续探测失败 ") < 2 { + t.Error("both skip paths (collectSource and due_sources) must carry a reason; " + + "a silently skipped host looks exactly like a host with no balance endpoint") + } + if strings.Count(src, "backing_off = true") < 2 { + t.Error("both skip paths must set the explicit backing_off flag; the page " + + "groups skipped sources by it, not by the message text") } // The recording guard has to REPLACE the memo, not merge into it. The bug this // guards was `if rep == nil and not HOST_MEMO[host]`, which is true only on a @@ -554,3 +561,90 @@ func hostOnly(baseURL string) string { } return u } + +// TestQuotaBackoffRecordIsSkippedAndFlagged pins the shape the page depends on. +// +// Two things went wrong in the same field, and both were silent: +// +// 1. The back-off record had no `skipped` flag, so the page's +// `filter(r => !r.skipped)` let it through to the card renderer. That +// renderer matches on known fields, and a record carrying only a reason +// matches none — so it fell through to a default that said "有数据". +// Thirteen dead upstreams were rendered as though they had answered. +// 2. Once it did carry `skipped`, the page grouped skipped sources by reading +// the REASON TEXT. Translating that string would have silently merged +// "backing off" into "exposes no balance endpoint" — a different claim +// about the upstream. The grouping keys on an explicit flag. +func TestQuotaBackoffRecordIsSkippedAndFlagged(t *testing.T) { + srv := newCountingServer(t) + ps := newQuotaPlugins(t, srv.URL) + + // One failure, enough to put the host inside its window. + if _, err := ps.Call("quota", "memo_record", map[string]interface{}{ + "results": []interface{}{map[string]interface{}{"host": srv.URL + "/v1"}}, + }); err != nil { + t.Fatal(err) + } + + out, err := ps.Call("quota", "due_sources", map[string]interface{}{ + "sources": []interface{}{map[string]interface{}{ + "name": "dead", "base_url": srv.URL + "/v1", "api_key": "k", + }}, + }) + if err != nil { + t.Fatal(err) + } + m, _ := out.(map[string]interface{}) + backing, _ := m["backing_off"].([]interface{}) + if len(backing) != 1 { + t.Fatalf("backing_off = %#v, want one record", m["backing_off"]) + } + rec, _ := backing[0].(map[string]interface{}) + if rec["skipped"] != true { + t.Errorf("back-off record has skipped=%v — the page filters cards on this "+ + "field, and without it the record is rendered as a source with data", + rec["skipped"]) + } + if rec["backing_off"] != true { + t.Errorf("back-off record has no explicit backing_off flag; the page groups " + + "by it instead of by reason text, so translating the message cannot " + + "merge this with 'no balance endpoint'") + } + if s, _ := rec["reason"].(string); s == "" { + t.Error("back-off record carries no reason; the card renders empty") + } +} + +// TestQuotaRenderCallSitesPassRecordsNotNames guards a bug that only appeared +// because ONE of two identical call sites was fixed. +// +// render() groups skipped sources by the backing_off flag. A call site that maps +// the records to their names first ("skipped.map(r => r.name)") therefore hands +// render an array of strings: the flag is gone, every entry falls into the +// "exposes no balance endpoint" bucket, and the footer lists N unnamed sources. +// +// The first fix changed only the refresh path. The snapshot path kept mapping to +// names, and the symptom was a footer reading "未列出 13 个源：、、、、…" — which +// reads like a rendering bug rather than a contract mismatch between two call +// sites of one function. +func TestQuotaRenderCallSitesPassRecordsNotNames(t *testing.T) { + src := readShippedPlugin(t, "quota.lua") + if strings.Contains(src, ".map(function(r){ return r.name; })") || + strings.Contains(src, ".map(function(r){return r.name;})") { + // A benign occurrence would be fine, but there is no other reason for a + // skipped list to be reduced to names, so treat any as the bug. + for _, line := range strings.Split(src, "\n") { + if strings.Contains(line, "skipped") && strings.Contains(line, ".map(") && + strings.Contains(line, "r.name") { + t.Errorf("a skipped list is mapped to names before reaching render(): %s\n"+ + "render() groups on the backing_off flag, which a name cannot carry", + strings.TrimSpace(line)) + } + } + } + // Both call sites must exist and both must hand over records. + if n := strings.Count(src, ".filter(function(r){return r.skipped;})") + + strings.Count(src, ".filter(function(r){ return r && r.skipped; })"); n < 2 { + t.Errorf("found %d skipped-list filters, want 2 (snapshot and refresh paths)", n) + } +}