fix(sync): 整页 apply 失败时降级逐条隔离,打破 pull 死循环

[Robs87]

修复内容

Fixes #367

根因

_applyPullPage 整页事务失败时返回 blocked: true，cursor 不推进。下次 pull 仍用 since=0，拉到同样的 500 条，同一条坏 change 再次失败 → 死循环，iOS 客户端数据永远同步不完。

服务端日志实证：

sync.pull device=dev_e1bdf867... since=0 returned=500 hasMore=True
sync.pull device=dev_e1bdf867... since=0 returned=500 hasMore=True
sync.pull device=dev_e1bdf867... since=0 returned=500 hasMore=True
... (无限循环)

修复方案

将 _applyPullPage 改为两阶段策略：

阶段	触发条件	行为
快路径	默认	整页批量事务，一起 commit（99% 场景，性能最优）
降级路径	批量事务抛异常	逐条独立事务，好 change 入库，坏 change 记入 pullErrors 并跳过，cursor 推进

回应 Review 意见

🔴#1 编译错误 — 已修复

sync_engine.dart:1237 的 logger.warning(tag, message, data, st) 传了 4 个参数，但 LoggerService.warning 只收 3 个位置参数。改为 logger.error(tag, message, error, stackTrace)。

🔴#2 降级跳过完全静默 — 已修复

问题：降级路径 pullErrors.record(...) 记下坏 change 后返回 blocked:false，但 _runPullLoop 在同一次 pull 里对整页每条 change 无条件 markResolved。markResolved = UPDATE ... SET resolvedAt WHERE change_id=? AND resolvedAt IS NULL → 刚记的错误立刻被标记已解决，UI 永远看不到。

修复：

• _PullPageOutcome 新增 failedChangeIds 字段，记录降级路径中失败的 changeId 集合
• _runPullLoop 中 markResolved 过滤掉 failedChangeIds 中的 change
• 坏 change 的 resolvedAt 保持 null，UI 通过 watchUnresolved() 正常展示

🟠#3 降级不区分瞬时/持久错误 — 已修复

问题：_applyOneWithBusyRetry 对 busy/locked 重试 2 次后 rethrow，降级 catch 一律 skip + 推进 cursor。某条好 change 若因瞬时锁竞争耗尽重试，会被永久跳过（cursor 已过，增量不再拉回）。

修复：

• 新增 SyncEngine._isTransientError(Object e) 静态方法，复用 _applyOneWithBusyRetry 中的 busy/locked 探测逻辑
• 降级路径 catch 中：瞬时错误 → 立即 abort 本页（blocked: true，cursor 不进，下次重试整页）；持久错误 → 隔离跳过（继续下一条）
• _applyOneWithBusyRetry 也改为调用 _isTransientError()，消除重复逻辑

🟠#4 补测试 — 已补

• 降级路径行为测试：5 条 change 中 1 条坏 → 4 条入库，1 条进 pullErrors，cursor 推进到 5
• markResolved 过滤测试：坏 change 的 resolvedAt 保持 null，UI 可见
• _isTransientError 判定逻辑测试：覆盖 SQLite busy/locked（瞬时）vs TypeError/FormatException（持久）

关于"先定位失败 change 根因"

Reviewer 提出的前置问题非常重要：在决定跳过之前，先搞清楚那条 change 为什么 apply 失败。

本次修复在降级路径的 logger.error 中增加了 err=${e.runtimeType} 字段，配合 pullErrors 表中已有的 error_class、error_message、rawChangeJson 字段，现在可以精确定位：

• 哪条 change_id 失败
• 哪个 entity_type / entity_sync_id
• 异常类名 + 第一行 message
• 完整的 change payload（rawChangeJson）

这意味着无论是 applyRemoteChange 自身的缺陷（如缺迁移、缺字段），还是服务端脏数据，都能从 sync_pull_errors 表中提取出来做根因分析。

降级路径的定位：它是安全网，不是替代品。对于真正不可恢复的脏数据，降级让同步继续；但对于 applyRemoteChange 的缺陷，应该通过上述日志定位后正面修复。

改动范围

仅修改 lib/cloud/sync/sync_engine.dart：

• _applyPullPage：两阶段策略 + 瞬时/持久区分 + failedChangeIds
• _runPullLoop：markResolved 过滤 + blocked 注释
• _applyOneWithBusyRetry：复用 _isTransientError()
• _PullPageOutcome：新增 failedChangeIds 字段
• 新增 SyncEngine._isTransientError() 静态方法

测试

• test/cloud/sync/sync_engine_e2e_test.dart：
  • 降级路径行为测试（好 change 入库，坏 change 隔离，cursor 推进）
  • markResolved 过滤测试（坏 change 不被 resolve）
  • _isTransientError 判定逻辑测试

[TNT-Likely]

Review 总评

死循环的根因链分析是准确的(blocked=true 让 cursor 卡在 0 → 反复拉同样 500 条),方向也对,fast-path 性能保住了,文档写得清楚。但当前不能合,有两层问题:一是实现层面没编译过 + 跳过是静默的;二是更根本的——在没搞清"那条 change 为什么 apply 失败"之前就直接跳过它。

🟥 先回答一个前置问题:那条失败的 change 到底是什么?

