fix(network): 缓存重复的 AAAA 查询 - #101
Conversation
|
Warning Review limit reached
Next review available in: 21 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughComputerManager 新增 AAAA 正负缓存、并发查询合并和网络切换失效。轮询流程复用解析结果,并按地址类型更新 IPv6 地址。 ChangesAAAA DNS 与 IPv6 地址处理
Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@entry/src/main/ets/service/ComputerManager.ets`:
- Around line 1120-1124: Update the IPv6-change branch in lookupAndSetIpv6 to
call notifyListChanged() after assigning computer.ipv6Address and saving the
updated computers, ensuring the UI refreshes immediately while preserving the
existing save behavior.
- Around line 562-580: Update the DNS lookup flow around
queryAndCacheAaaaLiteral to pass the normalized cacheKey, rather than the raw
host value, so queries and cached results consistently use the trimmed,
lowercased hostname.
- Around line 1105-1113: Update the lookup source construction in the hostname
handling block around activeAddress so `${hostname}.local` is added only when
hostname does not already end with `.local`; preserve adding the original
hostname and the existing LAN-only condition.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 02a56f33-cf7a-4152-8d0f-a8a2c27d4ce3
📒 Files selected for processing (1)
entry/src/main/ets/service/ComputerManager.ets
| // Sunshine 自报 hostname/.local 通常只在 LAN 有意义,禁止从公网轮询链路泄漏查询。 | ||
| // 手动输入的 DDNS 始终保留,因为它代表用户明确配置的公网身份。 | ||
| const activeAddress = computer.address || computer.localAddress; | ||
| if (hostname && isLanAddress(activeAddress)) { | ||
| lookupSources.push(hostname, `${hostname}.local`); | ||
| } | ||
| if (lookupSources.length === 0) return; | ||
|
|
||
| const uniqueLookupSources = Array.from(new Set<string>(lookupSources)); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
避免对已有 .local 后缀重复拼接。
如果 hostname 已是 host.local,Line 1109 会加入 host.local.local。去重无法消除这个不同字符串。首次查询无 AAAA 记录时,轮询会产生额外 DNS 查询。
建议修改
- if (hostname && isLanAddress(activeAddress)) {
- lookupSources.push(hostname, `${hostname}.local`);
+ const lanHostname = hostname?.trim().replace(/\.$/, '');
+ if (lanHostname && isLanAddress(activeAddress)) {
+ lookupSources.push(lanHostname);
+ if (!lanHostname.toLowerCase().endsWith('.local')) {
+ lookupSources.push(`${lanHostname}.local`);
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Sunshine 自报 hostname/.local 通常只在 LAN 有意义,禁止从公网轮询链路泄漏查询。 | |
| // 手动输入的 DDNS 始终保留,因为它代表用户明确配置的公网身份。 | |
| const activeAddress = computer.address || computer.localAddress; | |
| if (hostname && isLanAddress(activeAddress)) { | |
| lookupSources.push(hostname, `${hostname}.local`); | |
| } | |
| if (lookupSources.length === 0) return; | |
| const uniqueLookupSources = Array.from(new Set<string>(lookupSources)); | |
| // Sunshine 自报 hostname/.local 通常只在 LAN 有意义,禁止从公网轮询链路泄漏查询。 | |
| // 手动输入的 DDNS 始终保留,因为它代表用户明确配置的公网身份。 | |
| const activeAddress = computer.address || computer.localAddress; | |
| const lanHostname = hostname?.trim().replace(/\.$/, ''); | |
| if (lanHostname && isLanAddress(activeAddress)) { | |
| lookupSources.push(lanHostname); | |
| if (!lanHostname.toLowerCase().endsWith('.local')) { | |
| lookupSources.push(`${lanHostname}.local`); | |
| } | |
| } | |
| if (lookupSources.length === 0) return; | |
| const uniqueLookupSources = Array.from(new Set<string>(lookupSources)); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@entry/src/main/ets/service/ComputerManager.ets` around lines 1105 - 1113,
Update the lookup source construction in the hostname handling block around
activeAddress so `${hostname}.local` is added only when hostname does not
already end with `.local`; preserve adding the original hostname and the
existing LAN-only condition.
qiin2333
left a comment
There was a problem hiding this comment.
方向认可,实现里 single-flight、epoch 防旧网结果回写、finally 里比对 promise 再删 flight 这几个并发细节都处理对了。合并前建议看下面几点。
1. 负缓存会污染手动添加路径(建议合并前修)
addComputer 的 AAAA 回退(ComputerManager.ets:509)也走 resolveAaaaLiteral。用户手输域名添加失败后,60 秒内重试会直接命中负缓存、根本不发 DNS,于是再次失败——用户会觉得“重试没用”。
建议给 resolveAaaaLiteral 加 forceRefresh?: boolean,手动添加/手动刷新时跳过缓存读取(结果仍然写入缓存):
private async resolveAaaaLiteral(host: string, forceRefresh = false): Promise<string | undefined> {
// ...
const cached = forceRefresh ? undefined : this.aaaaCache.get(cacheKey);2. 去掉 !target.ipv6Address 守卫是超出标题的行为变更
原来拿到 IPv6 就永不再查,现在改成按 TTL 每 5 分钟重查一轮。为 DDNS 刷新说得通,但这是“新增周期性 DNS”而非“缓存重复查询”,和 PR 标题不太一致。
另外另外三个调用点(:242、:591、:618)仍保留 if (!ipv6Address) 守卫,改完之后语义不统一了。建议要么一并调整,要么在注释里写明为什么只有轮询链路需要周期刷新。
3. LAN 门控的判据和注释不符
isLanAddress 只认 private IPv4 / link-local / ULA / site-local,全局单播 IPv6(GUA)不算 LAN。所以局域网内通过 GUA 连上的主机,从此不再查 hostname/.local。实际影响有限(这类主机本来就有 IPv6),但注释写的“仅在 LAN 场景尝试”和代码行为对不上。
更值得注意的是 computer.address || computer.localAddress 这个回退方向反了:localAddress 是 Sunshine 自报的内网 IP,公网连接时它同样是内网 IP(:333 无条件写入)。一旦 address 为空就会把公网主机误判成 LAN,正好绕开了这次想加的门控。建议只用 computer.address,为空时显式决定放行还是跳过。
4. 缓存失效只挂在 netAvailable / netLost
目前只注册了这两个事件。Wi-Fi 切 Wi-Fi、VPN 起落、SLAAC 前缀重新编址都可能不触发它们,最坏会留 5 分钟的陈旧 IPv6。不是阻塞项,可以考虑补 netCapabilitiesChange,或把正缓存 TTL 降到 2 分钟。
小结:核心逻辑正确,没发现并发缺陷。第 1 条建议合并前处理,第 2、3 条至少把注释和代码行为对齐。
(另:CodeRabbit 摘要里的“直连失败后支持通过 IPv6 重试”是 base 已有能力,不是本 PR 新增。)
改了啥呀
.local仅在 LAN 场景尝试为啥要改
此前只要主机还没有全局 IPv6,每次 5 秒状态轮询成功后都会重新查询
manualAddress → hostname → hostname.local。A-only 域名、无 AAAA 记录或公网不可解析的局域网 hostname 会持续制造 DNS 请求;多个刷新入口还可能并发重复查询。AAAA-only 域名的 ETS 回退仍然有用,问题只是这个小杂鱼回退被放进了无限轮询生命周期里。这次保留兼容能力,同时给它补上缓存、并发合并和网络边界。
验证
npm run checkC:\Program Files\Huawei\DevEco Studio\tools\node\node.exe C:\Program Files\Huawei\DevEco Studio\tools\hvigor\bin\hvigorw.js assembleApp --mode project -p product=default -p buildMode=debug --no-daemonSummary by CodeRabbit