feat(streaming): add pre-stream bandwidth probing - #99
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough本次变更新增网络路径识别、Sunshine v1 HTTPS 二进制带宽探测、探测设置与缓存,并将测量结果接入串流初始码率和 ABR 上限控制。 Changes带宽预探测
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 1
🤖 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/network/BandwidthProbeService.ets`:
- Around line 175-182: 在 BandwidthProbeService.ets
的采样循环中为计费网络增加明确的总字节预算,按实际选中的样本大小累计流量,超出预算时跳过或停止后续探测,而不仅限制 requestedSamples。同步更新
SettingsPageV2.ets 中对应设置说明,显示实施后的实际最大探测流量。
🪄 Autofix (Beta)
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: 28defd91-f7f6-40f5-9809-3b5e0386076b
📒 Files selected for processing (13)
ci/sdk-stubs/kit.NetworkKit.d.tsdocs/SUNSHINE_BANDWIDTH_PROBE_API.mdentry/src/main/ets/pages/AppListPageV2.etsentry/src/main/ets/pages/SettingsPageV2.etsentry/src/main/ets/service/SettingsService.etsentry/src/main/ets/service/network/BandwidthProbeService.etsentry/src/main/ets/service/network/ConnectionPathService.etsentry/src/main/ets/service/streaming/AdaptiveBitrateService.etsentry/src/main/ets/service/streaming/NvHttp.etsentry/src/main/ets/service/streaming/StreamingSession.etsentry/src/main/ets/utils/HttpClient.etsentry/src/main/ets/utils/NetworkErrorClassifierSelfCheck.etsentry/src/main/ets/utils/RcpSessionPool.ets
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
scripts/sunshine-mock-connection-test.js (5)
917-920: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win把
assert移出策略模型。
BandwidthProbePolicyModel.executeProbe是被测模型,不应包含测试断言。Line 920 的assert.strictEqual(data.length, bytes)失败时会抛出,被 Line 942 的catch捕获,再按aborted状态决定是否 rethrow。这条路径把断言失败伪装成探测失败。若此时恰好已 abort,断言失败会被静默吞掉。建议在模型内改为显式的长度校验并抛出普通
Error,把assert留在测试函数中。♻️ 建议修改
const durationMs = Math.max(1, Date.now() - sampleStartedAt); - assert.strictEqual(data.length, bytes); + if (data.length !== bytes) { + throw new Error(`probe sample length mismatch: expected ${bytes}, got ${data.length}`); + }🤖 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 `@scripts/sunshine-mock-connection-test.js` around lines 917 - 920, 在 BandwidthProbePolicyModel.executeProbe 中移除对 assert.strictEqual 的依赖,改用显式检查 data.length,并在长度不等于 bytes 时抛出普通 Error;保留测试函数中的断言,使长度校验失败不会被 executeProbe 的 catch 按 aborted 状态吞掉。
727-733: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win补上
!probe.endpoint判空,与NvHttp契约对齐。
NvHttp.getNetworkProbeCapabilities(entry/src/main/ets/service/streaming/NvHttp.ets第 626 行附近)先判断!probe.endpoint再做格式校验。本模型缺少该判空。若服务端返回bandwidthProbe但缺少endpoint,probe.endpoint.startsWith('/')会抛TypeError,模型返回异常而不是null。这会让模型与生产的"不支持则静默回退"行为出现偏差。♻️ 建议修改
if (parsed.version < 1 || !parsed.features || !parsed.features.includes('bandwidth-probe-v1') || - !probe || probe.version !== 1 || !probe.endpoint.startsWith('/') || probe.endpoint.includes('://') || + !probe || probe.version !== 1 || !probe.endpoint || !probe.endpoint.startsWith('/') || + probe.endpoint.includes('://') || probe.endpoint.includes('?') || probe.endpoint.includes('#') || probe.minBytes < 1 || probe.maxBytes < probe.minBytes || probe.cooldownMs < 0) { return null; }🤖 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 `@scripts/sunshine-mock-connection-test.js` around lines 727 - 733, Update the validation condition in the probe capability parsing flow to reject a missing or falsy probe.endpoint before calling startsWith or other endpoint checks. Keep the existing invalid-capability behavior of returning null so responses without an endpoint silently fall back consistently with NvHttp.getNetworkProbeCapabilities.
1278-1281: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win补充 nonce 回显断言。
mock 在 Line 659 设置了
X-Bandwidth-Probe-Nonce响应头,但downloadNetworkProbe和本测试都不校验它。nonce 回显是防止缓存或代理返回陈旧响应的关键契约,docs/NETWORK_SMOKE_TEST_CASES.md的 NS-A08 也声称覆盖"二进制响应"契约。建议让
downloadNetworkProbe返回响应头,并在此断言X-Bandwidth-Probe-Version为'1'、X-Bandwidth-Probe-Nonce等于请求的 nonce。🤖 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 `@scripts/sunshine-mock-connection-test.js` around lines 1278 - 1281, Update downloadNetworkProbe to expose the response headers alongside the binary body, then extend the protocol smoke test to assert X-Bandwidth-Probe-Version equals '1' and X-Bandwidth-Probe-Nonce matches the nonce sent in the request. Preserve the existing body and capability-request assertions.
1352-1373: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuemock 的挂起定时器会延长测试进程。
capabilityDelayMs = 1500,但waitForLaunchProbe在 800 ms 时取消探测。mock.stop()返回后,handleRequest中 Line 510 创建的setTimeout仍在事件循环中挂起约 700 ms。res.destroyed检查保证不会写入已关闭的响应,但定时器本身会阻止事件循环空转退出。建议在 mock 中对这两处
setTimeout调用unref(),或在stop()中统一clearTimeout。♻️ 建议修改(scripts/sunshine-mock-connection-test.js Line 510-512 与 530-532)
- setTimeout(() => { + const timer = setTimeout(() => { if (!res.destroyed) this.writeJson(res, capabilities); }, this.capabilityDelayMs); + timer.unref(); return;- setTimeout(() => { + const timer = setTimeout(() => { if (!res.destroyed) this.writeBinary(res, bytes, nonce); }, durationMs); + timer.unref(); return;🤖 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 `@scripts/sunshine-mock-connection-test.js` around lines 1352 - 1373, Update the delayed response timers in handleRequest, including both setTimeout calls near the capability and probe response paths, so they are unrefed or tracked and cleared by the mock’s stop method. Ensure mock.stop() no longer leaves pending timers that extend the test process, while preserving the existing response and res.destroyed behavior.
1307-1313: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win这两条断言与真实计时强耦合,在慢速环境下可能不稳定。
mock 的服务端延迟为
bytes * 8 / probeKbps。在probeKbps = 20000下,各样本的服务端延迟为 26 ms、105 ms、419 ms。模型在样本耗时>= 250 ms时 break,因此采样序列恰好是[65536, 262144, 1048576]。余量很小:
- Line 1310-1313:若第二个样本(262144)的实测耗时因调度或回环开销超过 250 ms,模型提前 break,序列变成
[65536, 262144],deepStrictEqual失败。余量约 145 ms。- Line 1308:
bestKbps由第三个样本决定,约 19500 kbps,safeBitrateKbps约 13600。若该样本实测耗时超过约 490 ms,结果跌破 12000 下界。余量约 70 ms。建议放宽为区间断言。例如断言序列前缀为
[65536, 262144]且长度不超过 4,断言bitrateKbps落在更宽的区间。也可以调低probeKbps以放大服务端延迟占比,降低系统开销的相对影响。🤖 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 `@scripts/sunshine-mock-connection-test.js` around lines 1307 - 1313, 放宽该测试中对真实计时敏感的断言:在决策结果断言处使用覆盖慢速环境的更宽 bitrateKbps 范围,并将 mock.requests 对 /api/network/probe 的精确序列断言改为验证至少包含 [65536, 262144] 前缀且总长度不超过 4。保留 /api/network/capabilities 仅请求一次的断言,并定位相关断言所在的测试逻辑。
🤖 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 `@docs/NETWORK_SMOKE_TEST_CASES.md`:
- Line 47: Update the regression-test scope in the “回归重点” section from
NS-A01–NS-A07 to NS-A01–NS-A08 so it includes the newly added NS-A08 case.
---
Nitpick comments:
In `@scripts/sunshine-mock-connection-test.js`:
- Around line 917-920: 在 BandwidthProbePolicyModel.executeProbe 中移除对
assert.strictEqual 的依赖,改用显式检查 data.length,并在长度不等于 bytes 时抛出普通
Error;保留测试函数中的断言,使长度校验失败不会被 executeProbe 的 catch 按 aborted 状态吞掉。
- Around line 727-733: Update the validation condition in the probe capability
parsing flow to reject a missing or falsy probe.endpoint before calling
startsWith or other endpoint checks. Keep the existing invalid-capability
behavior of returning null so responses without an endpoint silently fall back
consistently with NvHttp.getNetworkProbeCapabilities.
- Around line 1278-1281: Update downloadNetworkProbe to expose the response
headers alongside the binary body, then extend the protocol smoke test to assert
X-Bandwidth-Probe-Version equals '1' and X-Bandwidth-Probe-Nonce matches the
nonce sent in the request. Preserve the existing body and capability-request
assertions.
- Around line 1352-1373: Update the delayed response timers in handleRequest,
including both setTimeout calls near the capability and probe response paths, so
they are unrefed or tracked and cleared by the mock’s stop method. Ensure
mock.stop() no longer leaves pending timers that extend the test process, while
preserving the existing response and res.destroyed behavior.
- Around line 1307-1313: 放宽该测试中对真实计时敏感的断言:在决策结果断言处使用覆盖慢速环境的更宽 bitrateKbps 范围,并将
mock.requests 对 /api/network/probe 的精确序列断言改为验证至少包含 [65536, 262144] 前缀且总长度不超过
4。保留 /api/network/capabilities 仅请求一次的断言,并定位相关断言所在的测试逻辑。
🪄 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: 48cf1c28-f50c-4188-8584-af370c7e083c
📒 Files selected for processing (3)
docs/NETWORK_SMOKE_TEST_CASES.mddocs/SUNSHINE_BANDWIDTH_PROBE_API.mdscripts/sunshine-mock-connection-test.js
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/SUNSHINE_BANDWIDTH_PROBE_API.md
改了啥呀
为啥要改
只看 LAN/WAN 或私网地址就猜码率实在有点杂鱼:拥塞 Wi-Fi 可能比公网更差,VPN 还经常披着私网地址。现在让真实端到端探测负责初始值,网络分类只做它擅长的权限、流量预算和缓存隔离;旧 Sunshine、探测失败、超时和低置信度样本都会安静回退,不让开流一直等。
验证
git diff --checknpm run checknpm run test:sunshine-mock:协议、同路径任务合并与缓存、蜂窝/计费授权与 320 KiB 预算、800 ms 启动超时均PASSprobeApplied=false、/api/network/probe请求数为 0DEVECO_SDK_HOME=/Applications/DevEco-Studio.app/Contents/sdk JAVA_HOME=/Applications/DevEco-Studio.app/Contents/jbr/Contents/Home node hvigorw.js assembleApp --mode project -p product=default -p buildMode=debug --no-daemon:BUILD SUCCESSFULSummary by CodeRabbit
新功能
改进
测试与文档