这个死循环是条件触发的:前提是待同步里某一条 change 在 applyRemoteChange 抛异常(且落在第一页 500 条内)。也就是说它不是普遍问题,而是"数据里碰巧有一条 apply 不过去的 change"的用户才会中。

但 issue #367 和本 PR 都只到"有一条 change 失败"为止,从没定位是哪条、为什么(描述里只是推测"脏数据/格式错误")。这恰恰是最该先查清的地方:

• 若失败是 applyRemoteChange 自身的缺陷(例如新版本写的某 entity_type/字段老客户端处理不了、缺迁移、apply 有顺序依赖)→ 正解是修 apply;此时"跳过"等于把一条合法数据静默丢掉,还把真 bug 盖住。
• 若确是服务端不可恢复的脏数据 → "隔离跳过"才合理,但必须可见(见 🔴#2)。

旧代码本来就 logger.error 打了 change_id+entity_type、并 pullErrors.record 落库了,报告人设备上应该能拿到。建议先补上失败 change 的 change_id / entity_type / 异常类名+message,据此判断是「该正面修的 apply 问题」还是「真脏数据」,再决定要不要跳。

死循环本身确实该防(卡死的客户端很糟),所以"降级别卡死"作为兜底有价值——但应是「定位并修根因」+「绝不死循环的可见兜底」两件事,本 PR 目前只做了兜底的一半。

🔴 必修

1. 不编译(blocking) — sync_engine.dart:1237

logger.warning('SyncEngine', '...(batch error: $batchError)', batchError, batchSt);

LoggerService.warning(tag, message, [data]) 只收 3 个位置参数,这里传了 4 个 → flutter analyze 直接报 error • Too many positional arguments: 3 expected, but 4 found,只有 error(tag, message, [error, stackTrace]) 才收后两个。结果:含该文件的构建 + 整个 test/cloud/sync/ 测试套件都编不过(本地实测编译失败)。说明这版没跑过 analyze/build。修:改回 logger.error(...),或把 batchError/batchSt 拼进 message。

2. 降级跳过完全静默,UI 永远看不到 — 与 _runPullLoop 冲突
降级路径 pullErrors.record(...) 记下坏 change 后返回 blocked:false,但 _runPullLoop(约 1178-1180)会在同一次 pull里对整页每条 change 无条件 markResolved(ch.changeId)。而 markResolved = UPDATE ... SET resolvedAt WHERE change_id=? AND resolvedAt IS NULL、watchUnresolved() 只查 resolvedAt IS NULL → 刚记的错误立刻被标记已解决。于是 PR 自述的"坏 change 记入 pullErrors,UI 已有展示机制"落空,实际是 skip + cursor 推进 = 静默丢数据、用户毫无感知。
修:_applyPullPage 返回本页失败的 changeId 集合,_runPullLoop 只对成功应用的 change markResolved(失败的不要 resolve)。

🟠 强烈建议一起处理

3. 降级不区分瞬时/持久错误 — 瞬时失败也会丢数据
_applyOneWithBusyRetry 对 busy/locked 重试 2 次后 rethrow,降级 catch 一律 skip + 推进 cursor。某条好 change 若因瞬时锁竞争耗尽重试,会被永久跳过(cursor 已过,增量不再拉回)。建议:复用那段 busy/locked 判定,瞬时错误不要跳——中止本页(cursor 不进、下次重试整页),只对持久错误才隔离。

4. 无测试 — 关键同步 + 数据完整性改动只给了"测试建议"没写测试。test/cloud/sync/ 有 e2e 设施,降级路径完全可测(造一条 apply 抛错的 change → 断言好的入库、坏的进 pullErrors 且不被 markResolved、cursor 推进)。补上的话,🔴#2 当场就能被测出来。

说明

• 本 PR 后 _applyPullPage 两条路径都 return blocked:false,_runPullLoop 里 if (outcome.blocked) 成了死代码 —— 要么清理,要么注释说明保留意图。
• sync_engine.dart:1485 的 curly_braces info 是历史代码(不在本 PR diff 内),非本 PR 引入。

---

综上 Request changes:🔴#1(编译)、🔴#2(静默丢数据)是硬阻断。更重要的是建议先定位那条 apply 失败的具体 change——很可能是个该正面修的 apply 问题,而不是"跳过了事";兜底降级保留亦可,但要能编译、可见、区分瞬时。辛苦了 🙏

[Robs87]

已按 review 意见全部修改完毕，请复审 🙏

修改清单

#	问题	状态	说明
🔴#1	编译错误	✅ 已修复	logger.warning → logger.error，参数签名匹配
🔴#2	静默丢数据	✅ 已修复	_PullPageOutcome 新增 failedChangeIds，_runPullLoop 的 markResolved 过滤掉失败的 changeId
🟠#3	瞬时/持久不分	✅ 已修复	新增 _isTransientError() 静态方法，降级路径中瞬时错误 abort 本页（blocked=true），持久错误隔离跳过
🟠#4	无测试	✅ 已补	3 个测试：降级路径行为、markResolved 过滤、_isTransientError 判定逻辑

额外说明

• _applyOneWithBusyRetry 也改为复用 _isTransientError()，消除重复逻辑
• PR description 已更新，详细回应了每条 review 意见
• 关于"先定位失败 change 根因"的建议：降级路径的 logger.error 已增加 err=${e.runtimeType}，配合 pullErrors 表的 error_class/error_message/rawChangeJson 字段，现在可以精确定位根因