Skip to content

[PR] fix: bracket IPv6 addresses in port-holder health probes - #6182

Open
Afloat16 wants to merge 1 commit into
ranxianglei:masterfrom
Afloat16:2026-11-10_ipv6-holder-probe
Open

Afloat16 wants to merge 1 commit into
ranxianglei:masterfrom
Afloat16:2026-11-10_ipv6-holder-probe

Conversation

@Afloat16

Copy link
Copy Markdown
Contributor

Fixes #6174

Problem

holderProbeOrigin concatenates a bind hostname and port without bracketing IPv6 literals. For ::1 it produces http://::1:PORT, which fetch cannot parse. The failed health probe then labels a healthy listener as wedged/dead and advises killing it during duplicate starts or terminal EADDRINUSE handling.

Changes

Bracket raw IPv6 literals when constructing the probe authority, while preserving IPv4, DNS names, already bracketed literals, and existing wildcard-to-loopback behavior.

Validation

  • Rebased onto master e3220adc0f5412af83163666ca70a6afa05e3247 (package version 0.1.213). The intervening [PR] thirdparty-scan: flag dsh-live-compact as a known conflict (#5984) #6166 changes only third-party plugin detection, its tests, announcements and configuration docs; none of this patch's source or test files changed. The complete patch is byte-identical to the locally validated patch on 33104b1f9545caeb0344193009917ceef9969682 and applies cleanly.
  • Negative control on 33104b1f9545caeb0344193009917ceef9969682: 2 new regression cases still fail before the production fix.
  • Independent source tree based on 33104b1f9545caeb0344193009917ceef9969682 (a0ed591736daa50b33b938f817a6b3856dc5b114): 29/29 focused tests passed, with no failures, skips or cancellations.
  • Full project TypeScript checking and the standard npm run build passed on the temporary integration tree e186774d3428c85e2a4974c6e609b2a7381d6153 containing the seven independently submitted fixes on 33104b1f9545caeb0344193009917ceef9969682. The repository compiler options and include set are unchanged.
  • git diff --check passed. Local runtime: Node.js 24.22.1 on Linux. Full cross-platform remote CI remains pending.

This is the local port-holder diagnostic path introduced by #4784. It is separate from upstream proxy socket hostname normalization in #6092. No configuration, kernel, or inference-wire changes.

@ranxianglei

Copy link
Copy Markdown
Owner

🤖 Powered by ework · qwen3.8-27b

(ai: vllm-qwen/qwen3.8-27b)

查重: 不重复。 Searched open+closed issues/PRs for this symptom/mechanism: #6182 is the only fix for #6174, no parallel fix in flight. Other open port-owner work (#4622 responder identity, #5064 sticky zone pointer, #5833 orphan reaping, #5482 diagnostics) covers different mechanisms; #4784 is the closed PR that introduced this diagnostic path.

标签: bug + p2 + scope/other applied to #6174 and this thread — defect in the startup/port-diagnostics path (#4784 family); usable overall but clear misdiagnosis under IPv6-loopback bind; matches my independent severity read (not unavailability, not cache-leak).

Review — verified live against head 9af4b7f56c (branch sits directly on current master e3220adc0f, no rebase needed):

Direction (①). The reported bug is real and confirmed at the mechanism level: holderProbeOrigin (src/port-owner.ts:228) built http://::1:PORT; I verified locally that new URL("http://::1:8080") throws Invalid URL while the bracketed form parses. Since probeOwnerInfo maps every fetch failure to "no evidence", the unparseable URL funnels into serving=false and holderHintFragment(false, …) — "wedged or dead … kill it" — at both call sites src/server.ts:734 and src/server.ts:1681, i.e. exactly the false-kill advice in the issue. The patch lands at the producer of the bad bytes (the concatenation), so it is a root-cause fix, not a mask. It also mirrors the identical bracketing expression already merged for the same class at src/server.ts:1277 (#6092).

Code (③). One-line guard: bracket when the host contains : and does not start with [. Any valid DNS name or IPv4 literal contains no colon, so the test isolates raw IPv6 literals precisely; wildcard mapping (0.0.0.0/:: → 127.0.0.1) runs first and is untouched; already-bracketed input passes through without double-bracketing. No new dependencies, no config/wire/persistence/kernel surface.

Tests (④). Both new cases have teeth. The pure case asserts exact output for ::1, a GUA, already-bracketed input (negative row), and localhost (negative row). The live case stands up a real listener on ::1, drives it through probeHolderForHint, and asserts the hint says "answered /__bili/health (pid …)" and NOT /wedged or dead|kill it/ — pre-fix this round trip yields the wedged wording, so the case pins the regression (consistent with your negative-control claim). undici is a declared devDependency (^7.29.0), ports come from listen(0) (no fixed-port luck), and cleanup closes both dispatcher and server.

Regression inventory (②). Every holderProbeOrigin caller checked (only src/server.ts:734/1681) plus all pre-existing test rows (wildcard rows byte-identical). Sole behavior delta: raw-IPv6-literal binds change from "probe always fails → false wedged/dead + kill advice" to "genuine probe result". That delta IS the fix and is disclosed in issue+PR body. IPv4/DNS/wildcard/bracketed inputs produce byte-identical URLs to before. No fail-fast→swallowing shift, no threshold/timing/format drift.

CI (authoritative per §7.2, read from mirror cache on head sha): 12/12 test shards green (ubuntu+windows × node 22/24), gate ×3, version-guard, static-gates ×2, mysql-contract all success; the 12 skipped items are env-gated optional e2e lanes (standard). I did not re-run the suite locally — CI on the pushed head is the verdict.

Diff & quality bar (⑤/⑤b): 2 files, +36/−1, single fix: commit, every line on-purpose, no whitespace churn, style consistent with the file. External-submission checklist passes on all items — no rework needed, nothing pushed to the branch.

Adjacent observation (pre-existing, out of scope here, noted instead of opened per platform rule): the same unbracketed host+port concatenation still exists at src/server/admin.ts:323, src/server/admin.ts:552, src/server/admin.ts:805 (web/plugin origins echoed back to hosts), src/launcher.ts:389 proxyOrigin() feeding functional fetches at src/launcher.ts:6052/6056/6149/6308, and src/startup-failure-page.ts:54 (display). An explicit --host ::1 bind produces unparseable origins there too — same mechanism family as #6174 but different consumer/symptom surfaces, so I deliberately did not widen this minimal PR onto those hot files. Owner can decide whether to sweep them in a follow-up.


【结构化八问】

  1. 是什么问题:bili 绑定 IPv6 地址(如 ::1)时,端口占用者健康探测的 URL 被拼成 http://::1:PORT(缺方括号)、无法解析,健康监听者被误诊为 wedged/dead,并给出让用户 kill 它的错误建议。
  2. 什么原因导致的:holderProbeOrigin(src/port-owner.ts:228)拼接 bind host 与端口时未对方括号包裹 IPv6 字面量(坏字节的产生点);叠加 probeOwnerInfo 把一切探测失败统一解读为「未服务」,失败即误报。
  3. 影响范围与严重程度:仅显式绑定 IPv6 地址的用户触发;默认绑定(0.0.0.0/127.0.0.1)完全不受影响。服务本身不受影响,但诊断文案错误且给出错误的 kill 建议(照做会杀掉健康实例)→ p2。
  4. 怎么修复的:在 URL 构造这一决策点加一行守卫——host 含 : 且不以 [ 开头则补方括号;与 [PR] fix: separate IPv6 socket hosts from CONNECT authorities #6092 已在 src/server.ts:1277 落地的同类实现同一表达式,修在根因处而非下游掩盖。
  5. 是否有回归问题:已清点全部调用点(src/server.ts:734、1681)与既有测试行——IPv4/DNS/通配符/已带括号输入的输出与修复前字节级一致;唯一行为变化是「IPv6 字面量绑定从恒假阳变为真实探测结果」,属本 PR 目标修复且已在 issue/PR 披露旧值→新值。无 fail-fast 变吞错、无阈值/时序/格式漂移。
  6. 是否破坏缓存:不涉及——未触碰任何缓存/重试/游标/去重逻辑。
  7. 是否改了设计方向:不涉及——纯本地诊断路径修复,无 kernel/配置/线协议变更。
  8. 是否引入了新的配置:无。

判定:可合入。 CI 在 head 9af4b7f56c 全绿,diff 干净,满足 §7.4 auto-merge 各项条件;合并动作归 owner,请自行 merge: #6182

@ranxianglei

Copy link
Copy Markdown
Owner

🤖 Powered by ework · qwen3.8-27b

(ai: vllm-qwen/qwen3.8-27b)

Status (per system nudge asking for an outcome post): the review above (previous comment) is the complete verdict — nothing pending on my side. Verified live just now:

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working p2 scope/other

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] IPv6 holder health URL falsely reports a healthy listener as wedged

2 participants