fastbee/yack-mini/docs/CODE_REVIEW.md
2026-09-27 15:17:17 +08:00

308 lines
20 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# yack-mini 小程序 Code Review
> 评审范围:`yack-mini/miniprogram/**`(业务代码约 6.0k 行,不含第三方 `libs/mqtt-wx.min.js` 887 行)
> 评审日期:2026-09-27
> 结论:**架构分层清晰、BLE 状态机工程质量高**,主要债务集中在「页面层的脏边界」与「重构后未清理的死代码」。
## 一、总体评价
| 层 | 文件 | 评价 |
| --- | --- | --- |
| BLE 状态机 | `services/ble.js` (1213) | ⭐ 优秀。把微信 BLE 的三条硬约束(already connect / 并发建链 / 切设备作废)用 seq + 单 Promise 闸门建模,每条约束都有注释说明来源,超时兜底齐全。全项目最扎实的模块 |
| 采集网关 | `services/gateway.js` (872) | ⭐ 良好。串行队列 `_chain` + 状态快照订阅 + 收帧快照归属(测点/productId/reportSn)设计正确,注释解释了"为什么"而非"是什么" |
| MQTT 包装 | `services/mqtt-client.js` (491) | 良好。实例校验防幽灵客户端、建连去重、用户意图标记都到位 |
| 页面层 | `pages/**` | ⚠️ 一般。脏边界集中在此:参数无校验、高频 setData、过期注释 |
| 工具层 | `utils/**` | ⚠️ 一般。死函数多,且存在一处会损坏数据的编码缺陷 |
| 回归测试 | `tests/**` | 良好质量,但**覆盖缺口大**:只锁了"设备停用不连蓝牙"和"三个开关"两个点,主链路(收帧→上报→离线补传)零覆盖 |
---
## 二、P0 — 会导致功能错误或数据损坏,必修
> ✅ **以下 5 条已于 2026-09-27 全部修复**,回归用例见 `tests/p0-regressions.test.js`(45 断言,含反向验证:
> 回退成缺陷写法后 5 项转红)。改动文件:`pages/mine/mine.js`、`utils/storage.js`、`pages/device/scan/scan.js`、
> `utils/crypto.js`、`services/mqtt-client.js`。P1 及以下未动。
### P0-1 产品ID 用 `parseInt` 无整数校验,NaN 会写进上报 topic
- 位置:`pages/mine/mine.js:98`、`mine.js:103`
- 根因:`parseInt('12.5') === 12`(静默截断小数)、`parseInt('abc') === NaN`。`if (!productId)` 只挡空串,NaN 能一路穿到 `storage.setMqttConfig({ productId: NaN })`。后续 `gateway._resolveProductId()` 的判空是 `cfg.productId == null || cfg.productId === ''`,**NaN 两个都不满足**,于是构造出 `/NaN/{sn}/{测点}/measure/post` 这样的脏 topic 并持续上报。
- 修复:
```js
// mine.js handleMqttConnect()
const pid = Number(productId);
if (!Number.isInteger(pid) || pid <= 0) {
wx.showToast({ title: '产品ID必须是正整数', icon: 'none' });
return;
}
const creds = auth.getMqttCredentials(serialNumber, pid, productSecret);
...
storage.setMqttConfig({ serialNumber, productId: pid, productSecret });
```
### P0-2 设备数达上限时 `addDevice` 静默失败,UI 谎报"已连接"
- 位置:`utils/storage.js:132-139`(超限 `return`,无返回值)+ `pages/device/scan/scan.js:145-160`
- 根因:`addDevice` 成功路径没有 `return`,超限路径 `return undefined`。scan 页不检查返回值,继续执行 `activateDevice`(找不到该 SN,空操作)→ `getDevice` 返回 null → `gateway.start(deviceConfig)` 传入一个**根本没落盘**的设备 → `_startNow` 里 `_isEnabled()` 为 false → 不采集。用户看到 toast "已连接设备",实际既没保存也没连。
- 修复(两处):
```js
// storage.js
function addDevice(device) {
const devices = getDeviceList();
const index = devices.findIndex(d => d.serialNumber === device.serialNumber);
const encryptedDevice = encryptFields(device, ENCRYPT_FIELDS);
if (index >= 0) {
devices[index] = encryptedDevice;
} else {
if (devices.length >= LIMITS.MAX_DEVICE_COUNT) {
console.warn('[Storage] 设备数量已达上限:', LIMITS.MAX_DEVICE_COUNT);
return false; // ← 明确失败语义
}
devices.push(encryptedDevice);
}
saveDeviceList(devices);
return true;
}
// scan.js handleSelectDevice()
if (!storage.addDevice(deviceConfig)) {
wx.hideLoading();
wx.showToast({ title: `最多添加 ${50} 个设备`, icon: 'none' });
return;
}
storage.activateDevice(serialNumber);
```
### P0-3 Base64 编码用 `setUint8` 静默截断,非 ASCII 密钥加密后不可逆损坏
- 位置:`utils/crypto.js:19-24`
- 根因:`dataView.setUint8(i, str.charCodeAt(i))` 只写低 8 位。XOR 后码点可能 > 255(例如中文 `0x4E2D` XOR `0x66` = `0x4E4B` = 20043),被静默截断 → 解密得到乱码 → **产品密钥永久损坏且无报错**。密钥通常纯 ASCII 所以没暴露,但这是"输入一旦含非 Latin-1 字符就毁数据"的定时炸弹。
- 修复:
```js
function base64Encode(str) {
try {
const bytes = new Uint8Array(str.length);
for (let i = 0; i < str.length; i++) {
const code = str.charCodeAt(i);
if (code > 0xFF) {
console.error('[Crypto] 含非 Latin-1 字符,拒绝编码:', i);
return '';
}
bytes[i] = code;
}
return wx.arrayBufferToBase64(bytes.buffer);
} catch (e) {
console.error('[Crypto] Base64 编码失败:', e);
return '';
}
}
```
> 附带:`encrypt/decrypt` 异常时 **fail-open 返回原文**(`crypto.js:71`、`crypto.js:94`),会把明文当密文落盘。建议改为返回空串并让调用方拒绝写入。
### P0-4 切换 clientId 时订阅表被清空,`_resubscribe` 恢复不到任何东西
- 位置:`services/mqtt-client.js:177-181` vs `mqtt-client.js:409-417`
- 根因:`connect()` 在拆旧客户端时 `this.subscriptions.clear()`,而 `_resubscribe()` 只在 `client.on('connect')` 里执行 —— 执行时表已空。**任何一次换 clientId 重连都会永久丢失全部订阅**。当前业务只 publish 不 subscribe,所以未暴露;一旦接入下行指令就是线上事故。
- 修复:把"订阅表"的生命周期从"连接"解耦 —— 只在 `disconnect()`(用户意图)时清,换 clientId 时保留:
```js
// connect() 内
if (this.client) {
console.log('[MQTT] 终止旧客户端:', this.currentClientId, '->', newClientId);
this._teardownClient(this.client); // 保留 this.subscriptions,交由 _resubscribe 恢复
}
```
### P0-5 离线补传串行 `await publish`,publish 无超时 → 一次不结算就永久堵死
- 位置:`services/gateway.js:733-772`(`_flushOffline`)+ `services/mqtt-client.js:332-349`(`publish`)
- 根因:`_flushOffline` 用 `await mqttClient.publish(...)` 逐条串行发送,且被 `_flushingOffline` 布尔闸门保护。而 `publish` 的 resolve/reject **完全依赖底层回调**:QoS1 的回调要等 PUBACK 或重试用尽。若底层在 ws 已断、`_inflight` 条目回调路径异常时永不结算,`_flushingOffline` 将永远停在 `true`(`finally` 永远执行不到)→ **此后所有离线补传静默失效,直到重启小程序**。BLE 侧所有 GATT 操作都做了超时兜底,唯独 publish 没有,属于防御不一致。
- 修复:给 publish 加超时出口(与 BLE 的 `_callWithTimeout` 同一模式):
```js
// mqtt-client.js
publish(topic, message, options = { qos: 1 }) {
const timeout = options.publishTimeout != null ? options.publishTimeout : 15000;
return new Promise((resolve, reject) => {
if (!this.client || !this.client.connected) {
reject(new Error('MQTT 未连接'));
return;
}
let settled = false;
const timer = setTimeout(() => {
if (settled) return;
settled = true;
reject(new Error('发布超时'));
}, timeout);
this.client.publish(topic, message, options, (err) => {
if (settled) return;
settled = true;
clearTimeout(timer);
if (err) { console.error('[MQTT] 发布失败:', err); reject(new Error('发布失败')); }
else { resolve(); }
});
});
}
```
> 另外 `_flushOffline` 建议加并发上限或分批,避免 1000 条缓存时逐条串行 await 拖很久。
---
## 三、P1 — 隐患 / 性能 / 一致性
### P1-1 通知风暴:每次 `_notify` 全量构造快照,页面再全量重建列表
- 位置:`services/gateway.js:820-829`(`getSnapshot` 里 `dataList.map(it => ({...it}))`,最多 200 条)+ `pages/device/detail/detail.js:190-196`(`slice().reverse().map()`,ASCII 模式下每条再跑一次 `hexToBytes`)
- 根因:收帧节流 100ms(`TIMEOUT.FLUSH_INTERVAL`),即每秒 10 次 × 200 条对象浅拷贝 × N 个订阅者,ASCII 视图下还要叠加 200 次 hex 解析。高频采集时主线程和 setData 都会明显吃紧。
- 建议:① `_notify` 合并到同一帧(`setTimeout(..., 16)` 去重,状态类更新仍可即时);② `getSnapshot` 增加 `dataListSince(id)` 增量接口,页面只 setData 新增部分;③ ASCII 转换结果缓存到 item 上(`item._ascii`),避免每次重算。
### P1-2 「我的」页在每次网关通知时同步读全量离线缓存
- 位置:`pages/mine/mine.js:165`(`checkStatus()` 内 `storage.getPendingData()`)
- 根因:`getPendingData()` 会做过期过滤 + 写回(同步 IO)。mine 页订阅了 gateway,收帧期间每 100ms 触发一次 → 每 100ms 一次全量读盘 + 可能的写盘。而 gateway 快照里**已经有 `offlineCount`**,这次读取完全多余。
- 修复:`const pendingCount = snap.offlineCount;`
### P1-3 `_upload` 无并发上限
- 位置:`services/gateway.js:596`
- 根因:自动模式下每批帧逐条 fire-and-forget 调 `_upload`,无在途数限制。MQTT 变慢时会堆积大量 QoS1 在途消息,触发底层的"在途消息过多"错误后又全部转离线缓存,形成雪崩。建议加在途计数器(如上限 20,超出直接进离线缓存)。
### P1-4 `_toOfflineItem` 可能返回 null 却被 push 进缓存数组
- 位置:`services/gateway.js:721-730` + `gateway.js:599`
- 根因:`device` 为 null 时返回 null,调用处未过滤 → 缓存里出现 null 元素 → `_flushOffline` 遍历到 `it.deviceId` 抛 TypeError,整批补传中断。改为 `const item = this._toOfflineItem(...); if (item) offline.push(item);`
### P1-5 环境配置硬编码,dev 还混搭了两个环境
- 位置:`env.config.js:7-20`
- 根因:`const ENV = 'development'` 是手改开关,发布时漏改就会把生产包指向 `http://192.168.110.117:8080`(内网 IP + 明文 HTTP)。更隐蔽的是 DEV 配置本身自相矛盾:`baseUrl` 是内网 IP,而 `mqttServer` 是公网 `47.121.180.229` —— 同一个"开发环境"里 API 和 MQTT 不在一处,真机联调极易误判"接口通了但数据上不去"。
- 建议:① 用 `wx.getAccountInfoSync().miniProgram.envVersion` 自动判定环境,彻底取消手改开关;② 生产环境必须 `https://` + `wxs://`(`project.config.json` 里 `urlCheck: true`,HTTP 域名真机必然被拦)。
### P1-6 `BLE_STATE.CONNECTED` 是永不出现的状态
- 位置:`services/ble.js:141`、`ble.js:1027`、`ble.js:1035`,`services/gateway.js:520`
- 根因:`_setState(BLE_STATE.CONNECTED)` 在整个项目里**从未被调用过**(状态机只经过 DISCONNECTED → CONNECTING → DISCOVERING → READY)。因此这 4 处的 `|| state === CONNECTED` 全是死分支,`isConnected()` 与 `isReady()` 是两份完全相同的实现。要么删掉 CONNECTED,要么在建链成功时显式置位(让"已连但未发现服务"有独立状态)。
### P1-7 超时配置双真相源
- 位置:`utils/constants.js:7-8,26` vs `services/ble.js:16-32`
- 根因:`TIMEOUT.BLE_SCAN` / `TIMEOUT.BLE_CONNECT` / `RETRY.BLE_RECONNECT_DELAY` **全项目无引用**;而 `ble.js` 的 `DEFAULT_CONFIG` 里重新硬编码了相同数值(10000 / 10000 / 重连基数 1000)。改一处不改另一处就会静默失效。
- 修复:让 `DEFAULT_CONFIG` 从 constants 取值:
```js
const { TIMEOUT, RETRY } = require('../utils/constants.js');
const DEFAULT_CONFIG = {
scanTimeout: TIMEOUT.BLE_SCAN,
connectTimeout: TIMEOUT.BLE_CONNECT,
reconnectBaseDelay: RETRY.BLE_RECONNECT_DELAY,
...
};
```
> 另外 `services/command-manager.js:6` 引入了 `LIMITS` 却从未使用,是无用 require。
### P1-8 回归测试只覆盖了两个点,主链路零覆盖
- 现状:`tests/gateway-disabled-device.test.js`(33 断言)+ `tests/detail-console-toggles.test.js`(31 断言),质量很高(含反向验证防假绿),但覆盖的是**开关语义**与**设备停用**两条边角。
- 缺口:完全没有覆盖主链路 —— 收帧 → 测点归属快照 → 自动上报 / 手动确认 / 离线缓存 → 补传。而 P0-2、P0-4、P0-5 全部出在这条链路上。建议按现有 `tests/helpers/fake-wx.js` 的可编程桩,补一套 `tests/gateway-measure-flow.test.js`(MQTT 桩可设 connected / publish hang / publish fail 三种态)。
### P1-9 `app.json` 未声明定位相关私有接口
- 位置:`app.json`
- 说明:Android 端 BLE 扫描依赖系统定位授权,且微信要求小程序在后台配置隐私协议并在 `app.json` 用 `requiredPrivateInfos` 声明所用接口。当前文件里没有相关声明,真机(尤其 Android)可能出现"扫描无任何结果且无报错"。建议核对微信后台隐私协议并补齐声明。
### P1-10 ⚡(性能类最优先)收帧链路上每 100ms 有 4~6 次同步读盘,且同一 key 被读两遍
- 位置:`services/gateway.js:572-577` → `measure-point.js:37,70` + `gateway.js:615,630`
- 根因:`_flush()` 每 100ms 执行一次,其中:
| 调用 | 位置 | 实际动作 |
| --- | --- | --- |
| `measurePoint.getCollectMode()` | `gateway.js:572` | `wx.getStorageSync` |
| `measurePoint.getMeasurePointId()` | `gateway.js:573` | `wx.getStorageSync` |
| `_resolveProductId()` | `gateway.js:576` → `615` | `getMqttConfig()` = `getStorageSync` + `decryptFields` |
| `_resolveSerialNumber()` | `gateway.js:577` → `630` | `getMqttConfig()` **再读一遍同一个 key** + 再解密一次 |
叠加 `mine.js:165` 的 `getPendingData()`(读盘 + 过期过滤 + 可能写回)与 `gateway.js:832` 的 `_refreshOfflineCount()`,收帧期间每秒约 40~60 次 `getStorageSync`。`wx.getStorageSync` 是**同步阻塞**调用,缓存量大(离线缓存上限 1000 条、设备上限 50 台)时单次可达毫秒级 —— 这正好卡在 BLE 高频收帧的关键路径上。
- 修复方向:上报上下文(productId / reportSn / 测点ID / 模式)本质是**会话级快照**,不该每次读盘。建议在 `_startNow` 时快照一次,`_flush` 直接读内存:
```js
// gateway.js
_refreshContext() {
const cfg = storage.getMqttConfig();
const dev = this._device;
this._ctx = {
productId: (cfg && cfg.productId != null && cfg.productId !== '')
? cfg.productId : (dev ? dev.productId : null),
reportSn: (cfg && cfg.serialNumber) ? cfg.serialNumber : (dev ? dev.serialNumber : null),
measurePointId: measurePoint.getMeasurePointId(),
mode: measurePoint.getCollectMode(),
};
}
// 失效时机:_startNow / resume / 「我的」页保存 MQTT 配置后 / 详情页改测点ID 后
```
> 注意:测点ID 与模式允许在采集过程中被用户修改(详情页),所以失效时机必须覆盖这两处,否则会退回"归属串点"的老问题。
### P1-11 测试桩能力缺口,恰好卡住主链路用例
- 位置:`tests/helpers/fake-wx.js:193-229`(`createFakeMqtt`)
- 根因:假 mqtt-client 的 `publish` **恒 `Promise.resolve()`**,无法模拟 hang / fail;且缺少 `allowAutoConnect()`(`app.js:101` 会调用它);`createWx` 里没有 `wx.request`。这直接导致 P1-8 建议补的主链路用例(含 P0-5 的"publish 永不结算"场景)现在写不出来 —— 桩要先行增强:
```js
// fake-wx.js: createFakeMqtt 增加
publishMode: 'ok', // 'ok' | 'hang' | 'fail'
publish: (topic, message) => {
rec.publishCalls.push({ topic, message });
if (rec.publishMode === 'hang') return new Promise(() => {}); // 永不结算
if (rec.publishMode === 'fail') return Promise.reject(new Error('发布失败'));
return Promise.resolve();
},
allowAutoConnect: () => { client.userDisconnected = false; },
```
---
## 四、P2 — 死代码与僵尸注释(建议一次性清理)
重构(product-device-measurePoint)后遗留较多,用户偏好"僵尸代码及时删"。
| 位置 | 内容 | 说明 |
| --- | --- | --- |
| `utils/auth.js:61,72,83,91,101` | `buildPropertyPostTopic` / `buildFunctionGetTopic` / `buildPropertyGetTopic` / `checkLoginStatus` / `getCurrentUserId` | 5 个导出函数全无调用方 |
| `utils/hex.js:111,137` | `crc16` / `verifyCrc16` | 无调用方(且 `verifyCrc16` 用 `===` 比大小写的 CRC,含小写输入会误判失败) |
| `utils/storage.js:45,81,159` | `removeToken` / `removeUserInfo` / `updateDevice` | 无调用方。⚠️ 其中 `updateDevice` **不加密敏感字段**(与 `addDevice`/`saveDeviceList` 不对称),一旦启用就把 productSecret 明文落盘 —— 要么删掉,要么补 `encryptFields` |
| `services/command-manager.js:112-170` | `getCommands` / `addCommand` / `updateCommand` / `removeCommand` / `incrementUseCount` / `clearHistory` | 6 个方法全无调用方;预设命令功能随 `pages/device/command` 页一起消亡 |
| `utils/command-storage.js:13-93,178` | `getCommands` / `saveCommands` / `addCommand` / `updateCommand` / `removeCommand` / `clearAll` | 同上,整层预设命令存储已死 |
| `services/command-manager.js:22` | `sendCommand(hex, method, device)` 的 `method` 参数 | 僵尸参数,永远是 `'BLE'`,函数体从不读它。建议删参数或改名为 `_`(更彻底:直接删) |
| `pages/index/index.wxml:4-11` | 注释掉的 `user-bar` 块 | 导致 `index.js:144 handleLogout` 成为死方法。要么恢复 UI,要么连方法一起删 |
| `pages/device/detail/detail.js:15-16` | 注释声称"「发送命令」页(pages/device/command)保留未删" | ❌ **该页在仓库里根本不存在**。过期注释比没注释更有害,必须更正 |
| `components/device-card/device-card.wxss:1-51` | `.swipe-wrap` / `.swipe-delete` / `.swipe-delete-text` / `.is-dragging` | 滑动删除交互已改成按钮删除,约 36 行样式残留 |
| `utils/crypto.js:9,31` + `constants.js:31` | `CRYPTO_KEY` 硬编码 | 注释自己承认"生产应从服务端动态获取"。客户端常量 key 的 XOR 只是防君子,**建议明确降级为"防明文落盘的混淆"并在注释里写清,不要给人安全错觉** |
---
## 五、P3 — 打磨项
1. `pages/device/detail/detail.js:444-451`:radio 取消用 `setData({collectMode: 0})` + `setTimeout(0)` 回拉,是绕过小程序受控组件限制的 hack。可行但脆弱,建议在注释里写明依赖的是"radio 内部选中态先于回调切换"这一未文档化行为。
2. `services/gateway.js`:`_flushOffline` 成功补传后不回写 `dataList` 里对应 item 的 `offline` 标记,UI 上"已缓存"标签会残留到下次清空。
3. `pages/device/detail/detail.js:492-499`:`handleFlushOffline` 用前后差算条数,若被 `_flushingOffline` 短路会显示"已上传 0 条",误导用户。建议用返回值条数而非快照差。
4. `pages/device/scan/scan.js:26`:`gateway.stop()` 的注释写"暂停正在跑的网关",实际是完整 teardown(phase → STOPPED)。命名与语义不符,建议改注释或提供真正的 `suspend`。
5. `services/ble.js:305`:`scanDevices` 的超时定时器无句柄,`stopScan()` 无法取消它 → 点"停止"后 10s 到点仍会回填扫描结果并覆盖 UI。建议保存 timer 并在 `stopScan` 里 clear,或给扫描加 seq 作废。
6. `pages/mine/mine.js:18`:`appVersion: '1.0.0'` 硬编码,与 `project.config.json` 无联动,发版必漏改。
7. `utils/request.js`:未设 `timeout`(默认 60s),登录流程弱网下会长时间 loading;401 分支只 reject 不做登录态收敛(与 `app.isLoggedIn()` 只看 userId 的设计一致,但两处口径应写在同一处注释里)。
8. `components/device-card/device-card.wxml:8`:启用但未连上显示"连接中",实际可能是"异常/未连接",语义偏乐观。
---
## 六、本次未验证项(需真机确认)
- 真机上 MQTT 底层 QoS1 在 ws 断连时的回调是否必然结算(决定 P0-5 是"防御性加固"还是"已发生的事故")。
- Android 端 BLE 扫描在缺少定位声明时的实际表现(P1-9)。
- 200 条 × 10 次/秒 setData 在中低端机上的实际帧率(P1-1)。