Conversation
synchronize.ts 避免 误判和误删synchronize.ts 避免 误判和误删
There was a problem hiding this comment.
Pull request overview
该 PR 主要针对 Service Worker 侧的云同步(SynchronizeService)做并发串行化与关键 await 修复,避免多入口并发触发时出现误判、误删,以及“异步未等待导致状态/摘要提前落盘”的问题;同时补充了针对这些回归点的单元测试。
Changes:
- 为云同步入口增加同机串行队列(
stackAsyncTask("cloud_sync", ...)),并将原同步逻辑下沉到syncOnceInternal() - 修复多处“未 await”的时序问题(等待本地删除、等待 pull 安装、避免在运行中向
Promise.allSettled的数组动态 push 新任务) - 调整 orphan 云端脚本(仅
.user.js无.meta.json)的处理策略为保守跳过,并新增对应测试用例
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/app/service/service_worker/synchronize.ts | 引入云同步串行锁;修复 delete/install/push 等关键流程 await;保守处理 orphan 云端文件;补齐部分 Promise 链 catch |
| src/app/service/service_worker/synchronize.test.ts | 新增 3 个测试覆盖:串行队列、orphan 不删除、pullScript 等待 installScript |
| // .meta 文件可能尚未上传完成,跳过本次以避免误删云端脚本 | ||
| this.logger.warn("skip orphan cloud script without meta", { | ||
| uuid, | ||
| file: file.script.name, | ||
| }); |
There was a problem hiding this comment.
这里选择“跳过 orphan 云端脚本(只有 .user.js、没有 .meta.json)”后仍会在后续用本地脚本列表重写 scriptcat-sync.json。若 orphan 文件对应的是“另一台设备上传中断”的中间态,云端原有的 status 记录可能会被本机这次同步覆盖/丢失,导致排序/启用状态在多设备间被意外重置。建议:在跳过 orphan 的情况下保留/合并对应 uuid 的 cloudStatus(或直接跳过本轮写回 scriptcat-sync.json),避免把不完整状态当成权威状态回写。
|
|
||
| // 同步一次 | ||
| async syncOnce(syncConfig: CloudSyncConfig, fs: FileSystem) { | ||
| return stackAsyncTask(SYNC_SERVICE_TASK_KEY, () => this.syncOnceInternal(syncConfig, fs)); |
There was a problem hiding this comment.
syncOnce 现在被串行队列包裹后,内部任意未捕获异常都会导致 stackAsyncTask 返回的 Promise reject。仓库里存在未 await/未 catch 的 syncOnce 调用路径(例如 alarm 触发时直接调用),这会造成未处理的 Promise rejection(MV3 下可能导致日志噪音或 Service Worker 提前终止)。建议在队列任务内部统一 try/catch 并记录日志,或确保所有调用点都显式处理返回 Promise。
| return stackAsyncTask(SYNC_SERVICE_TASK_KEY, () => this.syncOnceInternal(syncConfig, fs)); | |
| return stackAsyncTask(SYNC_SERVICE_TASK_KEY, async () => { | |
| try { | |
| await this.syncOnceInternal(syncConfig, fs); | |
| } catch (e) { | |
| this.logger.error("sync once error", Logger.E(e)); | |
| } | |
| }); |
|
|
||
| type PushScriptParam = TInstallScriptParams; | ||
|
|
||
| const SYNC_SERVICE_TASK_KEY = "cloud_sync"; |
There was a problem hiding this comment.
SYNC_SERVICE_TASK_KEY 使用了字符串 "cloud_sync",与 SystemConfig 的配置 key 同名(cloud_sync)。这在排查日志/埋点或后续扩展队列用途时容易产生歧义。建议把队列 key 改成更语义化、与配置区分开的值(例如 cloud_sync_queue / cloud_sync_task)。
| const SYNC_SERVICE_TASK_KEY = "cloud_sync"; | |
| const SYNC_SERVICE_TASK_KEY = "cloud_sync_queue"; |
| if (metaObj.isDeleted) { | ||
| // 删除脚本 | ||
| this.script.deleteScript(script.uuid, "sync"); | ||
| await this.script.deleteScript(script.uuid, "sync"); | ||
| InfoNotification( | ||
| i18n.t("notification.script_sync_delete"), | ||
| i18n.t("notification.script_sync_delete_desc", { scriptName: i18nName(script) }) | ||
| ); | ||
| } else { | ||
| // 否则认为是一个无效的.meta文件,进行删除,并进行同步 | ||
| await fs.delete(file.meta!.name); | ||
| result.push(this.pushScript(fs, script)); | ||
| await this.pushScript(fs, script); | ||
| } |
There was a problem hiding this comment.
这次改动引入了关键的“等待本地删除/等待上传完成”的时序修复(await deleteScript / await pushScript),但目前测试只覆盖了串行队列、orphan 不删除、以及 pullScript 等待 installScript。建议补一个单测验证:当云端 meta 标记 isDeleted 或出现无效 meta 触发 pushScript 时,syncOnce 在写回 digest / scriptcat-sync.json 前确实会等待对应的 deleteScript/pushScript 完成,从而避免回写到中间态。
| it("serializes concurrent syncOnce calls", async () => { | ||
| let releaseFirst!: () => void; | ||
| const firstGate = new Promise<void>((resolve) => { | ||
| releaseFirst = resolve; | ||
| }); | ||
| const order: string[] = []; | ||
| const fs1 = createFs({ | ||
| list: vi | ||
| .fn() | ||
| .mockImplementationOnce(async () => { | ||
| order.push("first:start"); | ||
| await firstGate; | ||
| order.push("first:end"); | ||
| return []; | ||
| }) | ||
| .mockResolvedValue([]), | ||
| }); | ||
| const fs2 = createFs({ | ||
| list: vi | ||
| .fn() | ||
| .mockImplementationOnce(async () => { | ||
| order.push("second:start"); | ||
| return []; | ||
| }) | ||
| .mockResolvedValue([]), | ||
| }); | ||
| const service = new SynchronizeService( | ||
| {} as any, | ||
| {} as any, | ||
| {} as any, | ||
| {} as any, | ||
| {} as any, | ||
| {} as any, | ||
| {} as any, | ||
| { | ||
| scriptCodeDAO: {}, | ||
| all: vi.fn().mockResolvedValue([]), | ||
| } as any | ||
| ); | ||
|
|
||
| const first = service.syncOnce(syncConfig, fs1); | ||
| await Promise.resolve(); | ||
| const second = service.syncOnce(syncConfig, fs2); | ||
| await Promise.resolve(); | ||
|
|
||
| expect(order).toEqual(["first:start"]); | ||
| expect((fs2.list as any).mock.calls.length).toBe(0); | ||
|
|
||
| releaseFirst(); | ||
| await Promise.all([first, second]); | ||
|
|
||
| expect(order).toEqual(["first:start", "first:end", "second:start"]); | ||
| }); |
There was a problem hiding this comment.
serializes concurrent syncOnce calls 目前把“第一轮阻塞点”放在 fs.list() 内部,但 syncOnceInternal 在 list 之后仍会继续执行(状态同步、写 scriptcat-sync.json、updateFileDigest 等)。如果未来有人不小心把锁粒度改小/提前 resolve,第二轮仍可能在第一轮后半段开始而此用例依然通过。建议把 gate 放到更靠后的关键步骤(例如 updateFileDigest / storage.set / fs.create 写入)或增加一个“第一轮完成标记”,从而验证第二轮确实在第一轮整体完成后才开始。
- 新增 deleteScript/pushScript 等待 digest 更新的回归测试 - 新增 orphan uuid cloudStatus 保留的回归测试 - syncOnce 加 try/catch 避免错误冒泡破坏队列后续任务 - 跳过 orphan 时保留其云端 status,避免覆盖另一台设备的半上传状态
- scriptInstall 走 cloud_sync 队列,且 push 后才更新 digest - scriptsDelete 走同一队列,跳过 deleteBy=sync,结束后更新 digest - cloudSyncConfigChange 的 buildFileSystem 失败被 .catch 吞掉
scriptsDelete 入口先过滤 deleteBy === "sync" 的条目,避免 syncOnce 通过 mq.publish 回灌的 sync 来源删除事件再排一次 buildFileSystem + updateFileDigest 的空跑任务。
* 修正 `synchronize.ts` 避免 误判和误删 * ✅ 补充 synchronize 测试并保留 orphan 状态 - 新增 deleteScript/pushScript 等待 digest 更新的回归测试 - 新增 orphan uuid cloudStatus 保留的回归测试 - syncOnce 加 try/catch 避免错误冒泡破坏队列后续任务 - 跳过 orphan 时保留其云端 status,避免覆盖另一台设备的半上传状态 * ✅ 补齐 synchronize 队列与错误兜底测试 - scriptInstall 走 cloud_sync 队列,且 push 后才更新 digest - scriptsDelete 走同一队列,跳过 deleteBy=sync,结束后更新 digest - cloudSyncConfigChange 的 buildFileSystem 失败被 .catch 吞掉 * 🐛 避免 syncOnce 内部删除事件回灌触发空跑同步任务 scriptsDelete 入口先过滤 deleteBy === "sync" 的条目,避免 syncOnce 通过 mq.publish 回灌的 sync 来源删除事件再排一次 buildFileSystem + updateFileDigest 的空跑任务。 --------- Co-authored-by: 王一之 <yz@ggnb.top>
1. 给云同步加“同机串行锁”
文件:synchronize.ts
改动:
syncOnce()外面包了一层stackAsyncTask("cloud_sync", ...)。意图:
同一台设备上,云同步原本可能被多个入口同时触发,比如:
cloud_sync配置变化时alarm到点时原先这些路径可能并发跑,导致几个问题:
file_digest被后一个旧结果回写现在意图很简单:
同一时刻,同机只允许一个 cloud sync 相关任务执行,后来的排队,不并发。
2. 把真正的同步逻辑移到
syncOnceInternal()文件:synchronize.ts
改动:
syncOnce()只负责进队列,原逻辑挪到私有方法syncOnceInternal()。意图:
这是配合第 1 点做的结构调整。目的不是改行为,而是:
这是很典型的“外层加调度,内层保留原实现”。
3. 同步里遇到云端删除标记时,真正等待本地删除完成
文件:synchronize.ts
改动:原来是
this.script.deleteScript(script.uuid, "sync");现在改成
await this.script.deleteScript(script.uuid, "sync");意图:
原实现是“发起删除就继续往下跑”,这会产生时序错误:
syncOnce()可能已经继续写scriptcat-sync.json这样会出现“逻辑上说删了,实际上本地还没删完”的中间态。
现在改成等待删除完成,意图是:
这一轮同步里,凡是决定要删本地脚本,就等它真的删完,再继续后续状态同步和收尾。
4. 修复
result.push(...)在异步内部追加任务但不会被Promise.allSettled等到的问题文件:synchronize.ts
改动:
原来有一段逻辑是:
resultresult.push(this.pushScript(...))我把它改成:
await this.pushScript(...)result里追加新 Promise意图:
Promise.allSettled(result)只会等调用当时数组里已有的 Promise。如果 Promise 运行到一半才
result.push(...)新任务,这个新任务通常不会被本轮allSettled等到。原先风险是:
现在的意图是:
每个分支自己的后续动作,必须在自己那个 Promise 里完整跑完,不能偷偷追加“游离任务”。
5. 遇到“只有
.user.js、没有.meta.json”的云端孤儿文件时,不再直接删云端脚本文件:synchronize.ts
改动:
原来逻辑是:
如果云端有
xxx.user.js,但没有xxx.meta.json,就直接fs.delete(file.script.name)。现在改成:
意图:
原逻辑太激进。因为这种状态不一定是脏数据,也可能只是“另一台设备刚上传到一半”:
.user.js.meta.json还没来得及写这属于典型的“把中间态误判成坏数据”。
现在改成跳过,意图是:
面对疑似半上传状态,宁可保守,不做破坏性处理。
等下一轮同步再看它是否补完整。
这是这次改动里很重要的一点,直接针对“不稳定网络/中断过程/多设备并发上传”场景。
6.
pullScript()里真正等待installScript()完成文件:synchronize.ts
改动:原来是
this.script.installScript(...)现在改成
await this.script.installScript(...)意图:
云端拉取脚本时,原实现也是“发起安装就算成功”,这有几个问题:
pullScript()提前返回syncOnce()以为这个脚本已经同步完成这样会造成:
现在改成等待安装完成,意图是:
“拉取成功”必须以本地真正安装落库成功为准,不是以“已经调用了安装函数”为准。
7. 本地安装脚本后的增量云同步,也走同一个串行队列
文件:synchronize.ts
改动:
scriptInstall()原来是直接buildFileSystem(...).then(...)现在改成:
cloud_sync串行队列pushScriptawait updateFileDigest意图:
本地安装脚本后会立即往云端推送。
如果这个推送和定时同步、启动同步同时发生,原先会并发冲突。
现在让它也进入同一队列,意图是:
“全量同步”和“安装后的单脚本上传”共享同一调度通道,避免互相覆盖。
另外这里顺手把
updateFileDigest(fs)改成了await。原因很直接:digest 是同步状态的一部分,不该异步悬空。
8. 本地删除脚本后的云端删除,也走同一个串行队列,并在结束后刷新 digest
文件:synchronize.ts
改动:
scriptsDelete()原来也是直接buildFileSystem(...).then(...)现在改成:
cloud_sync串行队列await updateFileDigest(fs)意图:
和第 7 点同理。
删除操作如果和别的同步并发,风险更高,因为删除本身就是破坏性动作。
现在这样改,目的有两个:
file_digest,避免本地还记着旧云端摘要否则下一次同步可能继续拿旧 digest 做“无变化判断”,出现误判。
9.
cloudSyncConfigChange()增加.catch(...),避免未处理 Promise 异常文件:synchronize.ts
改动:
原来
this.buildFileSystem(value).then(async (fs) => { ... })后面没有
.catch(...)。现在补了
.catch(...),统一记日志。意图:
原先如果这里任何一步抛异常:
可能产生未处理的 Promise rejection。
这不一定直接破坏业务,但会让错误变成“静默漂浮”状态,不利于排查,也可能污染后续执行。
现在补 catch,意图是:
至少保证这条异步链的失败被显式记录,而不是悄悄漏掉。
10. 新增测试,专门验证这次修的同步行为
文件:synchronize.test.ts
现在保留的 3 个测试,意图分别是:
serializes concurrent syncOnce calls验证两个
syncOnce()并发调用时,第二个不会抢先进入,而是等第一个结束后再跑。这是验证“同机串行锁”确实生效。
does not delete orphan cloud script without meta验证云端只有
.user.js、没有.meta.json时,不会再直接删除脚本文件。这是验证“面对半上传中间态,改为保守跳过”。
waits for installScript during pullScript验证
pullScript()不会在installScript()还没完成时提前返回。这是验证“拉取成功必须等本地安装完成”。
这次保留的改动,核心目标不是“让同步更积极”,而是“让同步在并发、半完成状态、异步未等待场景下更保守、更串行、更不容易误判和误删”。